From 8436ae56e7c1abaf61bc4c0af473241f97d0e14e Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Sun, 26 Jul 2020 09:50:41 -0400 Subject: [PATCH] Refactor to use built-in on Ember 3.20 --- addon/-internal/destructors.ts | 79 +++++++++++++++++++++++++++- addon/-internal/meta.ts | 1 - addon/-internal/patch-core-object.ts | 20 ++++--- addon/-internal/patch-meta.ts | 14 +++-- tests/unit/destroyable-test.ts | 14 ++++- yarn.lock | 9 ++++ 6 files changed, 121 insertions(+), 16 deletions(-) diff --git a/addon/-internal/destructors.ts b/addon/-internal/destructors.ts index f93454c..085005a 100644 --- a/addon/-internal/destructors.ts +++ b/addon/-internal/destructors.ts @@ -1,6 +1,9 @@ import { assert } from '@ember/debug'; import { schedule } from '@ember/runloop'; import { DEBUG } from '@glimmer/env'; +import Ember from 'ember'; + +import { gte } from 'ember-compatibility-helpers'; import { meta, deleteMeta } from './meta'; @@ -15,6 +18,29 @@ let DESTROYABLE_PARENTS: | WeakMap = new WeakMap(); const DESTROYABLE_CHILDREN = new WeakMap>(); +let _internalRegisterDestructor: Function; +let _internalAssociateDestroyableChild: Function; +let _internalIsDestroying: Function; +let _internalIsDestroyed: Function; +let _internalUnregisterDestructor: Function; +let _internalDestroy: Function; +let _internalAssertDestroyablesDestroyed: Function; +let _internalEnableDestroyableTracking: Function; + +if (gte('3.20.0-beta.4')) { + const glimmerRuntime = (Ember as any).__loader.require('@glimmer/runtime'); + + _internalRegisterDestructor = glimmerRuntime.registerDestructor; + _internalAssociateDestroyableChild = glimmerRuntime.associateDestroyableChild; + _internalIsDestroying = glimmerRuntime.isDestroying; + _internalIsDestroyed = glimmerRuntime.isDestroyed; + _internalUnregisterDestructor = glimmerRuntime.unregisterDestructor; + _internalDestroy = glimmerRuntime.destroy; + _internalAssertDestroyablesDestroyed = + glimmerRuntime.assertDestroyablesDestroyed; + _internalEnableDestroyableTracking = glimmerRuntime.enableDestroyableTracking; +} + function getDestructors(destroyable: T): Set> { if (!DESTRUCTORS.has(destroyable)) DESTRUCTORS.set(destroyable, new Set()); return DESTRUCTORS.get(destroyable)!; @@ -39,6 +65,10 @@ function getDestroyableChildren(destroyable: object): Set { * ``` */ export function isDestroying(destroyable: object): boolean { + if (gte('3.20.0-beta.4')) { + return _internalIsDestroying(destroyable); + } + return meta(destroyable).isSourceDestroying(); } @@ -56,6 +86,10 @@ export function isDestroying(destroyable: object): boolean { * ``` */ export function isDestroyed(destroyable: object): boolean { + if (gte('3.20.0-beta.4')) { + return _internalIsDestroyed(destroyable); + } + return meta(destroyable).isSourceDestroyed(); } @@ -97,6 +131,10 @@ export function associateDestroyableChild( parent: object, child: T ): T { + if (gte('3.20.0-beta.4')) { + return _internalAssociateDestroyableChild(parent, child); + } + if (DEBUG) assertNotDestroyed(parent); if (DEBUG) assertNotDestroyed(child); @@ -152,6 +190,10 @@ export function registerDestructor( destroyable: T, destructor: Destructor ): Destructor { + if (gte('3.20.0-beta.4')) { + return _internalRegisterDestructor(destroyable, destructor); + } + if (DEBUG) assertNotDestroyed(destroyable); const destructors = getDestructors(destroyable); assert( @@ -193,6 +235,10 @@ export function unregisterDestructor( destroyable: T, destructor: Destructor ): void { + if (gte('3.20.0-beta.4')) { + return _internalUnregisterDestructor(destroyable, destructor); + } + if (DEBUG) assertNotDestroyed(destroyable); const destructors = getDestructors(destroyable); assert( @@ -237,6 +283,11 @@ export function unregisterDestructor( * */ export function destroy(destroyable: object): void { + if (gte('3.20.0-beta.4')) { + _internalDestroy(destroyable); + return; + } + if (isDestroying(destroyable) || isDestroyed(destroyable)) return; const m = meta(destroyable); @@ -275,6 +326,19 @@ interface UndestroyedDestroyablesAssertionError extends Error { * assertDestroyablesDestroyed later. */ export function enableDestroyableTracking() { + if (gte('3.20.2')) { + return _internalEnableDestroyableTracking(); + } + if (gte('3.20.0-beta.4')) { + // on 3.20.0-beta.4 through 3.20.2 (estimated) there is an issue with the upstream + // `assertDestroyablesDestroyed` method that triggers the assertion in cases that it + // should not; in order to allow code bases to function on those specific Ember versions + // (including our own test suite) we detect and do nothing + // + // See https://github.com/glimmerjs/glimmer-vm/pull/1119 + return; + } + DESTRUCTORS = new Map>(); DESTROYABLE_PARENTS = new Map(); isTesting = true; @@ -288,6 +352,19 @@ export function enableDestroyableTracking() { * fact been destroyed. */ export function assertDestroyablesDestroyed(): void | never { + if (gte('3.20.2')) { + return _internalAssertDestroyablesDestroyed(); + } + if (gte('3.20.0-beta.4')) { + // on 3.20.0-beta.4 through 3.20.2 (estimated) there is an issue with the upstream + // `assertDestroyablesDestroyed` method that triggers the assertion in cases that it + // should not; in order to allow code bases to function on those specific Ember versions + // (including our own test suite) we detect and do nothing + // + // See https://github.com/glimmerjs/glimmer-vm/pull/1119 + return; + } + if (!isTesting) { throw new Error( 'Attempted to assert destroyables destroyed, but you did not start a destroyable test. Did you forget to call `enableDestroyableTracking()`' @@ -303,7 +380,7 @@ export function assertDestroyablesDestroyed(): void | never { if (destructors.size > 0 || children.size > 0) { const error = new Error( - `Not all destroyable objects were destroyed` + `Some destroyables were not destroyed during this test` ) as UndestroyedDestroyablesAssertionError; Object.defineProperty(error, 'destroyables', { diff --git a/addon/-internal/meta.ts b/addon/-internal/meta.ts index aace5c6..c9070e4 100644 --- a/addon/-internal/meta.ts +++ b/addon/-internal/meta.ts @@ -1,5 +1,4 @@ /* eslint-disable no-bitwise */ -// import { gte } from 'ember-compatibility-helpers'; import Ember from 'ember'; import { gte } from 'ember-compatibility-helpers'; diff --git a/addon/-internal/patch-core-object.ts b/addon/-internal/patch-core-object.ts index 88f0b18..7cf4050 100644 --- a/addon/-internal/patch-core-object.ts +++ b/addon/-internal/patch-core-object.ts @@ -1,14 +1,18 @@ +import { gte } from 'ember-compatibility-helpers'; + import CoreObject from '@ember/object/core'; import { destroy as _destroy, registerDestructor } from '..'; -const callWillDestroy = (instance: CoreObject) => instance.willDestroy(); +if (!gte('3.20.0-beta.4')) { + const callWillDestroy = (instance: CoreObject) => instance.willDestroy(); -CoreObject.prototype.init = function init() { - registerDestructor(this, callWillDestroy); -}; + CoreObject.prototype.init = function init() { + registerDestructor(this, callWillDestroy); + }; -CoreObject.prototype.destroy = function destroy() { - _destroy(this); - return this; -}; + CoreObject.prototype.destroy = function destroy() { + _destroy(this); + return this; + }; +} diff --git a/addon/-internal/patch-meta.ts b/addon/-internal/patch-meta.ts index c1bb023..b4cffac 100644 --- a/addon/-internal/patch-meta.ts +++ b/addon/-internal/patch-meta.ts @@ -1,9 +1,13 @@ +import { gte } from 'ember-compatibility-helpers'; + import { runDestructors } from './destructors'; import { Meta } from './meta'; -const { setSourceDestroying } = Meta.prototype; +if (!gte('3.20.0-beta.1')) { + const { setSourceDestroying } = Meta.prototype; -Meta.prototype.setSourceDestroying = function () { - setSourceDestroying.call(this); - runDestructors(this.source); -}; + Meta.prototype.setSourceDestroying = function () { + setSourceDestroying.call(this); + runDestructors(this.source); + }; +} diff --git a/tests/unit/destroyable-test.ts b/tests/unit/destroyable-test.ts index 285801f..de2942e 100644 --- a/tests/unit/destroyable-test.ts +++ b/tests/unit/destroyable-test.ts @@ -14,6 +14,8 @@ import { import CoreObject from '@ember/object/core'; import { run } from '@ember/runloop'; +import { gte } from 'ember-compatibility-helpers'; + function makeDestructor( assert: Assert, step: string, @@ -247,6 +249,16 @@ module('destroyable', function (_hooks) { }); module('assertDestroyablesDestroyed', function () { + if (gte('3.20.0-beta.4') && !gte('3.20.2')) { + // on 3.20.0-beta.4 through 3.20.2 (estimated) there is an issue with the upstream + // `assertDestroyablesDestroyed` method that triggers the assertion in cases that it + // should not; in order to allow code bases to function on those specific Ember versions + // (including our own test suite) we detect and do nothing + // + // See https://github.com/glimmerjs/glimmer-vm/pull/1119 + return; + } + test('it does not throw an error when destroyables have been destroyed', async function (assert) { assert.expect(1); @@ -286,7 +298,7 @@ module('destroyable', function (_hooks) { assert.throws(() => { assertDestroyablesDestroyed(); - }, /Not all destroyable objects were destroyed/); + }, /Some destroyables were not destroyed during this test/); }); test('errors if `enableDestroyableTracking` was not called previously', async function (assert) { diff --git a/yarn.lock b/yarn.lock index 94f61f0..657e17a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5431,6 +5431,15 @@ ember-compatibility-helpers@^1.1.2, ember-compatibility-helpers@^1.2.0, ember-co ember-cli-version-checker "^2.1.1" semver "^5.4.1" +ember-compatibility-helpers@^1.2.1: + version "1.2.1" + resolved "https://registry.yarnpkg.com/ember-compatibility-helpers/-/ember-compatibility-helpers-1.2.1.tgz#87c92c4303f990ff455c28ca39fb3ee11441aa16" + integrity sha512-6wzYvnhg1ihQUT5yGqnLtleq3Nv5KNv79WhrEuNU9SwR4uIxCO+KpyC7r3d5VI0EM7/Nmv9Nd0yTkzmTMdVG1A== + dependencies: + babel-plugin-debug-macros "^0.2.0" + ember-cli-version-checker "^2.1.1" + semver "^5.4.1" + ember-disable-prototype-extensions@^1.1.3: version "1.1.3" resolved "https://registry.yarnpkg.com/ember-disable-prototype-extensions/-/ember-disable-prototype-extensions-1.1.3.tgz#1969135217654b5e278f9fe2d9d4e49b5720329e"