Skip to content

Commit

Permalink
feat(modifier-managers): use 3.22 capabilities
Browse files Browse the repository at this point in the history
BREAKING CHANGE: destruction behavior changed to use new args proxy from
3.22

Resolves the issue
  described by: emberjs/ember.js#19162
  fixed by: emberjs/ember.js#19163
  • Loading branch information
NullVoxPopuli committed Oct 18, 2020
1 parent 8a80542 commit 6662268
Show file tree
Hide file tree
Showing 6 changed files with 138 additions and 70 deletions.
24 changes: 19 additions & 5 deletions addon/-private/class/modifier-manager.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { capabilities } from '@ember/modifier';
import { gte } from 'ember-compatibility-helpers';
import { set } from '@ember/object';
import { destroy, registerDestructor } from '@ember/destroyable';

Expand All @@ -10,16 +11,30 @@ function destroyModifier(modifier: ClassBasedModifier): void {
modifier.willDestroy();
}

type CreateModifierArgs313 = [
{ owner: unknown; class: typeof ClassBasedModifier },
ModifierArgs
];
type CreateModifierArgs322 = [typeof ClassBasedModifier, ModifierArgs];
type CreateModifierArgs = CreateModifierArgs313 | CreateModifierArgs322;

export default class ClassBasedModifierManager {
capabilities = capabilities('3.13');
capabilities = capabilities(gte('3.22.0') ? '3.22' : '3.13');

constructor(private owner: unknown) {}

createModifier(
factory: { owner: unknown; class: typeof ClassBasedModifier },
args: ModifierArgs
...createModifierArgs: CreateModifierArgs
): ClassBasedModifier {
const Modifier = factory.class;
const [factoryOrClass, args] = createModifierArgs;

let Modifier: typeof ClassBasedModifier;

if ('owner' in factoryOrClass) {
Modifier = factoryOrClass.class;
} else {
Modifier = factoryOrClass;
}

const modifier = new Modifier(this.owner, args);

Expand All @@ -35,7 +50,6 @@ export default class ClassBasedModifierManager {
}

updateModifier(instance: ClassBasedModifier, args: ModifierArgs): void {
// TODO: this should be an args proxy
set(instance, 'args', args);
instance.didUpdateArguments();
instance.didReceiveArguments();
Expand Down
17 changes: 14 additions & 3 deletions addon/-private/functional/modifier-manager.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { capabilities } from '@ember/modifier';
import { gte } from 'ember-compatibility-helpers';
import { FunctionalModifier } from './modifier';
import { ModifierArgs } from '../interfaces';

Expand Down Expand Up @@ -28,15 +29,25 @@ function setup(
MODIFIER_TEARDOWNS.set(modifier, teardown);
}

type CreateModifierArgs313 = [Factory];
type CreateModifierArgs322 = [FunctionalModifier];
type CreateModifierArgs = CreateModifierArgs313 | CreateModifierArgs322;

class FunctionalModifierManager {
capabilities = capabilities('3.13');
capabilities = capabilities(gte('3.22.0') ? '3.22' : '3.13');

createModifier(factory: Factory): FunctionalModifier {
createModifier(...[factoryOrClass]: CreateModifierArgs): FunctionalModifier {
// This looks superfluous, but this is creating a new instance
// of a function -- this is important so that each instance of the
// created modifier can have its own state which is stored in
// the MODIFIER_ELEMENTS and MODIFIER_TEARDOWNS WeakMaps
return (...args) => factory.class(...args);
return (...args) => {
if ('class' in factoryOrClass) {
return factoryOrClass.class(...args);
}

return factoryOrClass(...args);
};
}

installModifier(
Expand Down
7 changes: 3 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,7 @@
"ember-cli-normalize-entity-name": "^1.0.0",
"ember-cli-string-utils": "^1.1.0",
"ember-cli-typescript": "^3.1.3",
"ember-destroyable-polyfill": "^2.0.2",
"ember-modifier-manager-polyfill": "^1.2.0"
"ember-compatibility-helpers": "^1.2.1"
},
"devDependencies": {
"@ember/optional-features": "^1.3.0",
Expand All @@ -66,14 +65,14 @@
"ember-cli-sri": "^2.1.1",
"ember-cli-typescript-blueprints": "^3.0.0",
"ember-cli-uglify": "^3.0.0",
"ember-decorators-polyfill": "^1.0.6",
"ember-decorators-polyfill": "^1.1.5",
"ember-disable-prototype-extensions": "^1.1.3",
"ember-export-application-global": "^2.0.1",
"ember-load-initializers": "^2.1.1",
"ember-maybe-import-regenerator": "^0.1.6",
"ember-qunit": "^4.6.0",
"ember-resolver": "^8.0.0",
"ember-source": "~3.20.2",
"ember-source": "~3.22.0",
"ember-source-channel-url": "^2.0.1",
"ember-template-lint": "^2.9.1",
"ember-try": "^1.4.0",
Expand Down
105 changes: 76 additions & 29 deletions tests/integration/modifiers/class-modifier-test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { gte } from 'ember-compatibility-helpers';
import { module, test } from 'qunit';
import { setupRenderingTest } from 'ember-qunit';
import { render, settled } from '@ember/test-helpers';
Expand Down Expand Up @@ -463,35 +464,81 @@ module('Integration | Modifier Manager | class-based modifier', function (
) {
setupRenderingTest(hooks);

testHooks(
(callback) =>
class NativeModifier extends Modifier {
constructor(owner: unknown, args: ModifierArgs) {
super(owner, args);
callback('constructor', this);
}

didReceiveArguments(): void {
callback('didReceiveArguments', this);
}

didUpdateArguments(): void {
callback('didUpdateArguments', this);
}

didInstall(): void {
callback('didInstall', this);
}

willRemove(): void {
callback('willRemove', this);
}

willDestroy(): void {
callback('willDestroy', this);
}
}
);
if (gte('3.22.0')) {
module('capabilities(3.22)', function (hooks) {
testHooks(
(callback) =>
class NativeModifier extends Modifier {
constructor(owner: unknown, args: ModifierArgs) {
super(owner, args);
callback('constructor', this);
}

didReceiveArguments(): void {
for (let i = 0; i < this.args.positional.length; i++) {
// "noop" / consume the arg
this.args.positional[i];
}

for (const key of Object.keys(this.args.named)) {
// "noop" / consume the arg
this.args.named[key];
}

callback('didReceiveArguments', this);
}

didUpdateArguments(): void {
callback('didUpdateArguments', this);
}

didInstall(): void {
callback('didInstall', this);
}

willRemove(): void {
callback('willRemove', this);
}

willDestroy(): void {
callback('willDestroy', this);
}
}
);
});
} else {
module('capabilities(3.13)', function (hooks) {
testHooks(
(callback) =>
class NativeModifier extends Modifier {
constructor(owner: unknown, args: ModifierArgs) {
super(owner, args);
callback('constructor', this);
}

didReceiveArguments(): void {
callback('didReceiveArguments', this);
}

didUpdateArguments(): void {
callback('didUpdateArguments', this);
}

didInstall(): void {
callback('didInstall', this);
}

willRemove(): void {
callback('willRemove', this);
}

willDestroy(): void {
callback('willDestroy', this);
}
}
);
});
}

module('service injection', function () {
test('can participate in ember dependency injection', async function (this: TestContext, assert) {
Expand Down
23 changes: 19 additions & 4 deletions tests/integration/modifiers/functional-modifier-test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { gte } from 'ember-compatibility-helpers';
import { module, test } from 'qunit';
import { setupRenderingTest } from 'ember-qunit';
import { render, settled } from '@ember/test-helpers';
Expand Down Expand Up @@ -89,10 +90,17 @@ module('Integration | Modifiers | functional modifier', function (hooks) {

this.registerModifier(
'songbird',
modifier(() => callCount++)
modifier((_, positional: [number]) => {
// A change is not triggered unless args are consumed
if (gte('3.22.0')) {
positional[0];
}

return callCount++;
})
);

await render(hbs`<h1 {{songbird value}}>Hello</h1>`);
await render(hbs`<h1 {{songbird this.value}}>Hello</h1>`);

assert.equal(callCount, 1);

Expand All @@ -109,10 +117,17 @@ module('Integration | Modifiers | functional modifier', function (hooks) {

this.registerModifier(
'songbird',
modifier(() => () => callCount++)
modifier((_, positional: [number]) => {
// A change is not triggered unless args are consumed
if (gte('3.22.0')) {
positional[0];
}

return () => callCount++;
})
);

await render(hbs`<h1 {{songbird value}}>Hello</h1>`);
await render(hbs`<h1 {{songbird this.value}}>Hello</h1>`);

assert.equal(callCount, 0);

Expand Down
32 changes: 7 additions & 25 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4970,7 +4970,7 @@ ember-cli-babel@^6.0.0-beta.4, ember-cli-babel@^6.6.0, ember-cli-babel@^6.8.1:
ember-cli-version-checker "^2.1.2"
semver "^5.5.0"

ember-cli-babel@^7.0.0, ember-cli-babel@^7.1.2, ember-cli-babel@^7.10.0, ember-cli-babel@^7.11.0, ember-cli-babel@^7.12.0, ember-cli-babel@^7.18.0, ember-cli-babel@^7.19.0, ember-cli-babel@^7.20.5, ember-cli-babel@^7.22.1, ember-cli-babel@^7.7.3:
ember-cli-babel@^7.0.0, ember-cli-babel@^7.1.2, ember-cli-babel@^7.11.0, ember-cli-babel@^7.12.0, ember-cli-babel@^7.18.0, ember-cli-babel@^7.19.0, ember-cli-babel@^7.20.5, ember-cli-babel@^7.22.1, ember-cli-babel@^7.7.3:
version "7.22.1"
resolved "https://registry.yarnpkg.com/ember-cli-babel/-/ember-cli-babel-7.22.1.tgz#cad28b89cf0e184c93b863d09bc5ba4ce1d2e453"
integrity sha512-kCT8WbC1AYFtyOpU23ESm22a+gL6fWv8Nzwe8QFQ5u0piJzM9MEudfbjADEaoyKTrjMQTDsrWwEf3yjggDsOng==
Expand Down Expand Up @@ -5345,24 +5345,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-decorators-polyfill@^1.0.6:
ember-decorators-polyfill@^1.1.5:
version "1.1.5"
resolved "https://registry.yarnpkg.com/ember-decorators-polyfill/-/ember-decorators-polyfill-1.1.5.tgz#49203c302ea4486618ba4866923ec657cf2c9f3d"
resolved "https://registry.npmjs.org/ember-decorators-polyfill/-/ember-decorators-polyfill-1.1.5.tgz#49203c302ea4486618ba4866923ec657cf2c9f3d"
integrity sha512-O154i8sLoVjsiKzSqxGRfHGr+N+drT6mRrLDbNgJCnW/V5uLg/ppZFpUsrdxuXnp5Q9us3OfXV1nX2CH+7bUpA==
dependencies:
ember-cli-babel "^7.1.2"
ember-cli-version-checker "^3.1.3"
ember-compatibility-helpers "^1.2.0"

ember-destroyable-polyfill@^2.0.2:
version "2.0.2"
resolved "https://registry.yarnpkg.com/ember-destroyable-polyfill/-/ember-destroyable-polyfill-2.0.2.tgz#2cc7532bd3c00e351b4da9b7fc683f4daff79671"
integrity sha512-9t+ya+9c+FkNM5IAyJIv6ETG8jfZQaUnFCO5SeLlV0wkSw7TOexyb61jh5GVee0KmknfRhrRGGAyT4Y0TwkZ+w==
dependencies:
ember-cli-babel "^7.22.1"
ember-cli-version-checker "^5.1.1"
ember-compatibility-helpers "^1.2.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"
Expand Down Expand Up @@ -5391,15 +5382,6 @@ ember-maybe-import-regenerator@^0.1.6:
ember-cli-babel "^6.0.0-beta.4"
regenerator-runtime "^0.9.5"

ember-modifier-manager-polyfill@^1.2.0:
version "1.2.0"
resolved "https://registry.yarnpkg.com/ember-modifier-manager-polyfill/-/ember-modifier-manager-polyfill-1.2.0.tgz#cf4444e11a42ac84f5c8badd85e635df57565dda"
integrity sha512-bnaKF1LLKMkBNeDoetvIJ4vhwRPKIIumWr6dbVuW6W6p4QV8ZiO+GdF8J7mxDNlog9CeL9Z/7wam4YS86G8BYA==
dependencies:
ember-cli-babel "^7.10.0"
ember-cli-version-checker "^2.1.2"
ember-compatibility-helpers "^1.2.0"

ember-qunit@^4.6.0:
version "4.6.0"
resolved "https://registry.yarnpkg.com/ember-qunit/-/ember-qunit-4.6.0.tgz#ad79fd3ff00073a8779400cc5a4b44829517590f"
Expand Down Expand Up @@ -5453,10 +5435,10 @@ ember-source-channel-url@^2.0.1:
dependencies:
got "^8.0.1"

ember-source@~3.20.2:
version "3.20.2"
resolved "https://registry.yarnpkg.com/ember-source/-/ember-source-3.20.2.tgz#c8ea4fd43230ae91e9362c3136b37ed9bdd91c2a"
integrity sha512-9uPBKF7B7doz6u0z+0vBczGqaAVpcGmjqQkZdtf0C0aYY7NXRYDMZrx7vudy5DRhP13Ryo4rjZhlcRgbFPR44w==
ember-source@~3.22.0:
version "3.22.0"
resolved "https://registry.npmjs.org/ember-source/-/ember-source-3.22.0.tgz#aa09db2cc8e4f78de4bf9a12ce9ff499d416adc2"
integrity sha512-6F/fWA5et4AMFXm+siCIhpM2XrO8Emwqln71qK67JyUhvD3MJJtvwtBoKq7bzK9I/86LLw13JYm4o6T3d2gXBw==
dependencies:
"@babel/helper-module-imports" "^7.8.3"
"@babel/plugin-transform-block-scoping" "^7.8.3"
Expand Down

0 comments on commit 6662268

Please sign in to comment.