From 5cfd92e1101054b36d3f7f2749cb710ddb215804 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Tue, 26 Dec 2023 13:16:30 -0500 Subject: [PATCH 1/7] Deprecate named inject --- text/1001-deprecate-named-inject.md | 64 +++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 text/1001-deprecate-named-inject.md diff --git a/text/1001-deprecate-named-inject.md b/text/1001-deprecate-named-inject.md new file mode 100644 index 0000000000..4be5d722b9 --- /dev/null +++ b/text/1001-deprecate-named-inject.md @@ -0,0 +1,64 @@ +--- +stage: accepted +start-date: +release-date: +release-versions: +teams: # delete teams that aren't relevant + - cli + - data + - framework + - learning + - steering + - typescript +prs: + accepted: # update this to the PR that you propose your RFC in +project-link: +--- + + + +# Deprecate named inject + +## Summary + +As of `ember-source@4.1` (and [RFC#752](https://github.com/emberjs/rfcs/pull/752)), `inject` is an old alias that's no longer needed + +## Motivation + +`import { service } from '@ember/service'` +makes more sense than +`import { inject as service } from '@ember/service'` + +This allows us to slim down our public API surface area to more of _what's needed_. + + +## Transition Path + +Most folks can do a mass find and replace switch from `inject as service` to just `service`. + +## How We Teach This + +The docs / guides already use the new import path. + +## Drawbacks + +n/a + +## Alternatives + +n/a + +## Unresolved questions + +n/a From f315ed38430ca82795c1a5c0044009395f3e4082 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Tue, 26 Dec 2023 13:18:10 -0500 Subject: [PATCH 2/7] update meta --- text/1001-deprecate-named-inject.md | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/text/1001-deprecate-named-inject.md b/text/1001-deprecate-named-inject.md index 4be5d722b9..5153dc2928 100644 --- a/text/1001-deprecate-named-inject.md +++ b/text/1001-deprecate-named-inject.md @@ -1,17 +1,13 @@ --- stage: accepted -start-date: +start-date: 2023-12-26T00:00:00.000Z release-date: release-versions: teams: # delete teams that aren't relevant - - cli - - data - framework - - learning - - steering - typescript prs: - accepted: # update this to the PR that you propose your RFC in + accepted: https://github.com/emberjs/rfcs/pull/1001 project-link: --- From b43bd86fbc64b09c2cf04c9b454e75bf2098d6cf Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Tue, 26 Dec 2023 13:19:53 -0500 Subject: [PATCH 3/7] Update 1001-deprecate-named-inject.md --- text/1001-deprecate-named-inject.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/1001-deprecate-named-inject.md b/text/1001-deprecate-named-inject.md index 5153dc2928..c62737c88e 100644 --- a/text/1001-deprecate-named-inject.md +++ b/text/1001-deprecate-named-inject.md @@ -28,7 +28,7 @@ project-link: Leave as is ## Summary -As of `ember-source@4.1` (and [RFC#752](https://github.com/emberjs/rfcs/pull/752)), `inject` is an old alias that's no longer needed +As of [`ember-source@4.1`](https://blog.emberjs.com/ember-4-1-released) (and [RFC#752](https://github.com/emberjs/rfcs/pull/752)), `inject` is an old alias that's no longer needed ## Motivation From b4302e5f6428c4234c74305cecd2b90753215a20 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Wed, 27 Dec 2023 11:14:07 -0500 Subject: [PATCH 4/7] Update text/1001-deprecate-named-inject.md Co-authored-by: Katie Gengler --- text/1001-deprecate-named-inject.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/1001-deprecate-named-inject.md b/text/1001-deprecate-named-inject.md index c62737c88e..fc1a5ce914 100644 --- a/text/1001-deprecate-named-inject.md +++ b/text/1001-deprecate-named-inject.md @@ -24,7 +24,7 @@ prs: project-link: Leave as is --> -# Deprecate named inject +# Deprecate named `inject` export from `@ember/service` ## Summary From 5d917671ed624aca6d5833064ec9d383198635e5 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Fri, 1 Mar 2024 15:25:35 -0500 Subject: [PATCH 5/7] Add sample codemod --- text/1001-deprecate-named-inject.md | 123 ++++++++++++++++++++++++++++ 1 file changed, 123 insertions(+) diff --git a/text/1001-deprecate-named-inject.md b/text/1001-deprecate-named-inject.md index fc1a5ce914..f7a1617f40 100644 --- a/text/1001-deprecate-named-inject.md +++ b/text/1001-deprecate-named-inject.md @@ -43,6 +43,129 @@ This allows us to slim down our public API surface area to more of _what's neede Most folks can do a mass find and replace switch from `inject as service` to just `service`. +An example codemod could look [something like this](https://astexplorer.net/#/gist/119f88339ea024e7cde63c71f52ce216/4d128a1239cbb56e00a69d3f710d67c20ed0e431) +```js +export const parser = 'ts' + +export default function transformer(file, api) { + const j = api.jscodeshift; + + const importNames = new Set(); + + const root = j(file.source); + + // find things we want to get rid of + root + .find(j.ImportSpecifier) + .forEach(path => { + if (path.node.imported.name === 'inject') { + importNames.add(path.node.local.name); + } + }) + + // now it's time to replace + root.find(j.ClassProperty).forEach(path => { + let node = path.node; + + let hasInject = hasDecorators(node, [...importNames.values()]); + + if (!hasInject) return; + + node.decorators = node.decorators.map(decorator => { + let { expression } = decorator; + + if (expression.type === 'Identifier') { + if (importNames.has(expression.name)) { + decorator.expression = j.identifier('service'); + } + } + + if (expression.type === 'CallExpression') { + decorator.expression.callee = j.identifier('service'); + } + + return decorator; + }); + }); + + return root.toSource(); +} + +// Copied from: https://github.com/NullVoxPopuli/ember-concurrency-codemods/tree/main +function firstMatchingDecorator(node, named = []) { + if (!node.decorators) return; + + return node.decorators.find((decorator) => { + let { expression } = decorator; + + switch (expression.type) { + case 'MethodDefinition': { + } + case 'CallExpression': { + let { callee } = expression; + + switch (callee.type) { + case 'Identifier': + return named.includes(callee.name); + case 'MemberExpression': { + let { object } = callee; + + return named.includes(object.callee.name); + } + } + } + case 'Identifier': + return named.includes(expression.name); + } + }); +} + +function hasDecorators(node, named = []) { + return Boolean(firstMatchingDecorator(node, named)); +} +``` + +
The test scenarios + +```ts +import { inject } from '@ember/service'; +import { inject as service } from '@ember/service'; +// import Service from '@ember/service'; +import BaseService from '@ember/service'; +import { inject as serviceDecorator } from '@ember/service'; +import { inject as x } from '@ember/service'; +// import { service } from '@ember/service'; +import { service as y } from '@ember/service'; +// import Service, { inject, service } from '@ember/service'; +import Service, { inject as s } from '@ember/service'; + + +export default class Demo extends Service { + +} + +export default class Demo2 extends BaseService { + // simple + @inject router; + @service router1; + @x router2; + @y router3; + @serviceDecorator router4; + @inject('router') router41; + + // TS-only + @inject declare router5: Type; + @inject('router') declare router51: Type; + @service declare router6: Type; + @x declare router7: Type; + @y declare router8: Type; + @serviceDecorator declare router9: Type; +} +``` + +
+ + ## How We Teach This The docs / guides already use the new import path. From 9624bb85485ac12d148a5ba53bf90cbf62551fb4 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Fri, 1 Mar 2024 15:26:45 -0500 Subject: [PATCH 6/7] Add alternative --- text/1001-deprecate-named-inject.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/text/1001-deprecate-named-inject.md b/text/1001-deprecate-named-inject.md index f7a1617f40..24c68a1187 100644 --- a/text/1001-deprecate-named-inject.md +++ b/text/1001-deprecate-named-inject.md @@ -176,7 +176,10 @@ n/a ## Alternatives -n/a +do nothing, the cost of an export alias is: +- a few extra bytes +- mental gymnastics for teaching +- "another case to cover" for tooling ## Unresolved questions From 1630c796f6295fa5b015a64d94b90cfb6cf09556 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Wed, 6 Mar 2024 11:00:11 -0500 Subject: [PATCH 7/7] Update 1001-deprecate-named-inject.md --- text/1001-deprecate-named-inject.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/text/1001-deprecate-named-inject.md b/text/1001-deprecate-named-inject.md index 24c68a1187..7f7e484625 100644 --- a/text/1001-deprecate-named-inject.md +++ b/text/1001-deprecate-named-inject.md @@ -172,7 +172,9 @@ The docs / guides already use the new import path. ## Drawbacks -n/a +As with any deprecation, we introduce an upgrade cliff for addons that are updated infrequently, and consequently their consuming apps. +As a mitigation, we could, for v1 addons, add an additional transform to ember-cli-babel to automatically upgrade `inject` from `@ember/service` to `service`. +This does narrow the range a bit, as `service` was introduced in ember-source@4.1, so libraries could not support from 3.28 to 6 (or whichever major ends up removing the `inject`) without adding `@embroider/macros` to conditionally import `inject` or `service` based on the consumer's ember-source version. ## Alternatives @@ -181,6 +183,9 @@ do nothing, the cost of an export alias is: - mental gymnastics for teaching - "another case to cover" for tooling +add a lint against `inject` +- all the downsides of the above ("do nothing") may still be present + ## Unresolved questions n/a