Tracked Storage Primitives

[pzuraq]

Rendered

[buschtoens]

I love this! 👏

[buschtoens]

Suggested change

	sometimes they aren't the right tool for the job. For instance, you may need
	have a problem that requires you to work with a number of dynamically created
	sometimes they aren't the right tool for the job. For instance, you may
	have a problem that requires you to work with a number of dynamically created

[NullVoxPopuli]

Yaaaaas : 💯

[btecu]

I have a use case for this (dynamic tracked values), so I'm looking forward having it available.

[mixonic]

Can an example of initialValue be added? Besides being included in the typed signature, I don't see any mention or example use included in the RFC.

[pzuraq]

@mixonic it is mentioned in the API description/docs, and it is used in the re-implementing tracked section and the final example of the Creating custom tracked data structures section

[dgeb]

I'm in favor of introducing these primitives and like this design in general, but would like to reconsider one aspect of the design to both be more consistent with the higher-level @tracked and better represent the underlying tracking primitives.

Specifically, I propose that we change the name and default value of the parameter isEqual?: (oldValue: T, newValue: T) => boolean. Changing this function's name to isStable (or perhaps an inverse such as invalidates) better represents the underlying tracking primitives. And rather than returning the result of a strict equality check, returning false by default would keep the functionality consistent with @tracked, and thus be less surprising while still being easily overridden.

[pzuraq]

@dgeb I agree with the isStable framing and have updated the RFC. As for the default, I actually believe that the original behavior we chose for @tracked was in the end not ideal. This behavior was chosen specifically to enable the this.prop = this.prop pattern, with the idea being that this was how we would handle updating data structures in general.

This RFC is essentially about rolling back that pattern in favor of creating tracked data structures instead. As noted in the RFC motivation and the changes to the guides, the old resetting pattern had a number of problems and issues, and using tracked data structures prevents these issues from occurring in general. With this in mind, the fact that resetting a tracked property triggers a notification actually enables antipatterns, so ideally we would change the behavior at some point in the future.

That said, this behavior has proliferated already, so we can't change it without some sort of deprecation and roll-out plan, and possibly an optional feature. However, we can change the default behavior of new tracked features, such as this one, which is why I made the default stable under === equality.

[dgeb]

I agree with the isStable framing and have updated the RFC.

@pzuraq Thank you - the name change was my primary concern. I can accept and understand using a different default check at the primitive level.

As for the default, I actually believe that the original behavior we chose for @Tracked was in the end not ideal. This behavior was chosen specifically to enable the this.prop = this.prop pattern, with the idea being that this was how we would handle updating data structures in general.

Certainly, the behavior chosen for @tracked was pragmatic when it was introduced, and pretty much required to make the programming model feasible. I don't think this cat is going back in the bag, which seems fine to me. It seems preferable to introduce a new decorator that can live alongside @tracked that is based on strict equality. And of course, whatever else everyone dreams up with the new primitives :)

[tomdale]

s/hgetting/getting/

[rwjblue]

