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

Build failing with ember beta & canary #6961

Closed
igorT opened this issue Jan 12, 2020 · 3 comments
Closed

Build failing with ember beta & canary #6961

igorT opened this issue Jan 12, 2020 · 3 comments

Comments

@igorT
Copy link
Member

igorT commented Jan 12, 2020

https://github.com/emberjs/data/runs/384084391

@pzuraq is poking at it

@runspired runspired changed the title Build failing with ember canary Build failing with ember beta+canary Jan 21, 2020
@runspired runspired changed the title Build failing with ember beta+canary Build failing with ember beta & canary Jan 21, 2020
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.
@hjdivad
Copy link
Member

hjdivad commented Jan 22, 2020

Not a release blocker but nice to get in before we cut a release (est. this evening)

@ghost
Copy link

ghost commented Feb 7, 2020

@igorT I believe #7030 closed this.

@igorT
Copy link
Member Author

igorT commented Feb 7, 2020

@efx thank you!

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

No branches or pull requests

2 participants