Conversation
| pure = DynPure | ||
|
|
||
| dynToNode :: forall a. Dynamic a -> Node a | ||
| dynToNode (DynPure x) = unsafePerformEffect do |
There was a problem hiding this comment.
It's interesting: Dynamic Apply instance doesn't need this, Dynamic Bind instance does. And mapAsync does. It seems this should be avoidable so we have even less nodes. Or it isn't?
There was a problem hiding this comment.
Interesting observation!
In principle we could specialize bind and mapAsync so that we don't need it. It's just that in these cases I think we don't gain much, and it would require substantial work.
Firsy, in both cases the Pure has no chance of propagating further.
mapAsync on a Pure still has to return a DynNode, to handle the loading state.
And for RHS of a bind case: if the LHS is still dynamic, we can't propagate anyway. And we already handle the case where the LHS is Pure.
So this would be a small local gain. Async is usually so heavy that the specialization wouldn't make sense. For bind I think we can still consider it.
Second, in the current shape rhe change doesn't need to touch the Incremental layer, which buys us a but of "obviously correct"ness.
This PR modifies Dynamic by adding a special-case constructor for constants (
DynPure). Constants are propagated, for examplemapon a constant also produces a constant.This enables some optimizations:
DynPureis a no-op)This can help especially for computations which are expressed as
Dynamicbut where the actual dynamism is limited.