rfcs
rfcs copied to clipboard
Add `tracked-built-ins` dependency
Status: this was on the agenda for this week’s Framework Core Team meeting, but we had a couple of longer discussions about other open RFCs before we got to it. I expect we’ll be able to cover it next week.
Thanks for writing this RFC. I ever wanted tracked built-ins from the get-go of @tracked
and was one of the first to suffer from not knowning this.array = this.array
is needed.
However, since then auto-tracking has made really great improvements on the technical implementations to track "more things" (which is why I like this RFC) - however on the API surface, this has led to severe problems, and I'm afraid this RFC will not solve but instead promote them.
I gonna explain them as a story I've heard way to often (only just this year) and then provide coniderations for the API surface.
A Story About @tracked
The benchmark is the mental model @wycats gave in his keynote this year:
Mark your mutable state as "tracked"
That's enough to explain the mental model of it and simultaneously have a manual for what to do. Now let's see where and how it breaks - a story in three acts:
1. Act: @glimmer/tracking
Especially people new to ember/trying ember the first time, they get the grip of this model real quick and are executing it right away. A code might be looking like this:
import Component from '@glimmer/component';
import { tracked } from '@glimmer/tracked';
export default class MyComponent extends Component {
@tracked items = [];
@action
addItem(item) {
this.items.push(item);
}
}
The developer rightfully marked the mutable state with @tracked
and was very disappointed to not see any reactivity in their app. Which resulted in blaming ember for announcing its fabulous reactivity system, which then isn't working ?!?
Is the developer to blame for not reading into deep why it is not working? No absolutely not - the API is not fullfilling the given promise of marking mutable state with @tracked
.
2. Act: tracked-built-ins
Let's continue. Eventually that developer found out to use tracked-built-ins
- so fine, change the import to this:
import Component from '@glimmer/component';
import { tracked } from 'tracked-built-ins';
export default class MyComponent extends Component {
@tracked items = [];
@action
addItem(item) {
this.items.push(item);
}
}
Alright, done but still not working? Although the mutable state was masked with @tracked
? Well under the hood, the decorator is a proxy to @glimmer/tracking
only usable for primitive re-asssignments. Instead you have to read through the docs to stumble upon this fact and actually learn, to use items = tracked([]);
. With that the API has changed from using @tracked
to mask your mutable state to use tracked()
to mask your mutable state. Actually, this line @tracked items = tracked([]);
has its purpose (thought I wouldn't recommend) - but explains this very-careful-to-use API.
Is the developer to blame for not reading into deep why it is not working? No absolutely not - the API is not fullfilling the given promise of marking mutable state with @tracked
.
3. Act: ember-deep-tracked
Some might be finding ember-deep-tracked. Apparently, this package fullfills the promise of "mark your mutable state as tracked". You'll go by @tracked something = anything;
and it make your application reactive when the tracked state changes.
Considerations for an API
This is growing/becoming one of the most complex and careful APIs to use putting stress on the consumers of this API making their time developing apps less enjoyable.
When it comes to invocation, we've spotted @tracked
and tracked()
in the wild. I don't mind which of these invocations to use, but I want a consistent usage of them and more importantly not a mixture. I may see a different usage between application and tests. For historical reasons and without any further investigation, this seems fine to me:
- Application:
@tracked
- Tests:
tracked()
Additional "benchmarks" that I consider confusing and a hassle to work with such an API:
- If I need to think from where I import tracked (!= "don't make me think") => fail
- If I already have a
tracked
in my current file, but this is not the correct one and have to import a second import and reassign it to a different name => fail - If I read
@tracked
somewhere I cannot be sure what it actually tracks before reading the import => fail
Ideally, there is only one public export by the whole library: import { tracked } from '@glimmer/tracking';
and that is all consumers ever need.
Summary
As said in the beginning, I like the intent of this RFC to track more things. The way the API is emerging into drives more people to blame ember for its complicated API and turn around to try another framework instead. I would heavily, heavily advise to revisit this API surface.
Personally, I'd like to see a merge of tracked-built-ins
and ember-deep-tracked
into @glimmer/tracking
to make them work under the hood for what they become excellent at but keep them exporting one tracked
.
I echo everything @gossi said, and from fielding questions from dozens of new developers at work (both new in general, and more experienced, but net to ember), the api surface bloat in absolutely a problem. And merging the addons seems like a good way forward. Though, maybe deep requires a flag to be set, cause perf
import { tracked } from '@glimmer/tracking';
class Demo {
@tracked num = 2;
@tracked str = '2';
@tracked({ shallow: true }) array = [];
@tracked({ shallow: true }) obj = {};
@tracked({ shallow: true }) array = new Map();
@tracked({ shallow: true }) aset = new Set();
@tracked({ shallow: true }) weekarray = new WeakMap();
@tracked({ shallow: true }) weakset = new WeakSet();
@trackkd({ deep: true }) deep = [{ }]
}
This should be doable in a non-breaking way with the existing @glimmer/tracking
FWIW I disagree fairly strongly that deep-tracking should be a built-in capability or especially a default, but that's my own take only, and I’ll be curious to see what other folks from Framework, Learning, TypeScript, etc. think as they look at this and chime in.
I think it’s important to keep in mind that one of the goals of auto-tracking as a reactivity system is to be as small a layer over normal JS as possible. All of the things you pointed to are real things to learn, but they are things people need to learn about JavaScript’s semantics. If you want to track a Map
, you tracked(theMap)
or call new TrackedMap()
; doing @tracked someProp = theMap;
just does have different semantics (and that’s still true of things with the Stage 3 decorators proposal where that would be @tracked accessor someProp = theMap
, it's just more obvious there). While we could make it work there, doing so would be making tracked
much, much more complicated a thing to reason about, and while that can seem nice at first it also comes with costs, including to teaching.
Having small friction points or bumps can actually be really useful pedagogically, because they can help people. Our goal is not to have people avoid having to think about these things at all; it's to have them able to think about them at the appropriate level of granularity. It is of course possible to reasonably differ on what that level of granularity is, but it's not a given that “the most exhaustive end-to-end implementation” is the right spot in the design space.
one of the goals of auto-tracking as a reactivity system is to be as small a layer over normal JS as possible
I 100% agree with this, but I don't see the decorator as the reactivity primitive. It's a DX abstraction, and should focus on improving DX around the primitives (which is what @gossi and I's comments are really trying to arrive at).
much, much more complicated a thing to reason about
how so? of the non-OSS folks I've talked to, 0 folks have thought what we have today is a great situation
tracking objects and arrays, and creating dynamic buckets of tracked data is one of the most common questions from new starters. And since we have a potential to provide an obvious solution that feels good, I think we should explore it:
-
@tracked
- reference -
@tracked({ shallow: true })
- objects/array/map/set // throw on anything else? idk
that deep-tracking should be a built-in capability or especially a default,
I'm 100% here on the default, and I'm only 50% there on built-in. It's fairly trivial to build, but there are a number of things that folks have tried to use deep tracking for that just aren't possible -- and I don't want folks to get bitten by that.
I'm not sure my 👍 adequately conveys how much I agree with @gossi here. So let this by even more of a 👍
My ideal is that @tracked obj = {}
would just work. If tracking deeply by default is a significant performance hit then I'd like to see some type of warning rise up in linting requiring either @tracked(deep: false)
or @tracked(deep: true)
whenever some non-scalar is tracked. If only certain types of deep tracking are a significant hit then it would be much better for this to work by default and throw a development assertion whenever something too deep is encountered.
I found firsthand these pain points when I was transitioning from vue/react to Ember. It was the exact same path- where I found that I had to use other tracked imports to get what I wanted done done, or use some unsatisfying this.array = this.array
. I found it exceedingly awkward to import another tracked
and then have to name that tracked differently than the original tracked
I imported from @glimmer/tracking
.
I would really love to see @tracked({deep: true})
as a default, and then the opportunity to do shallow tracking if performance requires it. I think this would help developers learn the api in the correct order and eliminate these road bumps until they are more familiar with the framework or need to optimize.
Where does this mental model come from?
let array = [];
array.push('foo'); // array has just been mutated
Because to me array has not been mutated there. The underlying data structure (elements within the object have mutated but the variable array remains the same. The by-reference mental model harks back to the C days and has remained in programming languages through PHP, Ruby, Python, Java, etc. If we want the construct that mutating the encapsulated data within an object to mean mutating the reference itself then we are talking about immutable style code which is what immer.js is for.
React and Vue have the advantage of a virtual DOM allowing every mutation to be expressed as an immutable (pure) function (build the whole thing in one go). By contrast Ember cherry picks its updates based on what things it knows have been consumed and then mutated. Thus the mental models we have in the React mind set are not the same when in the Ember mindset and I think that should be okay.
I will admit there is some sugar (and performance improvements) to the idea of new TrackedMap()
but I don't think I would often find a need for such a thing (I haven't yet).
What I have found lacking is having a clean way to express consumption within my own classes. I often make classes to encapsulate my own data and within might use alternative primitives that are not as ergonomic when the only exposed API is @tracked
. It would be nice to have some low level APIs we can use to make our own performance optimized data objects.
for clarification on my stuff - I'm 100% in camp performant by default (which is just references) -- but I don't think that has to be at odds with the additional behavior(s)
Agree with performant by default, but the framework should provide a comprehensive toolset so developers won't be frustrated when they realize there are no built-in solutions.
Developers definitely will be encouraged and feel satisfied when they see a complete API around the reactive system; take Vue.js as an example:

No one would complain about too many features a swiss knife has, as long as they've been designed correctly and adequately. also, people should learn and understand the nature of language itself to choose the right tool for the right task.
The framework, on one side, provides the whole toolset to prove its powerfulness and flexibility. On the other hand, offers high-quality guides and other materials to teach users how to use them. These two aspects are not mutually exclusive.
Agree with performant by default, but the framework should provide a comprehensive toolset so developers won't be frustrated when they realize there are no built-in solutions. 💯
👋 I started using Ember about a year ago and used React before. I do have quite some experience with React and still use it for my biggest side hustle (Planubo). The tracked property was one thing that confused me since Ember does not handle it out of the box like React did.
After React switched to hooks, it took a while for most folks to find out what exactly is needed, but once there, it provides a bunch of things like useState
, useEffect
with dependency array, useRef
etc out of the box to get what you need. No need to google some other packages that might be available and if they are, that they are still maintained as well.
Personally, I'd like to see a merge of tracked-built-ins and ember-deep-tracked into @glimmer/tracking to make them work under the hood for what they become excellent at but keep them exporting one tracked.
☝️ Yes please 🙏
From @NullVoxPopuli
I'm 100% here on the default, and I'm only 50% there on built-in. It's fairly trivial to build, but there are a number of things that folks have tried to use deep tracking for that just aren't possible -- and I don't want folks to get bitten by that.
☝️ Can relate. Happened to me. 💯
From @NullVoxPopuli
@tracked num = 2;
@tracked str = '2';
@tracked({ shallow: true }) array = [];
@tracked({ shallow: true }) obj = {};
@tracked({ shallow: true }) array = new Map();
@tracked({ shallow: true }) aset = new Set();
@tracked({ shallow: true }) weekarray = new WeakMap();
@tracked({ shallow: true }) weakset = new WeakSet();
@tracked({ deep: true }) deep = [{ }]
☝️ 🤤
Notably useState
does not have the equivalent of deep tracking, and does have (or is otherwise in the space of implementing) equivalents to useRef
and useEffect
. I think it‘s one thing to say that Ember should provide comparable tools there—we all agree!—but quite another to determine exactly the right shape of the API for them. @tracked
and useState
have exactly the same basic behavior—and when you’re updating reference types, you’d more likely reach for useReducer
instead, but both useState
and useReducer
more or less require you to do the functional-style/immutable update! Simply by providing (a) the tracked versions of the most common built-in data structures and (b) the lower-level primitives on which @tracked
, TrackedArray
, etc. are all built, Ember has provided the same degree of support there.
That does not by itself determine whether we should also add further capabilities in the form of a deep-tracked variant of the tracked
decorator, of course. But it’s important to be clear about the actual details of the claims and comparisons we make!
Ember has provided the same degree of support there.
I do not think comparing our stuff to immutability APIs does us any favors though because we encourage mutability, so we inherently must consider different ergonomics.
(I'm still :shrug: about the deep tracking, btw -- more just pointing this out as something to consider -- we don't want to base decisions off comparisons to other framework's utilities (but it is totally fine (and encouraged) to be inspired by them!))
@NullVoxPopuli I addressed that explicitly by noting that we do in fact already provide support for common mutable data structures. Deep-tracking is its own ball of wax that goes very far beyond just having good support for mutability. That, again, is not getting into whether or not it’s worth having available with tracked
, much less whether or not it should be the default; it’s simply noting that “We encourage mutation so we should have built-in support for deep tracking in the primary tool used for tracking” is a non sequitur in the strict sense! It’s appropriate and important to consider what patterns our API designs encourage.
For example, even if we choose to support deep tracking out of the box, having it come as a different import (rather than options on the main tracked
tool) could be a really useful signal: This is not the default case. That could also be handy for documentation purposes, for simplicity of implementation, for performance (a future where if you don’t use deepTracked
, you don’t pay for it), for types (not needing lots of different overloads; overloads in TS are huge pain), etc.
I put this on the agenda for today’s spec group meeting (spec group == breakout session from the Framework Team meeting, where we deep dive on technical details of things, which other relevant folks are invited to as makes sense!) so we can hopefully make some progress on it!
We did not get to this today (you can see notes on today’s meeting here; we covered a lot, but didn’t make it to this)—but it’s at the top of the list for next week’s meeting!
At our last Framework team discussion of this, we tentatively concluded that the one area we might want to leave open the idea that constrain ourselves to match the public APIs except for constructors, since (e.g.) new TrackedObject()
can never actually produce the desired type, we don't have the ability to forbid access to the passed-in object, etc.[^rust] There's more design work to be done here, but I'm hopeful we'll have a conclusion there in the next few weeks.
On deep tracking, we noted that we think it's a valuable feature but (a) not actually obvious exactly what the right semantics should be, (b) definitely not viable as part of tracked()
itself, and (c) probably something which can be tackled separately.
As for (b) and the proposals above, consider—folks here have suggested this should work out of the box (as indeed it does today with tracked-built-ins):
let counter = tracked({ count: 0 });
Folks have also suggested that one of these should work:
@tracked({ shallow: true }) someObj;
@tracked({ deep: true }) someObj;
These are mutually exclusive. Either tracked()
can accept a POJO and decorate it, or tracked()
can accept configuration options and return a property decorator. It can't do both! (Yes, you could special-case shallow
or deep
but… that would be terrrrrrrible API design. We’re definitely not doing that!) Net, while deeply-autotracking is an interesting use case, it definitely will not be part of the API of tracked()
itself.
[^rust]: Rust-style move/ownership semantics sure would be nice here… 😏
Small comment to cross reference with the ember-modifier
RFC, as I think the same constraints apply here and that this RFC will have to do whatever we do there.
https://github.com/emberjs/rfcs/pull/811#issuecomment-1129186872
@SergeAstapov can you similarly update this one? The updates needed (as far as I recall) are:
-
to match the changes suggested in #811
-
to be explicit that we will be decorating only the 6 types most commonly used in real-world JS today (
Array
,Object
,(Weak)?(Set|Map)
, but that we are open to adding other built-in data structures in the future if and as usage demonstrates the necessity thereof -
to specify a design for the constructors, including the relationship to the original object passed in—i.e. that we do a shallow clone on the object, because an API where you mutate the original has extremely surprising results
-
to explicitly state that we are not adding any deep tracking in this case, because:
- the correct semantics for a framework-level hook there aren't obvious
- it is possible to build variants on it fairly cheaply in user-land given
tracked()
as updated here
(Feel free to ping me on Discord if you want/need input on the details there!)
I want to add another perspective on the question of tracked(new Map())
vs new TrackedMap()
: Extendability.
The target for tracked-built-ins
and this RFC is adding tracked version of Array
, Object
, Map
, Set
, WeakMap
and WeakSet
. I think that will cover many use cases. But it would be great to not block adding tracked versions of more built-ins classes in user-land.
The primitives available today should allow building tracked versions of Date
, BigInt64Array
, BigUInt64Array
, Float32Array
Float64Array
, Int8Array
, Int16Array
, Int32Array
, Uint8Array
, Uint16Array
, Uint32Array
and I guess many more, which I forget about. It is very likely that some projects may want to use tracked versions of them as well.
Extending tracked
function provided by @glimmer/tracking
would not be possible in user-land. Or at least would require monkey-patching, which we do not want. An addon providing a tracked version of Date
would either require exporting its own tracked
function, which adds support for Date
objects. Or it would need to export a TrackedDate
. Let's have a look how that looks in practice:
import { tracked } from 'tracked-date';
const aDate = tracked(new Date());
const aPojo = tracked({});
This pattern might look acceptable at first look. It is actually what tracked-built-ins
does today. But it does not scale. As soon as there is a tracked-arrays
addon, which provides tracked versions of Int8Array
etc. it break. The tracked
function from tracked-arrays
would not support Date
and the tracked
function from tracked-date
would not support Int8Array
. :exploding_head: Consumers would end up doing something like this:
import { tracked } from '@glimmer/tracking';
import { tracked as trackedDate } from 'tracked-date';
import { tracked as trackedArray } from 'tracked-arrays';
const aInt8Array = trackedArray(new Int8Array());
const aDate = trackedDate(new Date());
const aPojo = tracked({});
Let's compare with exporting tracked classes instead of a tracked
wrapper function:
import { TrackedObject } from '@glimmer/tracking';
import { TrackedDate } from 'tracked-date';
import { TrackedInt8Array } from 'tracked-arrays';
const aInt8Array = new TrackedInt8Array();
const aDate = new TrackedDate();
const aPojo = new TrackedObject();
The last version seems much cleaner to me.
One might argue, that tracked
wrapper function could support all mutable standard built-in objects. But that comes with trade-off of shipping code to many applications, which do not need it at all. Not every application deals with dates. And even less applications use Int8Array
and alike.
But there is another even more relevant trade-off: Developers may want to build tracked versions of objects, which are provided by third-party libraries as well. E.g. a team may want to have a tracked version of Moment
. We can not know upfront, for which objects a team may want to build a tracked version. We should design an API, which provides a good developer experience if using tracked versions of objects provided by user-land implementations in parallel.
I want to add another perspective on the question of tracked(new Map()) vs new TrackedMap(): Extendability.
I think this is an excellent point.
The most obvious solution, and the one I think I would favor, is to allow a constructor to register itself as a tracked builtin.
import { registerBuiltin } from "@glimmer/tracking";
class TrackedDate implements Date {
// ...
}
registerBuiltin(Date, () => new TrackedDate());
const date = tracked(Date);
This is helpful because it makes it obvious both to the user and to the the implementor that what they're doing is constructing something that has the same interface as Date
.
That said, I can see that this pattern might have implications for tree shaking that might make it worth considering an alternative. I'm very open to discussion on this.
When thinking through this, I think it's important to remember that the entire space of tracked builtins is limited to the list of JavaScript builtins, and unless an implementation got very elaborate, we'd probably eventually want to pull in any well-implemented version of something like TrackedDate
.
When thinking through this, I think it's important to remember that the entire space of tracked builtins is limited to the list of JavaScript builtins
Why that? I think it is foreseeable that we want tracked versions of some objects provided by third-party libraries.
Let's stay with the Moment.js example I have used before. Do you see better solved by importing from a wrapper library anyways? Something like:
import moment from 'tracked-moment';
const date = moment();
@jelhan I take your point.
My rough intuition, which I'm very open to changing, is that it makes sense to have a single entry point for the tracked builtins that map directly onto stuff in the ECMA-262 spec. This is both to make it super-clear that the intent is for the APIs to be 1:1 with the spec, as well as to establish core team ownership over the stuff that has a clear spec for quality control reasons.
Your pushback, aiui, is:
- There are going to be third-party implementations of ECMA-262 features (like tracked typed array) for the forseeable future.
- Other stuff, like
moment
, isn't so different in the mind of an average developer.
And therefore: having a handful of tracked builtins go through a single entry point, while seemingly similar APIs come through addons is a recipe for confusion.
Is that about right?
If so, what do you think is the best way to communicate the fact that the core team owns a large subset of the 262 tracked builtins, and that it is considered a fairly urgent bug if there is a difference in behavior from the spec?
As an aside, we might want to try to run some subset of the ECMAScript tests (specifically, the ones that deal with instance behavior). This wouldn't necessarily communicate the point to a broad audience, but I bet it would catch bugs and help us avoid introducing bugs!
sorry for the delay here
Small comment to cross reference with the
ember-modifier
RFC, as I think the same constraints apply here and that this RFC will have to do whatever we do there.
added the requested tweak in commit https://github.com/emberjs/rfcs/pull/812/commits/ca8525d8046c36f599274f2f18f819f08294c404
* to be explicit that we will be decorating only the 6 types most commonly used in real-world JS today (`Array`, `Object`, `(Weak)?(Set|Map)`, but that we are open to adding other built-in data structures in the future _if and as usage demonstrates the necessity thereof_ * to specify _a_ design for the constructors, including the relationship to the original object passed in—i.e. that we do a shallow clone on the object, because an API where you mutate the original has _extremely_ surprising results * to explicitly state that we are _not_ adding any deep tracking in this case, because: * the correct semantics for a framework-level hook there aren't obvious * it is possible to build variants on it fairly cheaply in user-land given `tracked()` as updated here
added the requested tweaks in commit https://github.com/emberjs/rfcs/pull/812/commits/401a58bfc052aa5901c3f731c51c33663cd2e6be
@chriskrycho @rwjblue please let me know if this does not match exactly what you were thinking/proposing.
We've moved this to FCP, intending to merge; @wycats is adding some notes on what we need to do to get it to Released and esp. Recommended.
We need to add a "before-stable" requirement (which doesn't block the approved stage):
- before advancing this change to "ready to release",
@glimmer/tracking
and@glimmer/validator
need to be implemented as normal JS packages instead of being hardcoded in the ember-source build.