mikesol / purescript-deku

A PureScript web UI framework
https://purescript-deku.surge.sh/
Apache License 2.0
123 stars 12 forks source link

RFC: getting rid of lock in Domable #91

Closed mikesol closed 1 year ago

mikesol commented 1 year ago

The Domable type constructor takes a lock parameter for the same reason ST does - to create a computational context. This is essential to the implementation of portal: portals create a referentially opaque blob that is a DOM element, and this element is tied to its context via the phantom type lock. This guarantees that a term carrying this type isn't passed out of its context (for example via a hook) - it will trigger an escaped skolem error.

Why is this necessary? Imagine that you have a dyn with a portal inside of it. That dyn is deleted, which means all of its children are deleted, including the portal. But if the portal is injected into other contexts (ie via a hook), then the contexts will have no clue what to do with it or its children. Neither it nor its children will have a deku id, for example, so deku won't be able to determine parent/child relationships. The lock parameter locks the portal into its context, but at the expense of a pesky type parameter.

But what if, internally, we represented portals as their referentially opaque blob and their constructor. The logic would go as follows:

  1. If the portal is defined in deku's internal engine, then by definition it is part of a current computational context and hasn't been deleted yet. Use the blob.
  2. If the portal is not defined in deku's engine, treat its constructor as a template for a deku component and create a new component.

The only downside of this is that it'd lead to a quirk where the portal, outside of its initial context, creates a new component every time it is referenced, which ofc defeats the purpose of portals. But that is better than a crash, and it could be a "feature" instead of a bug where we say that portals can only be used in the <#~> or dyn where they were created or direct children of those contexts, and any other usage is undefined.

Ofc, one could ask: "given that you have this nice type-safe language where you can avoid icky behavoir through the type system, why would you opt for a more permissive usage with undefined behavior in corner cases?" The answer is in deku's usage: folks pass around domables far more often then they use portals, and while I've seen people get tripped up by gnarly skolem errors that require writing out the full type, I've rarely seen anyone use portals, let alone use them in a way that sends them out of their original context.

In other words, lock currently prevents an incorrect yet esoteric use of an esoteric feature at the expense of the framework's ergonomics. This proposal would get rid of the lock parameter and give portals a sensible default (namely, they'd lose their referential opacity and become referentially transparent constructors like any ol' Domable) when used in a "naughty" way.

Thoughts?

MonoidMusician commented 1 year ago

is defined in deku's internal engine

Does this correspond to the existing scoping of the lock, or is it some kind of imperative state thing that could be defined or undefined depending on when it is checked? If the former I think it is okay to have different behavior for the thing that was previously checked by the type system – if it is embedding it directly or just not doing anything, I have no opinion.

mikesol commented 1 year ago

is defined in deku's internal engine

Does this correspond to the existing scoping of the lock, or is it some kind of imperative state thing that could be defined or undefined depending on when it is checked? If the former I think it is okay to have different behavior for the thing that was previously checked by the type system – if it is embedding it directly or just not doing anything, I have no opinion.

It corresponds to the existing scoping of lock. In other words, if a Domable's context is lock, then the internal engine must have a valid reference to it. Conversely, when a Domable with lock is deleted, that means that the internal engine frees the reference to it. Portals make this deletion safe by making sure that a referentially opaque Domable created by a portal can't be sent to an entirely different scope or even an entirely different runInElement if someone has two concurrent deku apps. If you try it, you get a skolem error.

So here, what we are getting (as you state) is different behavior for the thing that was previously checked by the type system. Whereas the type system previously prevented these domables from leaving their scope, in this RFC, they can leave their scope, but they'll act as a domable constructor as opposed to the blob.

To make it more concrete, let's say that a Domable is a portal containing a video, like in this example from the docs. In the current deku, it's impossible to inject this video anywhere else than a context governed by lock without hitting an escaped skolem. In this proposal, going outside of the context will still get you your video, but it will be recreated anew every time and will not be removed when a new one is instantiated. In other words, it'll be like if you wrote let v = D.video ... and then used v a bunch of places.

MonoidMusician commented 1 year ago

Okay, that sounds fine to me, and a big win for UX. (I confess I have not actually used portals tbh 😂)

mikesol commented 1 year ago

This is where the change would need to start:

https://github.com/mikesol/purescript-bolson/blob/7e65eef4f149b279c891c86896ae848b9e1a7cd5/src/Bolson/Control.purs#L48

Currently, giveNewParent takes very little information: basically what it needs to fetch the opaque element from the internal cache. We'd have to add an argument to giveNewParent of type obj lock payload that is the constructor to use when the ID isn't present.

Because the change is at the bolson level, deku, ocarina and rito will all be effected. ocarina uses portals tons (that's the fan operation, which is bread-and-butter stuff) and deku does as well, ie to pass a single scene to multiple three.js post-processors. Theoretically, this change will be for all of the frameworks, but I can't imagine it being problematic. For example, in ocarina, there are such limited opportunities to push an audio unit out of scope - you'd have to do it in the onended callback of an audio buffer, which I can't imagine anyone ever doing (famous last words...).

mikesol commented 1 year ago

Adding notes here as I come across ideas/issues.

The bolson implementation was straightforward and can be found here.

In deku, there's a thorny patch that can be summarized as follows:

  1. By the time a portal (or any deku component) has reached the interpretation stage, it knows nothing about events. For example, giveNewParent or setAttribute or createElement is an instruction received by the top-level subscription.
  2. When a portal is interpreted is when we'd make the call "is this in the cache or not?"
  3. If not, we'd need to make a new Domable. But as a Domable is essentially just an Event that spews instructions, we have nothing to subscribe it to.

So essentially this design would require deku to yield something like Event (Event Instruction) instead of Event Instruction, meaning that instead of each instruction like setAttribute or createElement being an atomic unit that is interpreted by the interpreter, each instruction would potentially contain others. Specifically wrt portals, a giveNewParent instruction that zaps a portal from A to B would contain all of the instructions to rebuild the element, and these instructions would be used to rebuild the portal in the unlikely event that it escapes its scope.

Now, you may be asking "but what if a portal contains a portal?" Well, that may require Event (Event (Event Instruction)). And then, you may ask "but what if a portal contains an arbitrary number of nested portals?" This leads to the inevitability that the new type would have to be something like Free Event Instruction. This is where performance issues start to worry me. Theoretically Free Event Instruction is fine, but as 99.99999% of these will evaluate to Event Instruction, and as a project may have 10k of these, the overhead of interpreting a free monad, even if lightweight, may add up to make the first render un-fast. This is just a premonition though: it could be that calling resume' on Free Event Instruction to gather up all the instructions is not a performance killer, but the possibility is there.

So we'd go from "a type-parameter used in a corner case of a corner case" to "a rococo implementation that accommodates a corner case of a corner case." Dunno which is worse!