-
-
Notifications
You must be signed in to change notification settings - Fork 405
A new reactive primitive: cell
#1071
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
base: main
Are you sure you want to change the base?
Conversation
|
NOTE: related RFC, tracked-storage-primitive, in implementation here: emberjs/ember.js#20814 |
|
I’d like to suggest that we consider alternative names for this feature, such as Aligning with these well-known terms could also foster greater consistency across frameworks, benefiting both new and experienced developers who are already familiar with these patterns. I also think this feature is a great addition to Ember. As reactive state management becomes a standard in modern frameworks, it's exciting to see Ember adopt this approach, providing developers with more flexibility and a declarative way to build dynamic UIs. |
|
I’d also like to propose introducing a new package for this feature, following a similar approach to the one in RFC #1068. Specifically, we could create a package like By using a clear, scoped package like import { ref } from '@ember/reactive';
// or
import { Cell } from '@ember/reactive';Additionally, this package could pave the way for moving |
|
I like where your head's at! As I've been looking to implement Cell in And a similar RFC in: I've found that a lot of stuff has a chain-of-re-exports... which is a bit annoying to work with. The underlying private packages from glimmer-vm export enough things where we could implement new APIs in ember -- but the tricky part is that if we want new default keywords, they need to go in the VM -- unless the VM provides a way to ember to add keywords (without build-time transforms, hopefully). So, this is probably an organizational thing we just need to fix. But this organizational split is probably why the re-exports exist in the first place -- glimmer-vm needs access to things, but we want better (and public) import paths for our users.
I don't really agree with
Until the signals proposal lands, I'm personally wary of landing on the name "Signal". The proposal in TC39 isn't even sure it wants to stick with that name. While the purpose of the Signals proposal is to unify the frameworks, within each ecosystem, developers are not meant to use signals directly, exactly, but to continue to use each of their ecosystem concepts and APIs. This makes Signals more of a low-level platform feature that app developers could use, and they would just work, but the benefits of using the ecosystem-specific APIs are far greater -- greater ergonomics, greater SEO, etc. This is why Svelte has "Runes", and why some other reactive frameworks are continuing to use "stores" (or "refs" in vue's case). For frameworks, Signals are an implementation detail rather than the user-facing API.
This is what the TC39 Signals Proposal is for: once implemented, frameworks can / should change out their underlying primitives for the platform-based signals implementation. No app dev would have to change a single piece of their code, but they'd suddenly be able to use raw signals if they want, and things would just work. The distinction is important, because already a As currently proposed at TC39, a Signal instance (for app-devs), has only the following APIs:
And that's it. The
So with the different set of APIs, and intent to make cell-usage ergonomic in templates (Signals would not be (until we get some kind of expression syntax)), I think it makes more sense to purposefully differentiate the name.
❤️ 🎉 |
|
Wondering, if it make sense to implement Also, seems we need a way to detect // or helper
function or(...args: unknown[]) {
return args.find((arg) => {
return isCell(arg) ? !!arg.value : !!arg;
});
}In addition, if we talking about |
maybe -- but I also don't think folks should be passing cells around without knowing that they're cells. for rendering tho, I'm thinking that we "collapse" the chains of "reactives": e.g.: const a = Cell.create(2);
const b = Cell.create(b);
<template>
{{a}} === {{b}}
renders: 2 === 2
</template>However, this convenience would probably lead to accidental passing of values without knowing their cells. All the reason to keep something like this low-level, and not expose to rendering. This is primarily an anti-footgun advice though. I'm not sure how common needing to check for cell-or-not would be, but due to future plans with similar APIs with current/read, I'd probably want something like: isReactivePrimitive(cell /* or formula (classless cached) */)resources could go in here, but there is a convention right now around calling a function first that return a resource -- this is typically where longer-lived state resides, and I don't think it makes sense to support using them in helpers (since helpers don't typically have lifetime) -- it'd make more sense to only allow resources to be composed minimally within resources, or any other higher-level concept -- this means that resources would not be a reactive primitive, but more some sort of lifetime primitive that has reactivity. This also lead mo to talk myself out of being opposed to HMMM |
|
@NullVoxPopuli, |
|
A note on the choice of
|
| * Prevents further updates, making the Cell | ||
| * behave as a ReadOnlyReactive | ||
| */ | ||
| freeze: () => void; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For "feature parity" with Object.freeze, would it make sense to also have isFrozen(), as in https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/isFrozen?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need to worry about checking this -- it's an optimization for caches'n' such
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tho, if you or anyone can think of a good use case for isFrozen, maybe we add it?
|
I was wondering about how exactly this relates to future TC39 Signals, and your long comment was very helpful in that regard. Maybe for future travellers a gist of that should be in the RFC, and maybe stating that once Signals eventually land, this would prevode the main integration point for them (subject to a future Signals integration RFC)?
Strong +1 from me! |
I think it's only one time learn thing, and there is no real reason to have If we have Imho, we should not do CS here, we should keep things simple in terms of naming and not create extra wording. |
|
Of note, a "TrackedArray" created with glimmerjs/glimmer-vm#1682 |
text/1071-cell.md
Outdated
| /** | ||
| * Utility to create a Cell without the caller using the `new` keyword. | ||
| */ | ||
| static create<T>(initialValue: T, equals?: (a: T, b: T) => boolean): Cell<T>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we support lazy values?
For example, if initial value is a tricky logic, there is no reason to resolve it during cell creation, because it may be used later.
// a-lot of compute
const value = new Array(10000).fill(null);
const arrays = Cell.create(value);
// lazy, no compute
const value = () => new Array(10000).fill(null);
const arrays = Cell.createLazy(value);(case for decorators too, where initializer is a may be reactive value, and we may be "looped")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What would the implementation of createLazy potentially look like?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like this: https://github.com/lifeart/glimmer-next/blob/master/src/utils/reactive.ts#L142 it's not 100% accurate, but main idea is same
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't dynamically created/modified objects create extra memory pressure?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@NullVoxPopuli it is, but it depends on usecase, in browser we limited by CPU, not by memory :)
also, it may be a case where we create value, but it's not used (set is called before get) and in this case we saving alot
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see why it couldn't be added later as well
It may make more sense to do lazy construction in objects/arrays/maps, etc?
…ations, and ensuring that there is only one in-memory copy of the class by making all arguments required
I second that. From an API perspective alone, if there is a As much as |
Cellcell
|
Let's make it clear that this RFC also effectively amends the earlier tracked storage primitives RFC to say that that one (which has no official implementation anyway) got superseded by this one. |
|
At RFC review we discussed whether we really need I also reiterated that I'm really not comfortable trying to use Cell as a foundational primitive without doing the "other half" of the design. The part that explains the low-level API for actually observing and reacting to Cell consumption. |
|
Resources RFC has started here: #1122 Ergonomics of Resources will somewhat depend on this RFC |
|
Coming here from the resources RFC. I understand the prior art with starbeam and the work that lead into signal proposal. This is the one thing, that keeps me irritation, the word Since it is embers signal impl, I'd be more happy to take a name more related towards that (same with |
|
What about I have been making a vanilla js library, and made an adapter for frameworks to provide their signal impl. Here is the one I made for ember: import { cell } from 'ember-resources';
const signalFactory: SignalFactory = <T>(t?: T) => {
const reactive = cell(t);
return {
get(): T {
return reactive.current;
},
set(val: T) {
reactive.set(val);
}
};
};and it works fantastically well. I used the TC39 signal RFC for my reference, and Now " Shouldn't it implement the import { signal } from 'ember/reactivity';
const signalFactory: SignalFactory = <T>(t?: T) => signal(t);I found it weird and wrong to use a wrapper for this. The argument for will be the technical implementation differs in nuances. Practically we will all carry around the same wrapper. Ideally agnostic libraries support signals and you can stuff in any reactive value from any framework and I would like Ember to support this. Similarly to how Standard Schema works. Maybe, there is a I know signal RFC is still under development and this can be defered, but a note on making this compatible would suffice (if there is, I overread it). |
| ### Naming: Cell | ||
| Naming in the broader context of the JavaScript ecosystem, and why we're using "Cell"[^other-rfcs-pending][^relationships-for-demonstration-only]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has been raised as a concern, an explanation is helpful for the background/research but doesn't eliminate the concerns. Actually it strengthens the the concerns.
After two (or more) years (?) of using ember-resources it was this RFC, that explained the origin for this word. It didn't become apparent throughout all this years.
Even after knowing, I find the word cell still terrible, as it contributes to the overall confusion already present in ember.
It is Ember's spiritual implementation of a Signal. Naming is pretty much given (by the final proposal then). We should be encouraging engineers trained on the usage of Signal and use the same API/naming as close as possible as the original (unless technical differences with ember's tracking disallow this).
At best:
- Engineers do know Signals (no special ember knowledge needed for this)
- Agnostic libraries support signals
- Ember's Signals can be used with agnostic libraries
- No extra education needed for using them in ember (apart from knowing where to import them from)
See my other comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is Ember's spiritual implementation of a Signal
it is not. This is the key disconnect.
Signals would be an implementation detail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No extra education needed for using them in ember
when/if TC39's Signals land, folks really don't need to use Cell, Tracked, etc.
folks can just decide to not learn anything and have a more primitive (😉 ) experience. It would all still work.
Signals are an implementation detail - when/if TC39 lands Signals, all our internals will be swapped for TC39's Signals (e.g.: we'll no longer need tags)
Now, we can add a This is something that can be added on to later. It's much easier to add APIs than re-work something that is fundamentally wrong in already in use. And! this is what the proposal encourages. The TC39 Signal's proposal encourages wrappers for frameworks to provide their own ergonomics around the lowest level primitive. We don't want users to be using Signals directly, because we believe we can provide a better out of the box experience (while at the same time, we know we have to be 100% compatible with folks who do use the lowest level APIs). Note TC39's Signals are still Stage 0 -- and while we can make sure that we don't make ourselves incompatible what TC39 is doing, it's far too early to assume their APIs aren't going to change -- they haven't even gathered implementor's feedback yet -- thinks could be vastly different by the time the proposal gets to Stage 1, let alone Stage 3.
Firstly, it's a bit too early to be using TC39 Signals. |
|
I'm experimenting right now with integrating other libs into ember. Some of them already have a signal-ish implementation which makes it compatible amongst other frameworks.
I highly disagree with that statement here. It is about compatibility (or the intend) to integrate with third party libraries, that work with signals1. Here is a nice example I find in integrating import { createAuthClient } from 'better-auth/client';
export const auth = createAuthClient();then there is a
That is, this is built on top of nanostores and Goal should be to have compatibility with these (I picked
That's cool.
Why do need this stuff then? Aren't Signals enough? Is this a requirement from the VM itself? I may not have run into the situation to need them. But reading on: Is
PS. Footnotes
|
There are bigger plans around this, which @ef4 is rightfully pressuring to me write out -- around the next phase of our renderr. In general though, the things you point out are for optimizations and ergonomics -- aiding in swapping out the VM with a new renderer.
nay. Signals don't provide any of the above (aside from set/get/read)
Everyone naming their non-signals signals is a causing a problem. Because then to specifically talk about the difference, you need more words.
naming for engineers is arguably something I think we should avoid, as we should strive to be understood by humans, rather than scratch the beards of fellow intellects <3
it's clear to me that the gaps left here in how my RFCs fit in to the bigger picture are .. noticable -- so my next step is to prototype out a little thing for our next phase of rendering and reactivity, and then RFC based on that -- this will basically be the Starbeam RFC |
But engineers are humans, no? This is a technical framework so its okay to have technical naming practices... Developer eXperience should be the focus here, not the end-users of products created using ember. |
Propose a new reactive primitive:
cellRendered
Summary
This pull request is proposing a new RFC.
To succeed, it will need to pass into the Exploring Stage, followed by the Accepted Stage.
A Proposed or Exploring RFC may also move to the Closed Stage if it is withdrawn by the author or if it is rejected by the Ember team. This requires an "FCP to Close" period.
An FCP is required before merging this PR to advance to Accepted.
Upon merging this PR, automation will open a draft PR for this RFC to move to the Ready for Released Stage.
Exploring Stage Description
This stage is entered when the Ember team believes the concept described in the RFC should be pursued, but the RFC may still need some more work, discussion, answers to open questions, and/or a champion before it can move to the next stage.
An RFC is moved into Exploring with consensus of the relevant teams. The relevant team expects to spend time helping to refine the proposal. The RFC remains a PR and will have an
Exploringlabel applied.An Exploring RFC that is successfully completed can move to Accepted with an FCP is required as in the existing process. It may also be moved to Closed with an FCP.
Accepted Stage Description
To move into the "accepted stage" the RFC must have complete prose and have successfully passed through an "FCP to Accept" period in which the community has weighed in and consensus has been achieved on the direction. The relevant teams believe that the proposal is well-specified and ready for implementation. The RFC has a champion within one of the relevant teams.
If there are unanswered questions, we have outlined them and expect that they will be answered before Ready for Release.
When the RFC is accepted, the PR will be merged, and automation will open a new PR to move the RFC to the Ready for Release stage. That PR should be used to track implementation progress and gain consensus to move to the next stage.
Checklist to move to Exploring
S-Proposedis removed from the PR and the labelS-Exploringis added.Checklist to move to Accepted
Final Comment Periodlabel has been added to start the FCP