Some functions in purescript-signal are not referentially transparent

I’ve been doing a bit of purescript-signal and noticed that some functions break referential transparency!

I will make a mention as it looks like @CarstenKoenig is currently doing maintenance on it. Of course, this is also a question for anyone else.

First, every function.

tmp = every 1000.0

main :: Effect Unit
main = launchAff_ do
  Aff.delay $ Milliseconds 500.0
  let
    pulse = tmp
  liftEffect $ runSignal $ pulse ~> logShow
main :: Effect Unit
main = launchAff_ do
  Aff.delay $ Milliseconds 500.0
  let
    pulse = every 1000.0
  liftEffect $ runSignal $ pulse ~> logShow

These behaviors are different, but what we have done is replace the bindings.

Next, merge function

main :: Effect Unit
main = do
  channel1 <- channel true
  channel2 <- channel true
  let
    mergedSignal = merge (subscribe channel1) (subscribe channel2)
  send channel2 false
  logShow =<< get mergedSignal

output false

main :: Effect Unit
main = do
  channel1 <- channel true
  channel2 <- channel true
  send channel2 false
  let
    mergedSignal = merge (subscribe channel1) (subscribe channel2)
  logShow =<< get mergedSignal

outputs true

For the same reason, filters are not referentially transparent either.

Do you think these APIs should be changed?
Personally I would like to change them (haven’t come up with a solution…)
However, purescript-signal follows Elm’s Signal FRP, and changing them to Effective would result in a different API.

Especially, it is very critical for merge, because it will not be Semigroup if it is changed.

2 Likes

Hi,

I mainly try to keep this package alive and help out the original author as she seems to have moved on from PureScript).
As you say changes on the semantics would possible break some code out there so I’m wary to change that on the fly.

Could you add the output to your first example?

My personal take on this is that the way to go forward is either open an issue and see who people (and if possible downstream users) feel about changing the API or forking a new package that does.

EDIT: Removed a false remark - example is of course correct - sorry

1 Like

75.52480700612068
1076.4696190059185
2078.1452220082283
3079.518830999732
4080.8751610070467
5081.603973001242
...

578.351668998599
1580.3739890009165
2580.902148991823
3582.3207959979773
4583.194288998842
...

every starts counting when first called place, so the second program is off by 500 ms.

I, too, think there are two solutions to fix this.

  • Modify the original Signal
  • Create a new, modified package

I agree with the idea of creating an Issue and asking opinions.

1 Like

Issue is now open. If anything comes up, we will discuss it here in the future.

1 Like

Yes thank you - I think if there is no answer/discussion in let’s say 10 days or so we can go on and fix it (doing a major version change).
Don’t know if this package is used anymore in any performance concerned situations but it’s probably ok to wrap the signals into () => ... (make em lazy) or something like this.
Open for suggestions and maybe there will be some discussion here also.

Thanks again for bringing that up - given the new backend-optimizations I think it’s important to either fix this or at least give some warnings.

1 Like