From c4b9e3ae62778906c9ba9205277e85060ba1a08a Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Wed, 17 Apr 2019 18:48:37 -0400 Subject: [PATCH] [FEATURE EMBER_GLIMMER_FN_HELPER] Initial implementation of fn helper. As proposed in https://emberjs.github.io/rfcs/0470-fn-helper.html. Deviates from the original RFC's proposal on handling `this`. Specifically, it uses a debug build proxy to issue an assertion if the callback function passed to `fn` uses `this` but isn't bound before hand (via `@action`, `(action`, etc). --- .../-internals/glimmer/lib/helpers/fn.ts | 57 ++++++ .../@ember/-internals/glimmer/lib/resolver.ts | 17 +- .../tests/integration/helpers/fn-test.js | 163 ++++++++++++++++++ packages/@ember/canary-features/index.ts | 2 + 4 files changed, 235 insertions(+), 4 deletions(-) create mode 100644 packages/@ember/-internals/glimmer/lib/helpers/fn.ts create mode 100644 packages/@ember/-internals/glimmer/tests/integration/helpers/fn-test.js diff --git a/packages/@ember/-internals/glimmer/lib/helpers/fn.ts b/packages/@ember/-internals/glimmer/lib/helpers/fn.ts new file mode 100644 index 00000000000..3a0474514b9 --- /dev/null +++ b/packages/@ember/-internals/glimmer/lib/helpers/fn.ts @@ -0,0 +1,57 @@ +import { HAS_NATIVE_PROXY } from '@ember/-internals/utils'; +import { assert } from '@ember/debug'; +import { DEBUG } from '@glimmer/env'; +import { Arguments, VM } from '@glimmer/runtime'; +import { ICapturedArguments } from '@glimmer/runtime/dist/types/lib/vm/arguments'; +import { InternalHelperReference } from '../utils/references'; + +let context: any = null; +if (DEBUG && HAS_NATIVE_PROXY) { + let assertOnProperty = (property: string | number | symbol) => { + assert( + `You accessed \`this.${String( + property + )}\` from a function passed to the \`fn\` helper, but the function itself was not bound to a valid \`this\` context. Consider updating to usage of \`@action\`.` + ); + }; + + context = new Proxy( + {}, + { + get(_target: {}, property: string | symbol) { + assertOnProperty(property); + }, + + set(_target: {}, property: string | symbol) { + assertOnProperty(property); + + return false; + }, + + has(_target: {}, property: string | symbol) { + assertOnProperty(property); + + return false; + }, + } + ); +} + +function fnHelper({ positional }: ICapturedArguments) { + assert( + `You must pass a function as the \`fn\` helpers first argument, you passed ${positional + .at(0) + .value()}`, + typeof positional.at(0).value() === 'function' + ); + + return () => { + let [fn, ...args] = positional.value(); + + return fn!['apply'](context, args); + }; +} + +export default function(_vm: VM, args: Arguments) { + return new InternalHelperReference(fnHelper, args.capture()); +} diff --git a/packages/@ember/-internals/glimmer/lib/resolver.ts b/packages/@ember/-internals/glimmer/lib/resolver.ts index 53ec8cad3c3..6c25171f12b 100644 --- a/packages/@ember/-internals/glimmer/lib/resolver.ts +++ b/packages/@ember/-internals/glimmer/lib/resolver.ts @@ -4,6 +4,7 @@ import { LookupOptions, Owner, setOwner } from '@ember/-internals/owner'; import { lookupComponent, lookupPartial, OwnedTemplateMeta } from '@ember/-internals/views'; import { EMBER_GLIMMER_ANGLE_BRACKET_BUILT_INS, + EMBER_GLIMMER_FN_HELPER, EMBER_MODULE_UNIFICATION, } from '@ember/canary-features'; import { assert } from '@ember/debug'; @@ -32,6 +33,7 @@ import { default as action } from './helpers/action'; import { default as array } from './helpers/array'; import { default as concat } from './helpers/concat'; import { default as eachIn } from './helpers/each-in'; +import { default as fn } from './helpers/fn'; import { default as get } from './helpers/get'; import { default as hash } from './helpers/hash'; import { inlineIf, inlineUnless } from './helpers/if-unless'; @@ -61,7 +63,11 @@ function makeOptions(moduleName: string, namespace?: string): LookupOptions { }; } -const BUILTINS_HELPERS = { +interface IBuiltInHelpers { + [name: string]: Helper | undefined; +} + +const BUILTINS_HELPERS: IBuiltInHelpers = { if: inlineIf, action, array, @@ -82,8 +88,13 @@ const BUILTINS_HELPERS = { '-mount': mountHelper, '-outlet': outletHelper, '-assert-implicit-component-helper-argument': componentAssertionHelper, + fn: undefined, }; +if (EMBER_GLIMMER_FN_HELPER) { + BUILTINS_HELPERS.fn = fn; +} + const BUILTIN_MODIFIERS = { action: { manager: new ActionModifierManager(), state: null }, }; @@ -96,9 +107,7 @@ export default class RuntimeResolver implements IRuntimeResolver(); - private builtInHelpers: { - [name: string]: Helper | undefined; - } = BUILTINS_HELPERS; + private builtInHelpers: IBuiltInHelpers = BUILTINS_HELPERS; private builtInModifiers: { [name: string]: ModifierDefinition; diff --git a/packages/@ember/-internals/glimmer/tests/integration/helpers/fn-test.js b/packages/@ember/-internals/glimmer/tests/integration/helpers/fn-test.js new file mode 100644 index 00000000000..03b81dfee1d --- /dev/null +++ b/packages/@ember/-internals/glimmer/tests/integration/helpers/fn-test.js @@ -0,0 +1,163 @@ +import { EMBER_GLIMMER_FN_HELPER } from '@ember/canary-features'; +import { Component } from '../../utils/helpers'; +import { RenderingTestCase, moduleFor, runTask } from 'internal-test-helpers'; + +import { set } from '@ember/-internals/metal'; + +if (EMBER_GLIMMER_FN_HELPER) { + moduleFor( + 'Helpers test: {{fn}}', + class extends RenderingTestCase { + beforeEach() { + this.registerHelper('invoke', function([fn]) { + return fn(); + }); + + let testContext = this; + this.registerComponent('stash', { + ComponentClass: Component.extend({ + init() { + this._super(...arguments); + testContext.stashedFn = this.stashedFn; + }, + }), + }); + } + + '@test updates when arguments change'() { + this.render(`{{invoke (fn this.myFunc this.arg1 this.arg2)}}`, { + myFunc(arg1, arg2) { + return `arg1: ${arg1}, arg2: ${arg2}`; + }, + + arg1: 'foo', + arg2: 'bar', + }); + + this.assertText('arg1: foo, arg2: bar'); + + this.assertStableRerender(); + + runTask(() => set(this.context, 'arg1', 'qux')); + this.assertText('arg1: qux, arg2: bar'); + + runTask(() => set(this.context, 'arg2', 'derp')); + this.assertText('arg1: qux, arg2: derp'); + + runTask(() => { + set(this.context, 'arg1', 'foo'); + set(this.context, 'arg2', 'bar'); + }); + + this.assertText('arg1: foo, arg2: bar'); + } + + '@test updates when the function changes'() { + let func1 = (arg1, arg2) => `arg1: ${arg1}, arg2: ${arg2}`; + let func2 = (arg1, arg2) => `arg2: ${arg2}, arg1: ${arg1}`; + + this.render(`{{invoke (fn this.myFunc this.arg1 this.arg2)}}`, { + myFunc: func1, + + arg1: 'foo', + arg2: 'bar', + }); + + this.assertText('arg1: foo, arg2: bar'); + this.assertStableRerender(); + + runTask(() => set(this.context, 'myFunc', func2)); + this.assertText('arg2: bar, arg1: foo'); + + runTask(() => set(this.context, 'myFunc', func1)); + this.assertText('arg1: foo, arg2: bar'); + } + + '@test a stashed fn result update arguments when invoked'(assert) { + this.render(`{{stash stashedFn=(fn this.myFunc this.arg1 this.arg2)}}`, { + myFunc(arg1, arg2) { + return `arg1: ${arg1}, arg2: ${arg2}`; + }, + + arg1: 'foo', + arg2: 'bar', + }); + + assert.equal(this.stashedFn(), 'arg1: foo, arg2: bar'); + + runTask(() => set(this.context, 'arg1', 'qux')); + assert.equal(this.stashedFn(), 'arg1: qux, arg2: bar'); + + runTask(() => set(this.context, 'arg2', 'derp')); + assert.equal(this.stashedFn(), 'arg1: qux, arg2: derp'); + + runTask(() => { + set(this.context, 'arg1', 'foo'); + set(this.context, 'arg2', 'bar'); + }); + + assert.equal(this.stashedFn(), 'arg1: foo, arg2: bar'); + } + + '@test a stashed fn result invokes the correct function when the bound function changes'( + assert + ) { + let func1 = (arg1, arg2) => `arg1: ${arg1}, arg2: ${arg2}`; + let func2 = (arg1, arg2) => `arg2: ${arg2}, arg1: ${arg1}`; + + this.render(`{{stash stashedFn=(fn this.myFunc this.arg1 this.arg2)}}`, { + myFunc: func1, + + arg1: 'foo', + arg2: 'bar', + }); + + assert.equal(this.stashedFn(), 'arg1: foo, arg2: bar'); + + runTask(() => set(this.context, 'myFunc', func2)); + assert.equal(this.stashedFn(), 'arg2: bar, arg1: foo'); + + runTask(() => set(this.context, 'myFunc', func1)); + assert.equal(this.stashedFn(), 'arg1: foo, arg2: bar'); + } + + '@test asserts if the first argument is not a function'() { + expectAssertion(() => { + this.render(`{{invoke (fn this.myFunc this.arg1 this.arg2)}}`, { + myFunc: null, + arg1: 'foo', + arg2: 'bar', + }); + }, /You must pass a function as the `fn` helpers first argument, you passed null/); + } + + '@test asserts if the provided function accesses `this` without being bound prior to passing to fn'() { + this.render(`{{stash stashedFn=(fn this.myFunc this.arg1)}}`, { + myFunc(arg1) { + return `arg1: ${arg1}, arg2: ${this.arg2}`; + }, + + arg1: 'foo', + arg2: 'bar', + }); + + expectAssertion(() => { + this.stashedFn(); + }, /You accessed `this.arg2` from a function passed to the `fn` helper, but the function itself was not bound to a valid `this` context. Consider updating to usage of `@action`./); + } + + '@test can use `this` if bound prior to passing to fn'(assert) { + this.render(`{{stash stashedFn=(fn (action this.myFunc) this.arg1)}}`, { + myFunc(arg1) { + return `arg1: ${arg1}, arg2: ${this.arg2}`; + }, + + arg1: 'foo', + arg2: 'bar', + }); + + assert.equal(this.stashedFn(), 'arg1: foo, arg2: bar'); + } + } + ); +} diff --git a/packages/@ember/canary-features/index.ts b/packages/@ember/canary-features/index.ts index 85727f20942..82ea75d4f09 100644 --- a/packages/@ember/canary-features/index.ts +++ b/packages/@ember/canary-features/index.ts @@ -22,6 +22,7 @@ export const DEFAULT_FEATURES = { EMBER_GLIMMER_ANGLE_BRACKET_NESTED_LOOKUP: true, EMBER_ROUTING_BUILD_ROUTEINFO_METADATA: true, EMBER_NATIVE_DECORATOR_SUPPORT: true, + EMBER_GLIMMER_FN_HELPER: null, }; /** @@ -88,3 +89,4 @@ export const EMBER_ROUTING_BUILD_ROUTEINFO_METADATA = featureValue( FEATURES.EMBER_ROUTING_BUILD_ROUTEINFO_METADATA ); export const EMBER_NATIVE_DECORATOR_SUPPORT = featureValue(FEATURES.EMBER_NATIVE_DECORATOR_SUPPORT); +export const EMBER_GLIMMER_FN_HELPER = featureValue(FEATURES.EMBER_GLIMMER_FN_HELPER);