[v2] Use @tracked underneath on Ember 3.16+ #354
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
maxfierke
changed the title
maxfierke
force-pushed
the
mf-v2_tracked_interop
branch
from
9297a4e
to
f370da5
Compare
No more `@computed` required to read derived state from Tasks and TaskInstances.
setProperties will consume the tag on initial reads (to read the initializer value) which causes the following set to fail, even if the property hasn't _really_ been read yet (by anything user-side.) Instead, we can avoid altogether and just use native Object.assign, which won't read & consume the property before setting.
maxfierke
force-pushed
the
mf-v2_tracked_interop
branch
from
f370da5
to
3e457b9
Compare
maxfierke
added
ember-octane
Case Number 483092 of forgetting about task groups
This'll put it on the microtask queue on modern Ember, which is what we expect
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a PR to use
@trackedforTaskandTaskInstancestate on Ember 3.16+ to enable use without using@computedwhen consuming derived state.In Ember < 3.16, e-c behaves as it currently does requiring
@computedto track task-derived state changes.This PR essentially replaces the state properties on the Ember implementations of
TaskandTaskInstancewith those properties decorated with@tracked(done dynamically, though w/ property descriptors. not redefining them with actual decorator syntax, so there's no dependency on decorator transforms.) This incurs a little bit of a cost, since it's redefining the properties, but practically shouldn't have much effect since it's applied to theTaskandTaskInstanceprototypes, rather than every instance, so it would only be incurred once.There's a few outstanding issues/open questions with this:TheFixedno-render-breakingtests are triggering re-render assertions (one of them might technically be a "correct" failure.)Seems thatsetPropertiesreads the properties it sets, consuming the tag and leading to the error when setting@trackedproperties. I think this can be worked around by usingObject.assigninstead.performCountneeds to be read to be updated. Seems this is fine in some contexts, but not others? (i.e. not in the case in one of the tests)_performCountand used for reads to ensure write-only to trackedperformCountHow does this work for add-ons that depend on ember-concurrency and support multiple Ember versions?@computedif they consume ember-concurrency state and support < 3.16. However,@computedJust Works™ with the tracked state, it seems.Within applications with both tracked & computed properties, if using a native getter to access task state, and wishing to use it alongside a computed property,
@dependantKeyCompatwill need to be used on the getter as expected with any other tracked-prop using getter.I'd expect there are probably other edge-cases with this as well.
Resolves #343