I'm a bit concerned that the usage of Storage conflicts with the web storage API, obviously in our applications you'd be importing it but if you forgot to import it you would be fairly annoyingly trolled (it would not lint against it, it would seem to "be a function", but it just isn't what you think it is)...

[boris-petrov]

I would like to explain my experience with migrating our core model layer to use auto-tracking instead of computed properties. I got a lot of help from @pzuraq so big thanks to him!

We have a single getter and setter pair which we use for all properties in our models. They dynamically dispatch to the correct place based on the property at hand. There are two specific use-cases that we had trouble with.

1. The getter has to set a default value in the model's data when a property is accessed and the value is empty. So that means doing something like if (this.data[property] == null) this.data[property] = getDefaultValueBasedOnProperty(property);. But this is doing a read before a write which leads to an assertion.

2. The setter checks whether the set value is the same as the current one (in order to set a dirty flag if needed). This means, again, doing a read before a write.

We've currently worked around these two problems by maintaining an untracked copy of the data. That, however, lead to a bunch of pretty nasty code in some places and is plain impossible in others: imagine that we pass a deeply nested value from the tracked data through a bunch of components to the last one which modifies it in a two-way bound input. To somehow keep that up-to-date with the untracked copy I would have to write tons and tons of code with proxies and whatnot... just not worth it.

All of that leads me to say that if we had a peek primitive, that would save us a lot of pain, code, performance and memory.

[scottmessinger]

I second @boris-petrov suggestion to add peek. Especially as these are explicitly primitives, I think adding this low level escape value will help solve problems like Boris's that are hard to solve otherwise. Ember has long had the rationale that it doesn't want to provide APIs that let users shoot themselves in the foot (the rationale @pzuraq mentioned against adding peek). This is mostly a great thing, but I've run into cases like @boris-petrov where the framework has limited me because of a fear devs would use a feature wrong (based on my understanding of the rationale from conversations with core members). There's nothing quite like feeling trapped and limited by a framework you otherwise love and being unable to deliver a feature in the way you need. So, while I haven't run into the problem Boris has (yet), I've run into problems like it with Ember and have come to feel strongly that Ember should be biased towards exposing functions that solve problems that are difficult/impossible to solve otherwise.

That all said, I am very much in agreement that Ember should lead devs on the happy path and I think @pzuraq's concerns shouldn't be ignored! Based on the rationale in this RFC, it sounds like the only concern with adding peek is that misuse could cause bugs/confusion. If that's the case, one possibility is to changing the name of the function to cause devs to give it a second look. E.g.: _peek, __peek__, __peekIfYouMust__, _dangerouslyPeekValue, _peekWithoutEntangling, or _carefullyPeekValue or some unconventional name. Personally, I prefer _peekWithoutEntangling as it clarifies how peek is different from simply reading the value.

TL;DR. I think Boris's examples feel very normal and thus strongly second providing peek for exactly the type of scenarios he lays out. In addition, I think addressing @pzuraq's concerns about peek tripping devs up shouldn't be ignored and could possibly addressed through changing peek to an unconventional name that causes devs to give a second thought to using it.

[pzuraq]

@scottmessinger @boris-petrov I am not the only person on the core team with these concerns, and I would say that we are generally aligned in this perspective. It's going to take significant evidence of crucial use cases that are common and widespread to change this view.

The types of failures we expect to arise if we were to allow users to easily peek tracked state are fundamental. I would put them in the same category as memory safety issues in languages where you have to manually manage memory. Currently, the system works as hard as possible to work consistently with derived state, like a memory safe language such as Rust does with memory. What you're asking for is analogous to wanting a way to write unsafe Rust, as such. This is a capability that Rust only has because it is targeted at being a systems level programming language, which means there are cases where it is unavoidable.

This works in the Rust community because:

1. unsafe is really uncommon
2. The community as a whole works to keep it that way

I think that if every Rust library were to start writing unsafe everywhere, it would not be good. In the same way, if every Ember addon and app were to start using peek, it would not be good. It is actually a critical part of the system that state should never be derived without being tracked.

It's an even worse sign that the common first question when someone runs into the backtracking assertion is "how do I peek?". If we make it available, even in a way that suggests it is dangerous like @scottmessinger suggests, I think it would likely be abused immediately because it does seem like a simple, easy thing to do, and the bugs it could cause are not readily apparent. This is why I think the status quo, where someone who wants to have this capability has to do a large workaround like @boris-petrov laid out, is actually ideal.

FWIW, we do not peek values even inside of Ember/Glimmer. We follow our own rules here! So I think this is very possible to do in general.

So, having said all that, I've discussed this in detail with @boris-petrov offline, and if I remember correctly there are two use cases that he has:

1. Not dirtying tags unless the value actually has dirtied
2. Initializing state if a value is not present (e.g. setting a tracked property after reading it to see if its undefined)

For the first issue, this is solved by this RFC. For the second one, I think the key thing to focus on is the fact that this is about initializing state. During initialization, you are creating a tracked value for the first time - you shouldn't actually have to read it. I actually think the ideal solution here would be to manually create and manage tracked storage instances within this data layer. The first time value is accessed, you create the tracked storage, and initialize it to the default value. This would likely be more efficient overall than using @tracked for this purpose.

[chriskrycho]

For what it’s worth, as the Octane migration lead for LinkedIn.com, I see these kinds of issues all the time as folks migrate from Ember Classic patterns—and precisely because of that experience, I strongly agree with @pzuraq and others on the framework team. Accordingly, and without digging into this specific example, I’d like to offer a general comment here in hopes that it’ll be useful more generally for the community thinking around migrating to Octane.

Fundamentally, Octane entails a new programming model—one that is not a direct translation of the old model into some new syntax. That means that some patterns that you’re used to using in the Classic paradigm simply do not work in the new paradigm (though not very many). However, in literally every single one of those I’ve hit over the last year, I’ve ultimately been very happy to end up doing the work of substantially rethinking and reworking whatever abstraction or implementation was at hand.

In practice, that has often meant:

1. Take a big step back and consciously choose to set aside the existing implementation’s constraints as inputs.
2. Write down the actual constraints on the design: the problem it needs to solve,
3. Create a new Octane design that solves the problem.
4. If necessary, because the old design was fundamentally coupled to Ember Classic idioms, design a migration path to the new API.

(4) has been rare, but has occasionally happened. For example, over the summer we took a system that made heavy use of Evented and reworked it into a new approach that funneled each event into a single piece of tracked data, and let the rest of the system derive from that single source of truth. This isn’t something that we could do under the existing API, because the existing API was just everyone using the events willy-nilly. (The famous reason React exists—keeping message state in sync—hit our app in a lot of places as a result! And now… it doesn’t.) Migrating all consumers to the new design was and is a slow process, but everywhere we do it, we’re really happy we did.

The single most common example of this kind of rewrite for us is figuring out how to replace uses of didReceiveAttrs. The easy, “translation”-style path is to grab {{did-insert}} and {{did-update}} and keep the implementation otherwise the same. And for the short term, this is sometimes reasonable. However, in every case we’ve hit, there was also a much better solution that involved rewriting in terms of derived state, using a custom modifier for managing DOM interactions, or a combination of the two. And when we make that investment, the result has consistently required less code that was easier to understand and test to solve the same problem than the original code or even a “translation” from Classic to Octane.

This process is both harder and slower than just trying to translate directly from Ember Classic into Ember Octane syntactically, to be sure! It has also paid off handsomely, though.

All of this to say: it’s very common to find your existing APIs and solutions to a problem aren’t Octane-friendly. One response is to say this is a problem with Octane. Depending on your views on software design, that might be fair! Another, though, is to say that it’s possible the abstraction would benefit from being reworked substantially. This is my POV, and again: every single place we’ve hit like this in our app has been improved by the rewrite—often massively so.

That doesn’t mean Octane is perfect. For one, there are still gaps: this RFC addresses one, and the work to get resources and effects addresses another. For another, observable-based systems (like Ember Classic) and incremental computation systems (like Ember Octane) simply have different tradeoffs and affordances—and you might prefer the former! But the fact that a pattern that worked in Classic doesn’t work in Octane doesn’t mean that Octane is wrong or even that it is missing something. It does mean that it makes fundamentally different tradeoffs in the design space than Classic did.

Me, I prefer the Octane flavor of the tradeoffs, and when you hit spots like this I encourage you to rethink the design. You may find yourself in a place that surprises you with how much cleaner and more maintainable it is! You may also occasionally find a spot where the tradeoffs are a little worse (hasn’t happened to me yet, but I’m sure it’s possible). But either way, the fundamental design is the way it is—and breaking the core abstraction would be much worse.

---

Edit, 2020/12/07: I turned this into a blog post.

[rwjblue]

We discussed this at todays core team meeting, and are in favor of moving this into final comment period.

[rwjblue]

Discussed today, and we are ready to merge.

@pzuraq - Mind fixing up the frontmatter linting issue and then merging?

[asakusuma]

the target argument is not used in the function, is this intended?