Closed nettybun closed 3 years ago
Might add bundle size. ... It'll add to bundle size.
Just my personal advice: forget about bundle size.
I've fallen into this trap one too many times in the past - obsessing about size/perf before actually reaching a complete, functional design with passing tests. These things are super important, but they really are a distraction at this stage - the problem matter itself is difficult enough without burdening yourself with optimizations for size and speed - concerns that tend to make the code harder to read and change, and often more sensitive to subtle bugs.
Be kind to yourself. 😄
Issues with wire state
Wires are driven as a Finite State Machine (FSM). This is a one-dimensional value; internally a integer from 0 to 4.
Where state is used today
The 0-4 states are used here:
wire
(akacreateWire
) during creation:wire
during a run:wire.inner
childrenwireReset
wire.fn()
call duration_runWires
before calling wires:wirePause
:This already has problems, seen right in the FSM names themselves: I'm overloading terms to imply, for instance, that a wire can never be idle and stale, and that a stale wire is a paused wire. This gets really messy with the introduction of computed-signals that also have a stale state (which is stale but never paused; oh no) and needs to be skipped somehow possibly via another state.
signal
(akasignalBase
) during a read-subscribe:signal
during any read (pass or subscribe):signal
when writing a wire:_runWires
doesn't call it)_runWires
:Issues with this state model
Note that I'm not interested in trying to prevent people from explicitly doing bad things like
wire.state = xyz
. I just don't want to introduce unexpected or accidental state transitions.Here are some of those pitfalls:
If you call
wirePause
during the wire function run (wire.fn
) you can move your own state from RUNNING into PAUSED skipping infinite loop detection (!).Calling a wire to unpause is magic. Especially since it breaks the principal "Wires can always be run manually by calling them and this won't cause other side-effects within the engine."
Calling a wire of a computed-signal removes its STALE state but doesn't update the stored value of the computed-signal This breaks the signal until the wire is marked STALE again.
Computed-signal can be initialized with any wire including ones that are PAUSED or that have already had their function wrapped to support DOM patching. When the computed-signal is taken down (writing a value so it becomes a normal signal) the wire is set to RESET. This breaks the wire and prevents it from doing any other tasks such as DOM patching since all subscriptions have been removed. There's no way to know it had other tasks; they're lost in the wrapping.
Although computed-signals are set as STALE in
_runWires
, it's possible that they'll run somehow and end up as IDLE between the 1st and 2nd run loops. This means_runWires
must checkwire.cs
and not rely only on state... Is belonging to a computed-signal a state now?It doesn't make sense to pause a computed-signal but it's not good enough to just hope they don't try pausing it; it can happen accidentally. This happens in wire function wrapping (used to patch a single wire into multiple areas of the DOM; i.e chaining). If one of those chains was in a
when()
block, the wire would be re-adopted andwhen()
would pause all patches, not only its child elements.Issues with chaining
The issue above about chaining is very concerning. It also came up when I tried fixing computed-signal's to have their saved value updated when a wire is manually run - a small optimization but worth considering.
Why not just wrap the wire function like DOM patching does? The value will be updated via closure...
It'd be impossible to undo. Computed-signals need to be taken down, and there's no way to unwrap a function. It's also not safe to restore to the previously seen function since the wire could have been wrapped again during the computed-signal's lifetime.
Fixes
This is how I'm trying to address all these issues...
Both pause and resume should be dedicated methods
Wires should always run when manually called since it's their expected behaviour. Don't have a magic "unpausing" call. This means a new
wireResume
next towirePause
.It'll drop bundle size.
Replace chaining with a
Set
of post-run callsIt's not clear, explicit, and debuggable to have a dedicated
wire.hooks
Set
that tracks all the post-run actions. During takedown of a computed-signals I can usewire.hooks.delete(...)
via function reference.Might add bundle size.
Split state into a bitmask
I've seen projects like Kairo do that. It could be smaller than adding more individual properties like
wire.paused
.It'll add to bundle size.
Here's how I broke it down:
I can drop IS_CW and use
wire.cs
instead. This is a two-way link between a signal and wire. It's there for principles of consistency and debugging, so I'm keeping it. I can drop IS_WIRED and usewire.sigRS
as well.I'm still overloading the term "stale" which mostly means "skip" in
_runWires
context and "run later" in other contexts.Lastly I need to convey that a skipped wire is overdue for a run. I'd also like
wireReset
to be able to indicate that it's uninitialized;wire.run
doesn't work since wires can always be manually reset even after they're run.Here's how this works for the above cases:
Pause/Resume:
wirePause
: Sets SKIP_RUN_QUEUE. Has no impact to computed-signals because they already have SKIP_RUN_QUEUE._runWires
: If SKIP_RUN_QUEUE is set then set NEEDS_RUN. This is the same for computed-signals too.wireResume
: Clears SKIP_RUN_QUEUE only if it's NOT a computed-signal.Computed-signals:
signal
setup: Sets SKIP_RUN_QUEUE,wire.cs
, andwire.hooks.add(...)
.signal
read-subscribe: If NEEDS_RUN, run.signal
any read (pass or subscribe): If NEEDS_RUN, run._runWires
: If SKIP_RUN_QUEUE is set then set NEEDS_RUN. This is the same for paused wires too.signal
takedown: UseswireReset
(clears state; sets only NEEDS_RUN). Usesdelete wire.cs
andwire.hooks.delete(...)
.Reset via
wireReset
:sigRS
and propagate subscriptions (i.esigIC
).if (wire.sigIC.size) wire.sigIC = new Set()
vswire.sigIC = new Set()
vswire.sigIC.clear()
Scenarios that are fixed using this model:
Mark a wire as paused (sets SKIP_RUN_QUEUE), have a signal try to run it (sets NEEDS_RUN), and then manually run it - it'll clear NEEDS_RUN but leaves it as SKIP_RUN_QUEUE so it's still paused.
Resume a wire during it's wire run - it doesn't break the RUNNING state.
Manually run a computed-signal's wire that is in NEEDS_RUN - it'll clear NEEDS_RUN and update the signal's saved value using
wire.hooks
.Create a computed-signal using an existing/used wire - it sets SKIP_RUN_QUEUE (has no effect on pausing/resuming since they respect
wire.cs
). If the wire has never run it'll be in NEEDS_RUN and I'll run it; else it'll be idle* and nothing happens. It's always safe to usesigRS
.Other considerations:
I don't warn or throw if the wire they pass has an empty
sigRS
. If they pass an unrunnable wire with no subscriptions it should be easy to debug outside of a computed-signal. Not my problem.Regarding the *; it has to be idling (RUNNING=0) since if it was (RUNNING && NEEDS_RUN) they'd be initializing the computed-signal within the running wire and it'll throw a loop error.