BUG: attempted to update a tag with a tag that has a more recent revision as its value #18678
Comments
|
I ran into this issue in a complicated project, and created this rudimentary reproduction in the single solitary unit test. |
pzuraq
pushed a commit
to glimmerjs/glimmer-vm
that referenced
this issue
Jan 22, 2020
Currently, if an UpdatableTag is updated with a tag that has a _more recent_ revision value, Glimmer throws an error. This is because the caching strategy that tags use does not have a way to bust caches on updates, without dirtying. However, in real world usage we've encountered a few cases where updating a tag with a more recent version _is_ valid. The use cases we've seen so far are: 1. Updating a ComputedProperty's tag with _lazy dependencies_ which were not used in the original computation: emberjs/data#6961 2. Consuming an argument for the first time asynchronously, after its tag has already be read by the VM: https://github.com/markcharyk/tag-with-a-tag-repro 3. Calculating the value for a ComputedProperty for the first time asynchronously, after its tag has been read by _observers_: emberjs/ember.js#18678 These cases have one thing in common - the tag updates aren't meant to signal a change in state, but to a value that has already been calculated to another value that has been calculated lazily, such that when the later value updates, the former will also update. I believe currently that this is applicable to all updates, since this is what updating a tag is fundamentally about. Either you're creating a tag tree before the fact (and don't need to worry about cache busting) or you're stitching it together after the fact, but you don't want to affect the outcome either way. \### What's in this PR This PR adds a mechanism that enables this laziness, without causing possible accidental cache busts. The issue it solves is this: ```js let tagA = createTag(); let tagB = createTag(); let snapshotA = value(tagA); dirty(tagB); // tagA.revision === 1, tagB.revision === 2 ``` Let's say we read the value of `tagA`. Now it is cached, and if we were to update it to point to `tagB`, it would still return the cached value: ```js validate(tagA, snaphotA); // true update(tagA, tagB); validate(tagA, snaphotA); // true, cache was not busted ``` Now, we create and dirty a third tag that is unrelated, `tagC`. This busts the cache for all tags, and the next time we attempt to validate `tagA` against a previous snapshot, it will return _false_, since it will now return the value of `tagB`: ```js let tagC = createTag(); dirty(tagC); validate(tagA, snaphotA); // false, even though tagA and tagB haven't changed! ``` To prevent this, this PR adds a cache of the value of `tagB` that is checked instead. IFF `tagB`'s value has changed, it will propogate that change, otherwise it will return the previous value, so any existing snapshots will remain valid.
|
I'm seeing this is in my own complicated project as well. I will attempt to reproduce if time permits. |
|
I tried resolving @glimmer/validator to 0.47.0 and it did not resolve my issue. |
|
The assertion shouldn't exist anymore in 0.47.0. Ember builds the glimmer packages prior to publish I believe, so it may be that simply resolving it won't actually update the code that is run. |
|
#18694 updates to include the fix linked above, and is cherry-picked into the beta branch. Once CI finishes publishing (approximately 20-30 min), new beta and canary test runs should avoid this issue. |
|
Fixed in #18694 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
In discussion with @pzuraq he explained the following error is an internal failure in Ember, and we should report occurrences of it from addon builds:
I see this in ember-table in the following build: https://travis-ci.org/Addepar/ember-table/jobs/636447971 The first test to fail is
Unit | Private | CollapseTree: can disable tree.The text was updated successfully, but these errors were encountered: