Order of port declarations in resulting JS influences behaviour
Jtalk opened this issue · 1 comments
Jtalk commented
SSCCE
https://ellie-app.com/dfBrYLCyNPga1
Environment
- Elm 0.19.1
- Elm/Core 1.0.5
Trivia
- This seems to be a special case of #896
- We're sending an outgoing port command that synchronously responds via another (incoming) port.
- When an outgoing port command is issued together with a model change that triggers an incoming port subscription, the response to that port might get skipped if the outgoing port is registered first in the resulting JS.
- In the above example, the first message from the button gets lost because Subs of the receiving port get updated after the Cmd is fired.
- In the above example, renaming
zsubscriptionstosubscriptionsputs it aboveupdatein the resulting JS, changing the behaviour of the button (Subs now come first & the incoming port's data is received correctly).
Problem
- The order of registration of incoming and outgoing ports in the resulting JS file influences behaviour for dynamic subscriptions.
Impact
- It makes synchronous data requests via Ports unpredictable as soon as we start making subscriptions dynamically.
- For me it breaks
localStorageaccess from nested components (components request data oninitand never get it back).
Possible fix
The problem seems related to how _Platform_dispatchEffects iterates over effect managers. It bundles Cmd and Sub updates into a single object & submits it to each manager in the order of their appearance in the managers object.
The fix would involve forcing the order, I see two options:
- Separate Sub and Cmd effects and submit them separately, iterating over the
managerscontainer twice (e.g. submit{ __subs: subs, __cmds: __List_Nil }to each of the managers followed by{ __subs: __List_Nil, __cmds: cmds }. The downside is that each manager would receive 2 events per lifecycle tick. - Introduce the idea of manager ordering & set priority for managers handling incoming ports over outgoing. It would require a new field in the managers, but the change should remain private within Kernel.Platform.
Since the existing behaviour is unpredictable, I guess it can be seen as backward-compatible patch-level fix.
I'm happy to raise a PR if you think it's worth fixing
github-actions commented
Thanks for reporting this! To set expectations:
- Issues are reviewed in batches, so it can take some time to get a response.
- Ask questions a community forum. You will get an answer quicker that way!
- If you experience something similar, open a new issue. We like duplicates.
Finally, please be patient with the core team. They are trying their best with limited resources.