From 4802e40419da25214b301ab129896bdde2f5254c Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Sun, 26 Jul 2020 14:21:43 -0400 Subject: [PATCH 1/2] Add failing tests for `assertDestroyablesDestroyed` when using `isDestroying`/`isDestroyed` --- .../runtime/test/destroyables-test.ts | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/packages/@glimmer/runtime/test/destroyables-test.ts b/packages/@glimmer/runtime/test/destroyables-test.ts index 10fd45e0c5..4d6da5c374 100644 --- a/packages/@glimmer/runtime/test/destroyables-test.ts +++ b/packages/@glimmer/runtime/test/destroyables-test.ts @@ -438,6 +438,28 @@ module('Destroyables', hooks => { assertDestroyablesDestroyed!(); }); + test('checking isDestroying does not trigger assertion', assert => { + assert.expect(1); + enableDestroyableTracking!(); + + let obj = {}; + + isDestroying(obj); + + assertDestroyablesDestroyed!(); + }); + + test('checking isDestroyed does not trigger assertion', assert => { + assert.expect(1); + enableDestroyableTracking!(); + + let obj = {}; + + isDestroyed(obj); + + assertDestroyablesDestroyed!(); + }); + test('attempting to call assertDestroyablesDestroyed() before calling enableDestroyableTracking() throws', assert => { assert.throws(() => { assertDestroyablesDestroyed!(); From 0f1f12e209a075c41730f84c2b9dc20eb72771a4 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Sun, 26 Jul 2020 14:36:50 -0400 Subject: [PATCH 2/2] Ensure `isDestroying` / `isDestroyed` checks do not trigger assertion. When checking `isDestroying`/`isDestroyed` there is no need to ensure that `DESTROYABLE_META` is initialized (it is totally possible to check `isDestroying` on an object that never has children or destructors). --- packages/@glimmer/runtime/lib/destroyables.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/@glimmer/runtime/lib/destroyables.ts b/packages/@glimmer/runtime/lib/destroyables.ts index e54027ea5a..0c13e78f8e 100644 --- a/packages/@glimmer/runtime/lib/destroyables.ts +++ b/packages/@glimmer/runtime/lib/destroyables.ts @@ -212,11 +212,15 @@ export function setScheduleDestroyed(fn: (fn: () => void) => void) { } export function isDestroying(destroyable: Destroyable) { - return getDestroyableMeta(destroyable).state >= DestroyingState.Destroying; + let meta = DESTROYABLE_META.get(destroyable); + + return meta === undefined ? false : meta.state >= DestroyingState.Destroying; } export function isDestroyed(destroyable: Destroyable) { - return getDestroyableMeta(destroyable).state === DestroyingState.Destroyed; + let meta = DESTROYABLE_META.get(destroyable); + + return meta === undefined ? false : meta.state >= DestroyingState.Destroyed; } ////////////