Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement RFC#1006, Deprecate (action) and {{action}} #20653

Merged
merged 8 commits into from
Mar 28, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions packages/@ember/-internals/deprecations/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,15 @@ export const DEPRECATIONS = {
until: '6.0.0',
url: 'https://deprecations.emberjs.com/v5.x/#toc_deprecate-implicit-route-model',
}),
DEPRECATE_TEMPLATE_ACTION: deprecation({
id: 'template-action',
url: 'https://deprecations.emberjs.com/id/template-action',
until: '6.0.0',
for: 'ember-source',
since: {
available: '5.9.0',
},
}),
};

export function deprecateUntil(message: string, deprecation: DeprecationObject) {
Expand Down
5 changes: 5 additions & 0 deletions packages/@ember/-internals/glimmer/lib/helpers/action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import { get } from '@ember/-internals/metal';
import type { AnyFn } from '@ember/-internals/utility-types';
import { assert } from '@ember/debug';
import { DEPRECATIONS, deprecateUntil } from '@ember/-internals/deprecations';
import { flaggedInstrument } from '@ember/instrumentation';
import { join } from '@ember/runloop';
import { DEBUG } from '@glimmer/env';
Expand Down Expand Up @@ -278,6 +279,10 @@ export const ACTIONS = new WeakSet();
@public
*/
export default internalHelper((args: CapturedArguments): Reference<Function> => {
deprecateUntil(
`Usage of the \`(action)\` helper is deprecated. Migrate to native functions and function invocation.`,
DEPRECATIONS.DEPRECATE_TEMPLATE_ACTION
);
let { named, positional } = args;
// The first two argument slots are reserved.
// pos[0] is the context (or `this`)
Expand Down
5 changes: 5 additions & 0 deletions packages/@ember/-internals/glimmer/lib/modifiers/action.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { InternalOwner } from '@ember/-internals/owner';
import { DEPRECATIONS, deprecateUntil } from '@ember/-internals/deprecations';
import { uuid } from '@ember/-internals/utils';
import { ActionManager, EventDispatcher, isSimpleClick } from '@ember/-internals/views';
import { assert } from '@ember/debug';
Expand Down Expand Up @@ -204,6 +205,10 @@ class ActionModifierManager implements InternalModifierManager<ActionState, obje
}

install(actionState: ActionState): void {
deprecateUntil(
`Usage of the \`{{action}}\` modifier is deprecated. Migrate to native functions and function invocation.`,
DEPRECATIONS.DEPRECATE_TEMPLATE_ACTION
);
let { element, actionId, positional } = actionState;

let actionName;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,16 @@
import { moduleFor, ApplicationTestCase, RenderingTestCase, runTask } from 'internal-test-helpers';
import {
testUnless,
moduleFor,
ApplicationTestCase,
RenderingTestCase,
runTask,
} from 'internal-test-helpers';

import Controller from '@ember/controller';
import { getDebugFunction, setDebugFunction } from '@ember/debug';

import { Component } from '../../utils/helpers';
import { DEPRECATIONS } from '../../../../deprecations';

const originalDebug = getDebugFunction('debug');
const noop = function () {};
Expand All @@ -14,16 +21,21 @@ moduleFor(
constructor() {
setDebugFunction('debug', noop);
super(...arguments);

expectDeprecation(
/Usage of the `\{\{action\}\}` modifier is deprecated./,
DEPRECATIONS.DEPRECATE_TEMPLATE_ACTION.isEnabled
);
}

teardown() {
setDebugFunction('debug', originalDebug);
}

['@test actions in top level template application template target application controller'](
assert
) {
assert.expect(1);
[`${testUnless(
DEPRECATIONS.DEPRECATE_TEMPLATE_ACTION.isRemoved
)} actions in top level template application template target application controller`](assert) {
assert.expect(2);

this.add(
'controller:application',
Expand All @@ -46,8 +58,10 @@ moduleFor(
});
}

['@test actions in nested outlet template target their controller'](assert) {
assert.expect(1);
[`${testUnless(
DEPRECATIONS.DEPRECATE_TEMPLATE_ACTION.isRemoved
)} actions in nested outlet template target their controller`](assert) {
assert.expect(2);

this.add(
'controller:application',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { DEBUG } from '@glimmer/env';
import { moduleFor, RenderingTestCase, applyMixins, strip, runTask } from 'internal-test-helpers';

import { isEmpty } from '@ember/utils';
import { action } from '@ember/object';
import { A as emberA } from '@ember/array';

import { Component } from '../../utils/helpers';
Expand Down Expand Up @@ -773,18 +774,16 @@ moduleFor(

this.registerComponent('my-action-component', {
ComponentClass: Component.extend({
actions: {
changeValue() {
this.incrementProperty('myProp');
},
},
changeValue: action(function () {
this.incrementProperty('myProp');
}),
}),
template: strip`
{{#my-component my-attr=this.myProp as |api|}}
{{api.my-nested-component}}
{{/my-component}}
<br>
<button onclick={{action 'changeValue'}}>Change value</button>`,
<button onclick={{this.changeValue}}>Change value</button>`,
});

this.render('{{my-action-component myProp=this.model.myProp}}', {
Expand Down Expand Up @@ -839,13 +838,12 @@ moduleFor(
['@test parameters in a contextual component are mutable when value is a param'](assert) {
// This checks that a `(mut)` is added to parameters and attributes to
// contextual components when it is a param.

this.registerComponent('change-button', {
ComponentClass: Component.extend().reopenClass({
positionalParams: ['val'],
}),
template: strip`
<button {{action (action (mut this.val) 10)}} class="my-button">
<button {{on "click" (fn (mut this.val) 10)}} class="my-button">
Change to 10
</button>`,
});
Expand Down Expand Up @@ -892,15 +890,13 @@ moduleFor(
this.registerComponent('outer-component', {
ComponentClass: Component.extend({
message: 'hello',
actions: {
change() {
this.set('message', 'goodbye');
},
},
change: action(function () {
this.set('message', 'goodbye');
}),
}),
template: strip`
message: {{this.message}}{{inner-component message=this.message}}
<button onclick={{action "change"}} />`,
<button onclick={{this.change}} />`,
});

this.registerComponent('inner-component', {
Expand Down Expand Up @@ -1447,7 +1443,7 @@ class MutableParamTestGenerator {
positionalParams: ['val'],
}),
template: strip`
<button {{action (action (mut this.val) 10)}} class="my-button">
<button {{on "click" (fn (mut this.val) 10)}} class="my-button">
Change to 10
</button>`,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@ import {
equalsElement,
runTask,
runLoopSettled,
testUnless,
} from 'internal-test-helpers';

import { action } from '@ember/object';
import { run } from '@ember/runloop';
import { DEBUG } from '@glimmer/env';
import { tracked } from '@ember/-internals/metal';
Expand All @@ -20,6 +22,7 @@ import { A as emberA } from '@ember/array';

import { Component, compile, htmlSafe } from '../../utils/helpers';
import { backtrackingMessageFor } from '../../utils/debug-stack';
import { DEPRECATIONS } from '../../../../deprecations';

moduleFor(
'Components test: curly components',
Expand Down Expand Up @@ -1441,25 +1444,23 @@ moduleFor(
componentInstance = this;
},

actions: {
click() {
let currentCounter = this.get('counter');
myClick: action(function () {
let currentCounter = this.get('counter');

assert.equal(currentCounter, 0, 'the current `counter` value is correct');
assert.equal(currentCounter, 0, 'the current `counter` value is correct');

let newCounter = currentCounter + 1;
this.set('counter', newCounter);
let newCounter = currentCounter + 1;
this.set('counter', newCounter);

assert.equal(
this.get('counter'),
newCounter,
"getting the newly set `counter` property works; it's equal to the value we just set and not `undefined`"
);
},
},
assert.equal(
this.get('counter'),
newCounter,
"getting the newly set `counter` property works; it's equal to the value we just set and not `undefined`"
);
}),
}),
template: `
<button {{action "click"}}>foobar</button>
<button {{on "click" this.myClick}}>foobar</button>
`,
});

Expand Down Expand Up @@ -3146,9 +3147,16 @@ moduleFor(
runTask(() => set(this.context, 'foo', 5));
}

['@test returning `true` from an action does not bubble if `target` is not specified (GH#14275)'](
[`${testUnless(
DEPRECATIONS.DEPRECATE_TEMPLATE_ACTION.isRemoved
)} returning \`true\` from an action does not bubble if \`target\` is not specified (GH#14275)`](
assert
) {
expectDeprecation(
/Usage of the `\{\{action\}\}` modifier is deprecated./,
DEPRECATIONS.DEPRECATE_TEMPLATE_ACTION.isEnabled
);

this.registerComponent('display-toggle', {
ComponentClass: Component.extend({
actions: {
Expand All @@ -3173,8 +3181,15 @@ moduleFor(
runTask(() => this.$('button').click());
}

['@test returning `true` from an action bubbles to the `target` if specified'](assert) {
assert.expect(4);
[`${testUnless(
DEPRECATIONS.DEPRECATE_TEMPLATE_ACTION.isRemoved
)} returning \`true\` from an action bubbles to the \`target\` if specified`](assert) {
assert.expect(5);

expectDeprecation(
/Usage of the `\{\{action\}\}` modifier is deprecated./,
DEPRECATIONS.DEPRECATE_TEMPLATE_ACTION.isEnabled
);

this.registerComponent('display-toggle', {
ComponentClass: Component.extend({
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { DEBUG } from '@glimmer/env';
import { moduleFor, RenderingTestCase, strip, runTask } from 'internal-test-helpers';
import { moduleFor, RenderingTestCase, strip, runTask, testUnless } from 'internal-test-helpers';

import { set, computed } from '@ember/object';

import { Component } from '../../utils/helpers';
import { backtrackingMessageFor } from '../../utils/debug-stack';
import { DEPRECATIONS } from '../../../../deprecations';

moduleFor(
'Components test: dynamic components',
Expand Down Expand Up @@ -429,7 +430,9 @@ moduleFor(
this.assertText('foo-bar Caracas Caracas arepas!');
}

['@test component helper with actions'](assert) {
[`${testUnless(
DEPRECATIONS.DEPRECATE_TEMPLATE_ACTION.isRemoved
)} component helper with actions`](assert) {
this.registerComponent('inner-component', {
template: 'inner-component {{yield}}',
ComponentClass: Component.extend({
Expand All @@ -449,6 +452,11 @@ moduleFor(
}),
});

expectDeprecation(
/Usage of the `\(action\)` helper is deprecated./,
DEPRECATIONS.DEPRECATE_TEMPLATE_ACTION.isEnabled
);

let actionTriggered = 0;
this.registerComponent('outer-component', {
template:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
import { moduleFor, RenderingTestCase, runDestroy, runTask } from 'internal-test-helpers';
import {
moduleFor,
RenderingTestCase,
runDestroy,
runTask,
testUnless,
} from 'internal-test-helpers';
import { set } from '@ember/object';
import { DEPRECATIONS } from '../../../../deprecations';

class InputRenderingTest extends RenderingTestCase {
$input() {
Expand Down Expand Up @@ -280,10 +287,16 @@ moduleFor(
// this.assertSelectionRange(8, 8); //NOTE: this fails in IE, the range is 0 -> 0 (TEST_SUITE=sauce)
}

['@test sends an action with `<Input @enter={{action "foo"}} />` when <enter> is pressed'](
[`${testUnless(
DEPRECATIONS.DEPRECATE_TEMPLATE_ACTION.isRemoved
)} sends an action with \`<Input @enter={{action "foo"}} />\` when <enter> is pressed`](
assert
kategengler marked this conversation as resolved.
Show resolved Hide resolved
) {
assert.expect(2);
assert.expect(3);
expectDeprecation(
/Usage of the `\(action\)` helper is deprecated./,
DEPRECATIONS.DEPRECATE_TEMPLATE_ACTION.isEnabled
);

this.render(`<Input @enter={{action 'foo'}} />`, {
actions: {
Expand All @@ -302,12 +315,10 @@ moduleFor(
['@test sends `insert-newline` when <enter> is pressed'](assert) {
assert.expect(2);

this.render(`<Input @insert-newline={{action 'foo'}} />`, {
actions: {
foo(value, event) {
assert.ok(true, 'action was triggered');
assert.ok(event instanceof Event, 'Native event was passed');
},
this.render(`<Input @insert-newline={{this.foo}} />`, {
foo(value, event) {
assert.ok(true, 'action was triggered');
assert.ok(event instanceof Event, 'Native event was passed');
},
});

Expand All @@ -316,10 +327,16 @@ moduleFor(
});
}

['@test sends an action with `<Input @escape-press={{action "foo"}} />` when <escape> is pressed'](
[`${testUnless(
DEPRECATIONS.DEPRECATE_TEMPLATE_ACTION.isRemoved
)} sends an action with \`<Input @escape-press={{action "foo"}} />\` when <escape> is pressed`](
assert
) {
assert.expect(2);
assert.expect(3);
expectDeprecation(
/Usage of the `\(action\)` helper is deprecated./,
DEPRECATIONS.DEPRECATE_TEMPLATE_ACTION.isEnabled
);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this test be converted to using functions?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair enough, updated!

this.render(`<Input @escape-press={{action 'foo'}} />`, {
actions: {
Expand Down
Loading
Loading