glimmer-vm icon indicating copy to clipboard operation
glimmer-vm copied to clipboard

[BUGFIX] ensure untracked `@tracked` initializers

Open chancancode opened this issue 5 years ago • 1 comments

Given...

class Thing {
  @tracked foo = `foo ${this.bar}`;
  @tracked bar = 'bar';
}

When this is rendered...

{{this.foo}}

The curly will invalidates when this.bar invalidates, if it happens to be the first thing that accesses this.foo on the object.

This is incorrect: initializers only run once, and whatever code were run inside should never cause the tracked property to become invalidated.

chancancode avatar Jan 29 '20 01:01 chancancode

I'm unsure about this direction for a few reasons.

First, it's inconsistent, in that it would be the first instance of using untrack for a non-legacy API. Up until now, any property access has always been tracked, and we haven't shied away from that.

Consider:

class LocalTranslatorUntracked {
  constructor(owner) {
    setOwner(this);

    if (this.locale === 'en') {
      // do something
    }
  }

  @service i18n;
  @tracked locale = this.i18n.locale;
}

class LocalTranslatorTracked {
  @service i18n;
  @tracked locale;

  constructor(owner) {
    setOwner(this, owner);
    
    this.locale = this.i18n.locale;

    if (this.locale === 'en') {
      // do something
    }
  }
}

class MyComponent extends Component {
  get untrackedTranslator() {
    return new LocalTranslatorUntracked(getOwner(this));
  }

  get trackedTranslator() {
    return new LocalTranslatorUntracked(getOwner(this));
  }
}

The fact that these two would behave differently seems bad, especially if we ever intend to try to have tracked properties behave like normal initializers.

More broadly, I think it violates the theory behind autotracking a bit. If you were to create and use a class during the course of a computation, you are still consuming the value, and things can still get out of sync if you, say, mutate that value later on:

class Foo {
  @tracked bar = 123;
  @tracked baz = this.bar;
}

let compute = memoComputation(() => {
  let foo = new Foo();

  console.log(foo.baz); // 123
  foo.bar = 456; // foo.baz is now out of sync with foo.bar
});

Initializers don't run more than once, this is true, but I still think this should throw the autotracking transaction error because it alerts the user to what is likely an antipattern - their computation is no longer guaranteed to be correct.

The second reason is more practical. We may not be able to untrack initialization in the future, as the decorators spec evolves, much like how we aren't able to untrack foo.trackedProp++. I'm nervous about adding an arbitrary restriction to tracking if we aren't sure we'll be able to enforce it in the future.

pzuraq avatar Jan 29 '20 18:01 pzuraq