Skip to content
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

[QUEST] Scope Down Mandatory Setter #18769

Closed
pzuraq opened this issue Feb 27, 2020 · 3 comments
Closed

[QUEST] Scope Down Mandatory Setter #18769

pzuraq opened this issue Feb 27, 2020 · 3 comments

Comments

@pzuraq
Copy link
Contributor

pzuraq commented Feb 27, 2020

The mandatory setter is a long standing, DEBUG only error message that Ember adds to try to let users know when they need to use Ember.set or @tracked. Originally, it only warned users about using Ember.set, but the refactors done to support autotracking fundamentally changed how property updates worked and the old mandatory setter code no longer made sense as it was, so we updated it.

Since then a number of issues have popped up with the new implementation. After some debate, it has become clear that the mandatory setter is no longer particularly helpful in a number of cases. This quest issue is to track scoping down the mandatory setter in these cases.

Motivation

There are a few main issues that were raised with the mandatory setter since the autotracking refactors:

  1. It can no longer be automatically removed. In the old world, observers/chain watchers were what added the setter in the first place, and since these constructs had a teardown step, we could remove the setter once they were torn down. This is no longer the case with autotracking, and this has resulted in a number of issues, including difficulties for ember-mocha.

  2. It interferes with external libraries. There was a lot of excitement around using Immer.js with Ember's new autotracking model, until users realized that the mandatory setter and Immer would collide. Immer will throw errors whenever it encounters a getter, which is what mandatory setter installs. There are workarounds, but they are not very ergonomic, and are definitely a pain point.

  3. Perhaps the biggest issue, the mandatory setter could no longer be applied uniformly. For instance, in the following example, using the bar in two different ways will result in two different behaviors:

    class Foo extends Component {
      // {{this.bar}} adds mandatory setter to bar
      bar;
    
      // {{this.baz}} does not add mandatory setter to bar
      get baz() {
        return this.bar;
      }
    }

    This is because dependencies aren't listed in autotracking, so there's no way for us to know that baz will (or did) access bar. This is a much worse DX, because users cannot know when Ember will or won't add the mandatory setter to their code with confidence, and as such cannot rely on it.

The mandatory setter was a product of its time, when Ember users had to always vigilantly use set everywhere to make sure that their app's state updated correctly. This is no longer the world we live in today, and as such it makes sense to scope the mandatory setter back down to classic APIs, and eventually remove it from the programming model.

Detailed Design

The mandatory setter should apply exclusively to classic APIs, because they are the ones that can be covered comprehensively. These include:

  • Dependencies of computed properties and computed property macros, and
  • Properties that are watched by observers

These APIs currently use the chain tags APIs internally to gather the tags of their dependencies, so we can use these functions to setup the mandatory setter.

How Do We Teach This

We should double up our efforts to make sure that users understand what the rules of autotracking are in our documentation, including examples of common cases.

One possibility, would be to include an example early on in the guides where we setup a template, and try to update things, but it doesn't work - so we need to add @tracked.

Overall, the main point is that we need to make the connection that the first thing to check when your template's aren't updating is if everything is properly @tracked.

@chriskrycho
Copy link
Contributor

I do not yet have the relevant expertise here to do this myself, but I volunteer as tribute! nonetheless: this seems like a great spot to learn some of these pieces of the framework, and I’d love to help out as I’m able!

pzuraq pushed a commit that referenced this issue May 9, 2020
Scopes down the mandatory setter, like it was described in the
[quest issue](#18769). It
should only be applied when a value is being used as a dependency in a
computed property or an observer.
pzuraq pushed a commit that referenced this issue May 9, 2020
Scopes down the mandatory setter, like it was described in the
[quest issue](#18769). It
should only be applied when a value is being used as a dependency in a
computed property or an observer.
@rwjblue
Copy link
Member

rwjblue commented May 23, 2020

@pzuraq - Would you mind confirming that this is fully completed now?

@knownasilya
Copy link
Contributor

knownasilya commented Aug 15, 2020

Was this released?

Looks like in 3.20 atleast

@rwjblue rwjblue closed this as completed Oct 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants