From c3c62841f23120798d3d70e18547d9559a7f5e0e Mon Sep 17 00:00:00 2001 From: Krystan HuffMenne Date: Wed, 16 Jan 2019 16:01:07 -0800 Subject: [PATCH] Don't remove dep keys in `didUnwatch` This fixes #17243 by only removing dependent keys in `teardown`. Since #16978 coalesces the work to add dependent keys to happen at most once, it is now incorrect to eagerly remove them in `didUnwatch` because that could happen for a variety of reasons while there are still other interested parties (see attached test cases). This limits the work of removing dependent keys to `teardown` only. The logic is that, while we want to defer setting up the "link" to the target property as lazily as possible, it is less important to "unlink" it eagerly once the work is done. It _may_ be possible to accomplish this, but the current amount of book-keeping is not sufficient to track the count properly. In our tests, we have created sequences like this: 1. setup (peekWatching = 0, so nothing happens) 2. get (consumed here) 3. willWatch (already consumed, no-op) 4. get (already consumed, no-op) 5. didUnwatch 6. ... In this case, it would be incorrect to "unlink" at step 5. As of PR #16978, `CONSUMED` is essentially a boolean flag, so it is not sufficient to track the balance, and also, there is no counterpart to `get`, which makes eager "unlinking" impossible without much more book-keeping. It's unclear that it would be worthwhile to do that. On the other hand, if we only "unlink" on teardown, this ensures that we are only "unlinking" at most once, and it is guaranteed that there are no longer any interested parties. Fixes #17243 --- packages/@ember/-internals/metal/lib/alias.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/@ember/-internals/metal/lib/alias.ts b/packages/@ember/-internals/metal/lib/alias.ts index 38471c23882..b8f3b6cdb1e 100644 --- a/packages/@ember/-internals/metal/lib/alias.ts +++ b/packages/@ember/-internals/metal/lib/alias.ts @@ -46,10 +46,6 @@ export class AliasedProperty extends Descriptor implements DescriptorWithDepende this.consume(obj, keyName, meta); } - didUnwatch(obj: object, keyName: string, meta: Meta): void { - this.unconsume(obj, keyName, meta); - } - get(obj: object, keyName: string): any { let ret = get(obj, this.altKey); this.consume(obj, keyName, metaFor(obj));