-
-
Notifications
You must be signed in to change notification settings - Fork 28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Zooming into a Var should not require an Owner? #119
Comments
Sorry, I've accidentally pressed Cmd+Enter and posted the half-written issue too early. I've now updated it with the full text. |
🥳 Very exciting! I do have some general thoughts/questions about the purity requirements. Do users currently depend on Perhaps the user could perform some logging/tracking in here? Yet, if one wished to execute some logic each time the parent And because the Of course, I completely understand that it's impossible to guess how users might be relying on some particular behavior. In fact, it's nearly certain that for any library of modest user-base, someone somewhere is pinning the hopes of their entire application on each incidental bit of functionality, conceived of and maintained by the author or not. And then, I'm sure the maintenance of such invariants is useful to you, yourself, as you develop a library with such intrinsic complication. You're clearly well versed in the multifarious gotchas of this domain, as well as the guarantees you've thus far painstakingly preserved. So, I guess what I'm devil's advocating for here is the possibility of the wholesale replacement of The benefits would be:
And the downside would be the relaxing of some previous behavior. But of course, the recommended alternative would be to simply observe the parent var's signal if you wanted to be notified exactly-once (and no less) per emission. I do see the argument for My lesser, backup (imp's advocacy) argument would be that perhaps the current behavior could get demoted to A final, orthogonal thought on API-design is that I do enjoy a post-fix pattern for method alternatives, if only because it more naturally works with autocomplete. The user would type Okay! All done! I'm super happy either way, just thought I'd put those arguments out there in case they're helpful. |
Those are all good points, thanks for elaborating! At the moment I can't give a specific, compelling, example, for what impure things the users could legitimately be doing in the I may be wrong in calling the requirement on Naming-wise, we already use
I must say that IntelliJ gives me all the relevant autocompletions regardless of whether the method starts with what I typed, or just contains it somewhere in the middle, so the main advantage of that strategy is kind of diluted IMO. Still, I try to follow this when the resulting method name sounds fine (e.g. |
Brilliant! Sounds like a plan 😎 I'll put my lil' demo code to work and report back if any weird side-effects crop up. 🥳🥳🥳 |
You can now preview this feature in Airstream v17.1.0-M1. It can be used with Laminar 17.0.0. The new method name is |
Background
Currently, when you
.zoom(...)
into aVar
to create a derivedVar
, Airstream internally maps over the parent var's signal, and adds an observer to it to make sure that it's strictly evaluated. To clean up the resulting subscription when this derived var is no longer needed, Airstream currently requires anOwner
to call thezoom
method.Requiring an owner here is annoying, as explained in #112, raquo/Laminar#130 and raquo/Laminar#148
Owner is not required
@kitlangton suggested on Discord that we don't actually need an Owner in this scenario, and it turns out that he was right!
Suppose we have:
We want derivedVar to be strictly evaluated, i.e. we should be able to call
derivedVar.now()
any time and get the value ofzoomInFn(parentVar.now())
. On its own, such "pulling" of a signal's current value does not require anOwner
. TheOwner
is needed to "push" new values fromparentVar
toderivedVar
wheneverparentVar
updates[1]. That's how zooming works today –derivedVar
is auto-updated wheneverparentVar
is updated, and callingderivedVar.now()
does not trigger any computation, it simply retrieves the already-computed value.In contrast, without such auto-updates, updating
parentVar
would not evaluatezoomInFn(parentVar.now())
immediately – it would only be evaluated if and when we callderivedVar.now()
. This way we don't need anOwner
, but we need to deal with other issues.Purity will be required
As you've likely noticed, using the Owner-less "pull" approach requires
zoomInFn
to be a pure function, since the number of times it's called is up to the user, as is the timing of those calls, e.g. for some updates ofparentVar
,zoomInFn
might not be called at all, if the user does not callderivedVar.now()
before the next update toparentVar
.Normally, Airstream's public API does not require user-provided callbacks to be pure, and this is by design. I think the batch version of the Var update method –
Var.update(var1 -> updateFn1, var2 -> updateFn2)
– is currently the only method that advises the user to provide pure callbacks – in that case it's because some of these callbacks might fail, and the whole batch update is aborted, so you wouldn't want any user-specified side effects to still happen in that case.In practice,
zoomInFn
is typically used to select aval
member containing an immutable value from an immutable class, so it's very likely to already be pure. So, maybe requiring purity here is a good tradeoff. Not needing anOwner
is a huge advantage, after all. Or, perhaps we should have a separate method to create such "lazy" derived vars, e.g.pureZoom(...)
instead ofzoom(...)
. Not sure yet – feedback welcome.Nesting and types
Currently we can safely assume that the type
Var
is strictly evaluated, so if wezoom
intoparentVar
,zoomInFn
will be evaluated wheneverparentVar
is updated. However, if bothzoom
andpureZoom
return subtypes ofVar
, we need to take care to maintain this assumption, i.e. if we callderivedVar.zoom
, derivedVar must become strictly evaluated even if it was "lazy" before (created withpureZoom
). I think this will happen naturally, asderivedVar.zoom
will startderivedVar.signal
using the provided owner, but this is one more area to watch.Similar concern applies to StrictSignal's proposed
pureMap
method (see below), make sure to also think that through.Sync with derived var's signal
Any Var's
.now()
value must always be in sync with this Var'ssignal.now()
. In practice, this should be easy to achieve – simply put the pull-based implementation of .now() in the Var's signal, rather than in the Var itself. This would also let us query the parent signal's_lastUpdatedId
to check whetherparentVar
has updated since the last time we calledderivedVar.now()
, eliminating redundant evaluations ofzoomInFn
(but not solving the problem of potentially missing evaluations if.now()
is not called, so purity is still required).As Kit suggested, the implementation of
.now()
would basically fall back to current behavior ifderivedVar.signal
is started (i.e. is being observed by other listeners), the pulling logic would only be enabled if the signal is stopped.Signals already re-sync their value with the parent signal when they're started after being stopped, so I think the sync between
derivedVar.now()
and current value seen byderivedVar.signal
's observers would be already built-in, although extra care needs to be taken to review the starting process ofderivedVar.signal
– I need to figure out the right time in the process to switch from standard logic to pull-based logic.Implications for other parts of the API (StrictSignal)
We restrict the Airstream public API to only expose memory-safe operations, to prevent users from accidentally creating memory leaks, or reading stale values. We can't rule out such things in principle (e.g. users can always pass the wrong owner), but we can make them exceptionally unlikely (e.g. by not requiring users to provide owners explicitly, having Laminar take care of it behind the scenes).
As mentioned before, one of the restrictions that I put on Airstream API is that it shouldn't require user-provided callbacks to be pure – but I put such restrictions for pragmatic reasons – to make Airstream more forgiving – not ideological reasons, and so such restrictions are subject to revision if the right tradeoff opportunity presents itself.
In this case, remember that Var is essentially a bundle of StrictSignal (the read channel including
.now()
) and Observer (the write channel). So, if we canzoom
into aVar
without anOwner
and get anotherVar
, we should be able tomap
aStrictSignal
without anOwner
to get anotherStrictSignal
, no? Currentlymap
-ing aStrictSignal
does not require an Owner, but it results in a regularSignal
that is lazy and does not have a public.now()
(see raquo/Laminar#130). We would then need to call.observe(owner)
on that mapped signal to convert it back to aStrictSignal
.So, we could override
def map
onStrictSignal
to return another StrictSignal without an Owner, with similar pull-based semantics as we discussed for lazy derived vars above. That way you could do e.g.:As you see, it's very much like a derived var, but without the write channel.
However, unlike derived var's
zoomInFn
, theproject
callback ofsignal.map
has more varied use cases, and requiring it to be pure gives me more of a pause. On the other hand, the regularsignal.map
is not affected by it, it's only the signals created viastrictsignal.map
that are affected, and only when the resulting signal has no observers – a case when currently, the project callback would not evaluate at all (and calling .now() on it is not possible).It is also not clear which of StrictSignal's methods would be "upgraded" to such purity-requiring versions that return StrictSignal instead of Signal.
map
– yes, but any others?recover
,debugWith
, it's not obvious which other ones would support this.Perhaps we could ease into this new pattern by adding a new
pureMap(pureProject)
method onStrictSignal
, and if we're doing that, we might as well implement the new lazy derived var functionality underpureZoom
, keeping the oldzoom
alive.Kit's implementation
This assumes that
zoomIn
is pure, and should give you a good idea of what using this kind of API would be like. You can already use this in your code, it uses extension methods and does not require any changes to Airstream internals.(I took the liberty of replacing
override def now
withoverride def tryNow
to make tryNow() work too)Summary
This approach looks very promising. At the very least, it works as "lazy derived vars" mentioned in #112, while retaining the full API of real vars (except for requiring purity). I will need to think some more how this finding affects all the other linked issues, perhaps more opportunities for improvements will be revealed.
Footnotes:
[1] because for that to happen,
parentVar
needs to keep a reference toderivedVar
, and that creates circular references which long story short, garbage collection can easily miss in realistic use cases, so we need an Owner to break those circular references when they're no longer neededThe text was updated successfully, but these errors were encountered: