From f776ac84baa67ae29fb2c13a94f2ca55a967b2b3 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Wed, 30 Sep 2020 15:31:02 -0400 Subject: [PATCH] Enforce usage of `capabilities` generation. Prior to this change it was possible to avoid using a given type's `capabilities` builder function (intentionally or on accident) by doing something like: ```js class MyManager { capabilities = { // magical properties from Ember's internals } } ``` The API's that we _intended_ folks to be using is something like: ```js import { capabilties as modifierCapabilities } from '@ember/modifier'; class MyManager { capabilities = modifierCapabilities('3.22'); } ``` This commit ensures that Ember's own internal structures can not be "spoofed" (avoiding our constraints, or creating a frankenstein manager). --- .../glimmer/lib/component-managers/custom.ts | 27 ++--- .../-internals/glimmer/lib/helpers/custom.ts | 7 +- .../glimmer/lib/modifiers/custom.ts | 12 +- .../-internals/glimmer/lib/utils/managers.ts | 56 ++++++++- .../custom-component-manager-test.js | 112 ++++++++++++++++++ .../custom-modifier-manager-test.js | 37 +++++- .../helpers/helper-manager-test.js | 28 +++++ 7 files changed, 246 insertions(+), 33 deletions(-) diff --git a/packages/@ember/-internals/glimmer/lib/component-managers/custom.ts b/packages/@ember/-internals/glimmer/lib/component-managers/custom.ts index 7f5f822a0d3..2538dc4d01c 100644 --- a/packages/@ember/-internals/glimmer/lib/component-managers/custom.ts +++ b/packages/@ember/-internals/glimmer/lib/component-managers/custom.ts @@ -19,6 +19,7 @@ import { EmberVMEnvironment } from '../environment'; import RuntimeResolver from '../resolver'; import { OwnedTemplate } from '../template'; import { argsProxyFor } from '../utils/args-proxy'; +import { buildCapabilities, InternalCapabilities } from '../utils/managers'; import AbstractComponentManager from './abstract'; const CAPABILITIES = { @@ -49,6 +50,12 @@ export interface OptionalCapabilities { }; } +export interface Capabilities extends InternalCapabilities { + asyncLifeCycleCallbacks: boolean; + destructor: boolean; + updateHook: boolean; +} + export function capabilities( managerAPI: Version, options: OptionalCapabilities[Version] = {} @@ -64,11 +71,11 @@ export function capabilities( updateHook = Boolean((options as OptionalCapabilities['3.13']).updateHook); } - return { + return buildCapabilities({ asyncLifeCycleCallbacks: Boolean(options.asyncLifecycleCallbacks), destructor: Boolean(options.destructor), updateHook, - }; + }) as Capabilities; } export interface DefinitionState { @@ -77,21 +84,9 @@ export interface DefinitionState { template: OwnedTemplate; } -export interface Capabilities { - asyncLifeCycleCallbacks: boolean; - destructor: boolean; - updateHook: boolean; -} - -// TODO: export ICapturedArgumentsValue from glimmer and replace this -export interface Args { - named: Dict; - positional: unknown[]; -} - export interface ManagerDelegate { capabilities: Capabilities; - createComponent(factory: unknown, args: Args): ComponentInstance; + createComponent(factory: unknown, args: Arguments): ComponentInstance; getContext(instance: ComponentInstance): unknown; } @@ -114,7 +109,7 @@ export function hasUpdateHook( export interface ManagerDelegateWithUpdateHook extends ManagerDelegate { - updateComponent(instance: ComponentInstance, args: Args): void; + updateComponent(instance: ComponentInstance, args: Arguments): void; } export function hasAsyncUpdateHook( diff --git a/packages/@ember/-internals/glimmer/lib/helpers/custom.ts b/packages/@ember/-internals/glimmer/lib/helpers/custom.ts index 3f673627420..912842630b6 100644 --- a/packages/@ember/-internals/glimmer/lib/helpers/custom.ts +++ b/packages/@ember/-internals/glimmer/lib/helpers/custom.ts @@ -3,10 +3,11 @@ import { DEBUG } from '@glimmer/env'; import { Arguments, Helper as GlimmerHelper } from '@glimmer/interfaces'; import { createComputeRef, UNDEFINED_REFERENCE } from '@glimmer/reference'; import { argsProxyFor } from '../utils/args-proxy'; +import { buildCapabilities, InternalCapabilities } from '../utils/managers'; export type HelperDefinition = object; -export interface HelperCapabilities { +export interface HelperCapabilities extends InternalCapabilities { hasValue: boolean; hasDestroyable: boolean; hasScheduledEffect: boolean; @@ -29,11 +30,11 @@ export function helperCapabilities( !options.hasScheduledEffect ); - return { + return buildCapabilities({ hasValue: Boolean(options.hasValue), hasDestroyable: Boolean(options.hasDestroyable), hasScheduledEffect: Boolean(options.hasScheduledEffect), - }; + }); } export interface HelperManager { diff --git a/packages/@ember/-internals/glimmer/lib/modifiers/custom.ts b/packages/@ember/-internals/glimmer/lib/modifiers/custom.ts index 3dda936be71..63cf4a97227 100644 --- a/packages/@ember/-internals/glimmer/lib/modifiers/custom.ts +++ b/packages/@ember/-internals/glimmer/lib/modifiers/custom.ts @@ -6,6 +6,7 @@ import { registerDestructor, reifyArgs } from '@glimmer/runtime'; import { createUpdatableTag, untrack, UpdatableTag } from '@glimmer/validator'; import { SimpleElement } from '@simple-dom/interface'; import { argsProxyFor } from '../utils/args-proxy'; +import { buildCapabilities, InternalCapabilities } from '../utils/managers'; export interface CustomModifierDefinitionState { ModifierClass: Factory; @@ -25,7 +26,7 @@ export interface OptionalCapabilities { }; } -export interface Capabilities { +export interface Capabilities extends InternalCapabilities { disableAutoTracking: boolean; useArgsProxy: boolean; passFactoryToCreate: boolean; @@ -40,11 +41,11 @@ export function capabilities( managerAPI === '3.13' || managerAPI === '3.22' ); - return { + return buildCapabilities({ disableAutoTracking: Boolean(optionalFeatures.disableAutoTracking), useArgsProxy: managerAPI === '3.13' ? false : true, passFactoryToCreate: managerAPI === '3.13', - }; + }) as Capabilities; } export class CustomModifierDefinition { @@ -124,11 +125,6 @@ class InteractiveCustomModifierManager let { delegate, ModifierClass } = definition; let capturedArgs = vmArgs.capture(); - assert( - 'Custom modifier managers must define their capabilities using the capabilities() helper function', - typeof delegate.capabilities === 'object' && delegate.capabilities !== null - ); - let { useArgsProxy, passFactoryToCreate } = delegate.capabilities; let args = useArgsProxy ? argsProxyFor(capturedArgs, 'modifier') : reifyArgs(capturedArgs); diff --git a/packages/@ember/-internals/glimmer/lib/utils/managers.ts b/packages/@ember/-internals/glimmer/lib/utils/managers.ts index 7045a77af58..db378330787 100644 --- a/packages/@ember/-internals/glimmer/lib/utils/managers.ts +++ b/packages/@ember/-internals/glimmer/lib/utils/managers.ts @@ -1,8 +1,10 @@ import { Owner } from '@ember/-internals/owner'; -import { deprecate } from '@ember/debug'; +import { assert, deprecate } from '@ember/debug'; import { COMPONENT_MANAGER_STRING_LOOKUP } from '@ember/deprecated-features'; +import { DEBUG } from '@glimmer/env'; +import { _WeakSet } from '@glimmer/util'; import { ManagerDelegate as ComponentManagerDelegate } from '../component-managers/custom'; -import InternalComponentManager from '../component-managers/internal'; +import InternalComponentManager, { isInternalManager } from '../component-managers/internal'; import { HelperManager } from '../helpers/custom'; import { ModifierManagerDelegate } from '../modifiers/custom'; @@ -17,6 +19,8 @@ const COMPONENT_MANAGERS = new WeakMap< ManagerFactory | InternalComponentManager> >(); +const FROM_CAPABILITIES = DEBUG ? new _WeakSet() : undefined; + const MODIFIER_MANAGERS = new WeakMap>>(); const HELPER_MANAGERS = new WeakMap>>(); @@ -71,6 +75,7 @@ function getManagerInstanceForOwner( if (instance === undefined) { instance = factory(owner); + managers.set(factory, instance!); } @@ -94,7 +99,15 @@ export function getModifierManager( const factory = getManager(MODIFIER_MANAGERS, definition); if (factory !== undefined) { - return getManagerInstanceForOwner(owner, factory); + let manager = getManagerInstanceForOwner(owner, factory); + assert( + `Custom modifier managers must have a \`capabilities\` property that is the result of calling the \`capabilities('3.13' | '3.22')\` (imported via \`import { capabilities } from '@ember/modifier';\`). Received: \`${JSON.stringify( + manager.capabilities + )}\` for: \`${manager}\``, + FROM_CAPABILITIES!.has(manager.capabilities) + ); + + return manager; } return undefined; @@ -114,7 +127,16 @@ export function getHelperManager( const factory = getManager(HELPER_MANAGERS, definition); if (factory !== undefined) { - return getManagerInstanceForOwner(owner, factory); + let manager = getManagerInstanceForOwner(owner, factory); + + assert( + `Custom helper managers must have a \`capabilities\` property that is the result of calling the \`capabilities('3.23')\` (imported via \`import { capabilities } from '@ember/helper';\`). Received: \`${JSON.stringify( + manager.capabilities + )}\` for: \`${manager}\``, + FROM_CAPABILITIES!.has(manager.capabilities) + ); + + return manager; } return undefined; @@ -161,8 +183,32 @@ export function getComponentManager( ); if (factory !== undefined) { - return getManagerInstanceForOwner(owner, factory); + let manager = getManagerInstanceForOwner(owner, factory); + + assert( + `Custom component managers must have a \`capabilities\` property that is the result of calling the \`capabilities('3.4' | '3.13')\` (imported via \`import { capabilities } from '@ember/component';\`). Received: \`${JSON.stringify( + (manager as ComponentManagerDelegate).capabilities + )}\` for: \`${manager}\``, + isInternalManager(manager) || FROM_CAPABILITIES!.has(manager.capabilities) + ); + + return manager; } return undefined; } + +declare const INTERNAL_CAPABILITIES: unique symbol; + +export interface InternalCapabilities { + [INTERNAL_CAPABILITIES]: true; +} + +export function buildCapabilities(capabilities: T): T & InternalCapabilities { + if (DEBUG) { + FROM_CAPABILITIES!.add(capabilities); + Object.freeze(capabilities); + } + + return capabilities as T & InternalCapabilities; +} diff --git a/packages/@ember/-internals/glimmer/tests/integration/custom-component-manager-test.js b/packages/@ember/-internals/glimmer/tests/integration/custom-component-manager-test.js index fc5ade2b2e5..3cc855fc8af 100644 --- a/packages/@ember/-internals/glimmer/tests/integration/custom-component-manager-test.js +++ b/packages/@ember/-internals/glimmer/tests/integration/custom-component-manager-test.js @@ -522,6 +522,62 @@ moduleFor( this.assertHTML(`

hello max

`); assert.verifySteps(['updateComponent', 'didUpdateComponent']); } + + '@test capabilities helper function must be used to generate capabilities'(assert) { + let ComponentClass = setComponentManager( + () => { + return EmberObject.create({ + capabilities: { + asyncLifecycleCallbacks: true, + destructor: true, + update: false, + }, + + createComponent(factory, args) { + assert.step('createComponent'); + return factory.create({ args }); + }, + + updateComponent(component, args) { + assert.step('updateComponent'); + set(component, 'args', args); + }, + + destroyComponent(component) { + assert.step('destroyComponent'); + component.destroy(); + }, + + getContext(component) { + assert.step('getContext'); + return component; + }, + + didCreateComponent() { + assert.step('didCreateComponent'); + }, + + didUpdateComponent() { + assert.step('didUpdateComponent'); + }, + }); + }, + EmberObject.extend({ + greeting: 'hello', + }) + ); + + this.registerComponent('foo-bar', { + template: `

{{greeting}} {{@name}}

`, + ComponentClass, + }); + + expectAssertion(() => { + this.render('{{foo-bar name=name}}', { name: 'world' }); + }, /Custom component managers must have a `capabilities` property that is the result of calling the `capabilities\('3.4' \| '3.13'\)` \(imported via `import \{ capabilities \} from '@ember\/component';`\). /); + + assert.verifySteps([]); + } } ); @@ -774,5 +830,61 @@ moduleFor( runTask(() => this.context.set('value', 'bar')); assert.verifySteps([]); } + + '@test capabilities helper function must be used to generate capabilities'(assert) { + let ComponentClass = setComponentManager( + () => { + return EmberObject.create({ + capabilities: { + asyncLifecycleCallbacks: true, + destructor: true, + update: false, + }, + + createComponent(factory, args) { + assert.step('createComponent'); + return factory.create({ args }); + }, + + updateComponent(component, args) { + assert.step('updateComponent'); + set(component, 'args', args); + }, + + destroyComponent(component) { + assert.step('destroyComponent'); + component.destroy(); + }, + + getContext(component) { + assert.step('getContext'); + return component; + }, + + didCreateComponent() { + assert.step('didCreateComponent'); + }, + + didUpdateComponent() { + assert.step('didUpdateComponent'); + }, + }); + }, + EmberObject.extend({ + greeting: 'hello', + }) + ); + + this.registerComponent('foo-bar', { + template: `

{{greeting}} {{@name}}

`, + ComponentClass, + }); + + expectAssertion(() => { + this.render('', { name: 'world' }); + }, /Custom component managers must have a `capabilities` property that is the result of calling the `capabilities\('3.4' \| '3.13'\)` \(imported via `import \{ capabilities \} from '@ember\/component';`\). /); + + assert.verifySteps([]); + } } ); diff --git a/packages/@ember/-internals/glimmer/tests/integration/custom-modifier-manager-test.js b/packages/@ember/-internals/glimmer/tests/integration/custom-modifier-manager-test.js index b37d26f785e..c05ee65d7e5 100644 --- a/packages/@ember/-internals/glimmer/tests/integration/custom-modifier-manager-test.js +++ b/packages/@ember/-internals/glimmer/tests/integration/custom-modifier-manager-test.js @@ -21,7 +21,7 @@ class ModifierManagerTest extends RenderingTestCase { expectAssertion(() => { this.render('

hello world

'); - }, /Custom modifier managers must define their capabilities/); + }, /Custom modifier managers must have a `capabilities` property /); } '@test can register a custom element modifier and render it'(assert) { @@ -266,6 +266,41 @@ class ModifierManagerTest extends RenderingTestCase { this.render('

hello world

', { person: new Person() }); }, expectedMessage); } + + '@test capabilities helper function must be used to generate capabilities'(assert) { + class OverrideCustomModifierManager extends this.CustomModifierManager { + capabilities = { + disableAutoTracking: false, + useArgsProxy: true, + passFactoryToCreate: false, + }; + } + + let ModifierClass = setModifierManager( + (owner) => { + return new OverrideCustomModifierManager(owner); + }, + EmberObject.extend({ + didInsertElement() { + assert.step('didInsertElement'); + }, + didUpdate() { + assert.step('didUpdate'); + }, + willDestroyElement() { + assert.step('willDestroyElement'); + }, + }) + ); + + this.registerModifier('foo-bar', ModifierClass.extend()); + + expectAssertion(() => { + this.render('

hello world

'); + }, /Custom modifier managers must have a `capabilities` property that is the result of calling the `capabilities\('3.13' \| '3.22'\)` \(imported via `import \{ capabilities \} from '@ember\/modifier';`\). /); + + assert.verifySteps([]); + } } moduleFor( diff --git a/packages/@ember/-internals/glimmer/tests/integration/helpers/helper-manager-test.js b/packages/@ember/-internals/glimmer/tests/integration/helpers/helper-manager-test.js index 33821e44280..81b0862058b 100644 --- a/packages/@ember/-internals/glimmer/tests/integration/helpers/helper-manager-test.js +++ b/packages/@ember/-internals/glimmer/tests/integration/helpers/helper-manager-test.js @@ -289,6 +289,34 @@ if (EMBER_GLIMMER_HELPER_MANAGER) { this.assertText('hello'); } + + '@test capabilities helper function must be used to generate capabilities'(assert) { + class OverrideTestHelperManager extends TestHelperManager { + capabilities = { + hasValue: true, + hasDestroyable: true, + hasScheduledEffect: false, + }; + } + + class TestHelper {} + + setHelperManager((owner) => new OverrideTestHelperManager(owner), TestHelper); + this.registerCustomHelper( + 'hello', + class extends TestHelper { + value() { + return 'hello'; + } + } + ); + + expectAssertion(() => { + this.render('{{hello}}'); + }, /Custom helper managers must have a `capabilities` property that is the result of calling the `capabilities\('3.23'\)` \(imported via `import \{ capabilities \} from '@ember\/helper';`\). /); + + assert.verifySteps([]); + } } ); }