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

Ensure @tracked assertion can be made a deprecation. #1205

Merged
merged 1 commit into from
Nov 20, 2020

Conversation

rwjblue
Copy link
Member

@rwjblue rwjblue commented Nov 19, 2020

During recent refactors that landed in glimmer-vm@0.57 (Glimmer VM version that landed in Ember 3.22) we moved the internal infrastructure to ensure everything was in a tracking frame. This greatly increased the number of locations that would now fall under the standard read + write assertions.

For example, in glimmer-vm@0.56 the following would run without error:

import Component from '@glimmer/component';
import { tracked } from '@glimmer/tracking';

export default class extends Component {
  @tracked counter = 0;

  constructor() {
    super(...arguments);

    this.counter++;
  }
}

Now, obviously this is a bit contrived, but the point is that since component construction was not previously within a tracking frame it would not error due to the read then write of a tracked property.

As of glimmer-vm@0.60+ we do run this within a tracking frame (because nearly everything is automatically within a tracking frame).

The infrastructure being updated here (forceHardError argument to assertTagNotConsumed) was intended to ensure that it was not possible to end up with a @tracked property that was set after consumption during a tracking frame, but to still allow for things using Ember's markObjectsAsDirty (via notifyPropertyChange) to trigger a deprecation. Unfortunately, now that we have increased the areas of the codebase that are within tracking frames we can no longer use forceHardError internally if we want our host environments to satisfy their SemVer requirements (shouldn't be adding new failures for things that worked "fine" in a minor release of Ember).

This change in and of itself doesn't really do anything (all of the same things would still error) however it does allow host environments to leverage deprecateMutationsInTrackingTransaction as appropriate to issue a warning instead of a failure for cases that previously worked.

Related to emberjs/ember.js#19192
PR in Ember to take advantage: emberjs/ember.js#19282

@rwjblue rwjblue force-pushed the allow-deprecations-for-trackedData-setters branch from 63746d2 to c3fc026 Compare November 19, 2020 21:23
During recent refactors that landed in glimmer-vm@0.57 (Glimmer VM
version that landed in Ember 3.22) we moved the internal infrastructure
to ensure **everything** was in a tracking frame. This greatly increased
the number of locations that would now fall under the standard read +
write assertions.

For example, in glimmer-vm@0.56 the following would run without error:

```js
import Component from '@glimmer/component';
import { tracked } from '@glimmer/tracking';

export default class extends Component {
  @Tracked counter = 0;

  constructor() {
    super(...arguments);

    this.counter++;
  }
}
```

Now, obviously this is a bit contrived, but the point is that since
component construction was not previously within a tracking frame it
would **not** error due to the read then write of a tracked property.

As of glimmer-vm@0.60+ we **do** run this within a tracking frame
(becauase nearly everything is automatically within a tracking frame).

The infrastructure being updated here (`forceHardError` argument to
`assertTagNotConsumed`) was intended to ensure that it was not possible
to end up with a `@tracked` property that was set after consumption
during a tracking frame, but to still allow for things using Ember's
`markObjectsAsDirty` (via `notifyPropertyChange`) to trigger a
deprecation. Unfortunately, now that we have _increased_ the areas of
the codebase that are within tracking frames we can no longer use
`forceHardError` internally if we want our host environments to satisfy
their SemVer requirements (shouldn't be adding new failures for things
that worked "fine" in a minor release of Ember).

This change in and of itself doesn't _really_ do anything (all of the
same things would still error) however it does allow host environments
to leverage `deprecateMutationsInTrackingTransaction` as appropriate to
issue a warning instead of a failure for cases that previously worked.
@rwjblue rwjblue force-pushed the allow-deprecations-for-trackedData-setters branch from c3fc026 to e740429 Compare November 19, 2020 22:56
@rwjblue rwjblue merged commit 1d1b3e3 into master Nov 20, 2020
@rwjblue rwjblue deleted the allow-deprecations-for-trackedData-setters branch November 20, 2020 02:40
rwjblue added a commit to emberjs/ember.js that referenced this pull request Nov 20, 2020
Includes the following bugfixes:

* [#1209](glimmerjs/glimmer-vm#1209) Ensure `<output form="some-value">` works properly
* [#1205](glimmerjs/glimmer-vm#1205) Ensure `@tracked` assertion can be made a deprecation
* [#1204](glimmerjs/glimmer-vm#1204) Ensure `loc` is populated by `build ers.element(...)`

Changelog here:

https://github.com/glimmerjs/glimmer-vm/releases/tag/v0.66.1
rwjblue added a commit to emberjs/ember.js that referenced this pull request Nov 23, 2020
Includes the following bugfixes:

* [#1209](glimmerjs/glimmer-vm#1209) Ensure
  `<output form="some-value">` works properly
* [#1205](glimmerjs/glimmer-vm#1205) Ensure
  `@tracked` assertion can be made a deprecation

Changelog here:

https://github.com/glimmerjs/glimmer-vm/releases/tag/v0.65.1
rwjblue added a commit to emberjs/ember.js that referenced this pull request Nov 23, 2020
Includes the following bugfixes:

* [#1209](glimmerjs/glimmer-vm#1209) Ensure `<output form="some-value">` works properly
* [#1205](glimmerjs/glimmer-vm#1205) Ensure `@tracked` assertion can be made a deprecation

Changelog here:

https://github.com/glimmerjs/glimmer-vm/releases/tag/v0.62.5
sly7-7 pushed a commit to sly7-7/ember.js that referenced this pull request Mar 10, 2021
Includes the following bugfixes:

* [emberjs#1209](glimmerjs/glimmer-vm#1209) Ensure `<output form="some-value">` works properly
* [emberjs#1205](glimmerjs/glimmer-vm#1205) Ensure `@tracked` assertion can be made a deprecation
* [emberjs#1204](glimmerjs/glimmer-vm#1204) Ensure `loc` is populated by `build ers.element(...)`

Changelog here:

https://github.com/glimmerjs/glimmer-vm/releases/tag/v0.66.1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants