-
Notifications
You must be signed in to change notification settings - Fork 16
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
Some long-needed renaming and consolidation #86
Closed
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit takes a first stab at some holistic renaming of internal concepts. I'm going to take another stab at what this commit calls "SubscriptionTarget" before submitting the PR (I think it's overly narrow), but the overall cleanups here are good. This commit also takes a step toward cleaning up and clarifying the implementation of subscriptions. More to come.
Run & review this pull request in StackBlitz Codeflow. |
Previously, the implementation of tags lived in `TIMELINE`, which has a whole bunch of other unrelated stuff. So the abstraction went directly from the interfaces defined in `@starbeam/interfaces` to the full-fledged timeline definition in `@starbeam/timeline`. This created an awkward situation where tags ought to have been real objects (so they can share utility code), but we had utility function instead that worked with the interfaces to avoid dragging all of TIMELINE into the definition of tags. This commit separates tags into their own package which clearly defines the semantics of tags and provides concrete implementations for them. This simplifies the code considerably, and also more clearly communicates what tags are for.
Also consolidate and simplify the functions for interacting with them
wycats
force-pushed
the
chore/naming-and-subscription-cleanup
branch
from
March 24, 2023 23:33
f5f5945
to
c0dc1b3
Compare
And start establishing a minimal Runtime concept (related to #87)
Getting close to a new runtime foundation!
For some reason, the iterable on collections isn't invalidating. The framework renderers also need to be updated.
wycats
changed the title
Some long-needed renaming
Some long-needed renaming and consolidation
Mar 27, 2023
The third attempt to reimplement resource on top of the new primitives worked beautifully. It supports `ResourceList`, but where the previous implementation worked by implicitly adopting resources across runs, the new implementation of `ResourceList` manages the lifetimes of its child resources explicitly. The code is nearly as compact, in part because the new resource implementation is more honest about separating entire-resource state from per-run state. There's a bit of logic still left over supporting the adoption use-case, which I will remove soon, but even with this leftover code, the new resource implementation is still fairly compact. Reworking resources is the last part of the current PR, and this commit largely wraps up the design work on that. The main remaining piece of work is updating the React and Preact renderers, which should be straight forward.
Superseded by #90 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR restructures the internals around the concepts that emerged from use over time, and cleans up and consolidates concepts that weren't paying their way.
[STATUS] The core of it is done except for resources. The renderers also need to be revised, but since they're based on the concepts that stuck around, updating them should be mostly superficial once resources are done.
The various "Internals" types have been renamed to "Tag" (the Glimmer concept, more or less, but with significantly more focus on avoiding the Glimmer footguns that came from free-floating tags disconnected from values).
Much more in the new tags README.
The library of reactive primitives was consolidated to focus on enumerating the fundamental primitives needed to build reactive abstractions. Cells and markers mostly got a coat of fresh paint, while Formula got a much more significant retooling. Previously, there was a lot of duplication and not-fully-fleshed out concepts (
PolledFormula
and the primitives to enable formulas that were manually started and finished, to support cases where a framework provides before/after lifecycle hooks but no way to wrap rendering itself).Formulas were consolidated and fleshed out into:
FormulaLifecycle
, which gives you a function to call when your computation is done. It then gives you a stableFinalizedFormula
back which you can callupdate()
on to repeat the process.FormulaLifecycle
replaces all of the frame infrastructure, as well as theFormulaValidation
infrastructure.Formula
, which takes a compute function and gives you a reactive value back. Like before,Formula
is also a function (so you can refactor a normal function into aFormula
and not have to change all of the callers to use.current
). You can subscribe to a formula, butFormula
replacesPolledFormula
: it's not cached. Which brings us to...CachedFormula
, which behaves exactly likeFormula
, but will return the last value when reactive dependencies haven't changed.This change in emphasis mostly comes from our experience implementing renderers that interface with existing reactivity systems. In the common case, we want to notify a framework that Starbeam dependencies have changed, but allow the framework's own reactive values to also invalidate the computation. For example, in React, we want users to be able to transparently use a
useState
value with Starbeam values, and have the component rerender correctly when either of them changes.Subscription without caching fits the bill perfectly, and it ended up being a more sensible default in mixed-reactivity environments. Caching is still frequently very useful, and is appropriate in situations where you know that your formulas only rely on Starbeam reactivity.
More in the new reactive README. I took a detour to flesh out the tags README, so it's not yet complete.
Finally, this PR starts to define the fundamental runtime requirements of the system. The current released version of Starbeam combines fundamental concepts with many, many conveniences in
TIMELINE
, and this PR sharply minimizes the runtime interface.This change focuses in on the two primary runtime requirements:
Defining the interface carefully highlights how truly minimal it is, which enabled a lot of the cleanup and simplification around frames and formulas. Among other things, the entire concept of "frame" has been completely eliminated: it was really a facade for a few methods that helped us evolve the requirements, but which had started to become the tail wagging the dog.
The conveniences that
frame
used to provide are now part of theFormula
primitives, which makes them easier to understand, but also useful in more situations.A couple of things I think will come out of this work:
@starbeam/shared
. TBD, but I have a much clearer picture of the requirements now, and think it's now clear what we need to do.This PR renames the
timeline
package toruntime
, but I have not yet written a README for it.