Greedy `withAccumArray`

[dederer]

The resulting array Array (Sub i), which is passed to the commit function, contains duplicate elements. It looks like a bug.

Here is the code that demonstrates it (full code at: https://github.com/dederer/spork-accum-problem).

type Model = Unit
data Action = None
data Effect a = Effect String a
data Sub a = Sub String

app :: App.App Effect Sub Model Action
app =
  { render: const H.empty
  , update: const <<< App.purely
  , init: 
      { model: unit
      , effects: App.batch
          [ Effect "eff 1" None
          , Effect "eff 2" None
          ]
      }
  , subs: const $ App.batch
      [ Sub "sub 1"
      , Sub "sub 2"
      ]
  }

effectInterpreter :: forall i eff. Interpreter (Eff (console :: CONSOLE | eff)) Effect i
effectInterpreter = liftNat case _ of
  Effect t next -> log t $> next

subInterpreter :: forall i eff. Interpreter (Eff (console :: CONSOLE | eff)) Sub i
subInterpreter = Interpreter $ EventQueue.withAccumArray \queue -> do
  let commit :: Array (Sub i) -> Eff (console :: CONSOLE | eff) Unit
      commit new = log $ show $ new <#> \(Sub t) -> t
  pure commit

main :: Eff (App.AppEffects (console :: CONSOLE)) Unit
main = _.run =<< App.makeWithSelector (effectInterpreter `merge` subInterpreter) app "#app"

Expected output to console:

eff 1
eff 2
["sub 1","sub 2"]

Instead, as a result:

eff 1
eff 2
["sub 1","sub 2","sub 1","sub 2","sub 1","sub 2"]

[natefaubion]

I don't think this is actually an issue with withAccumArray. The issue is that subscriptions are really a view artifact, but we are calculating them on each app input rather than lazily, like we do with the view proper.

In your example, you have the initial subscriptions, and two initial effects. So it goes:

• Add initial effects to the queue
• Add initial subscriptions to the queue
• Process first effect, which synchronously pushes a new input
• Process second effect, which synchronously pushes a new input
• Process subscriptions
• Process first input, which results in subs being pushed again
• Process second input, which results in subs being pushed again
• Process subscriptions
• No more items queued, so commit the effect/subscription interpreter

Note that this results in the subscriptions being calculated 3 times during the course of the initial run of the event queue before the commit, which is why you get duplicates. I think the answer is probably to just calculate subscriptions whenever we render, not on each input.

[dederer]

I see. In your sub-example no effects and therefore no problem with duplicate subscriptions.

To avoid duplication in my example, I can use the special InitialSub as a marker:

data Sub a 
  = InitialSub
  | Sub String

Put it first in the subs:

  , subs: const $ App.batch
      [ InitialSub
      , Sub "sub 1"
      , Sub "sub 2"
      ]

And use it to remove duplicates:

subInterpreter :: forall i eff. Interpreter (Eff (console :: CONSOLE | eff)) Sub i
subInterpreter = Interpreter $ EventQueue.withAccum \queue -> pure 
  { init: []
  , update: \buffer i -> pure $ case i of
      InitialSub -> []
      _ -> A.snoc buffer i
  , commit: \buffer -> do
      log $ show $ buffer <#> case _ of
        InitialSub -> ""
        Sub t -> t
      pure []
  }  

How do you think, is there another, more correct solution?

[natefaubion]

The correct solution is to fix how the library queues subscriptions :P. This current behavior is clearly lacking. I might go ahead and change the event queue stuff to always take an Array as input, which avoids the need to have things like withAccumArray anyway.

[dederer]

Thanks!