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.

Sign up for GitHub

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

Deprecate named inject export from @ember/service #1001

Merged
merged 8 commits into from
Mar 8, 2024
Merged

Deprecate named inject export from @ember/service #1001

Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5cfd92e
Deprecate named inject
NullVoxPopuli Dec 26, 2023
f315ed3
update meta
NullVoxPopuli Dec 26, 2023
b43bd86
Update 1001-deprecate-named-inject.md
NullVoxPopuli Dec 26, 2023
b4302e5
Update text/1001-deprecate-named-inject.md
NullVoxPopuli Dec 27, 2023
5d91767
Add sample codemod
NullVoxPopuli Mar 1, 2024
9624bb8
Add alternative
NullVoxPopuli Mar 1, 2024
1630c79
Update 1001-deprecate-named-inject.md
NullVoxPopuli Mar 6, 2024
a0b8c23
Merge branch 'master' into deprecate-named-inject
ef4 Mar 8, 2024
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
60 changes: 60 additions & 0 deletions text/1001-deprecate-named-inject.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
---
stage: accepted
start-date: 2023-12-26T00:00:00.000Z
release-date:
release-versions:
teams: # delete teams that aren't relevant
- framework
- typescript
prs:
accepted: https://github.com/emberjs/rfcs/pull/1001
project-link:
---

<!---
Directions for above:

stage: Leave as is
start-date: Fill in with today's date, 2032-12-01T00:00:00.000Z
release-date: Leave as is
release-versions: Leave as is
teams: Include only the [team(s)](README.md#relevant-teams) for which this RFC applies
prs:
accepted: Fill this in with the URL for the Proposal RFC PR
project-link: Leave as is
-->

# Deprecate named inject

## Summary

As of [`[email protected]`](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

`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
Copy link
Member

Choose a reason for hiding this comment

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

I'm surprised that there are no listed drawbacks here. Indeed, you can easily change this in your app code, but if this is being imported by an addon that you don't control, there will be a cliff of ember versions that a non-updated addon can be supported on.

This is probably true generally, but it seems like a low-value artificial cliff to do this just for an export alias.

I ran a code search on emberobserver.com and it gives us lots of results for this usage: 714 addons (6562 usages)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need better filters on emberobserver 🤔 when I was doing all the research for #1003, I found that most usages were in addons that were no longer used, updated, or not compatible with a version of ember that would even see the deprecation. Not saying that this is the case (I see the same results, btw), but I just... can't know right now -- I should see what it would take to add some more filtering to code search on emberobserver.

This is probably true generally

aye, this is every deprecation, really.

like a low-value artificial cliff to do this just for an export alias.

as an alternative, we can auto-upgrade v1 addons via ember-cli-babel -- thoughts?
v2 addons, who don't use ember-cli-babel, are either already using plain service, or can quickly update

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a note here on drawbacks, and I think, in doing so, I've talked myself (or you helped me talk myself) into deferring this deprecation until Ember 6.1, targeting for removal in 7.

That gives addon authors a massive support range, should they choose to do so, of 3.28 to 6.12.x, with v7 dropping inject, making any folks with 3.28 support have to either drop it (supporting 4.1 as a minimum), or adding @embroider/macros to keep support.

This also gives the addon ecosystem a year and a half to decide how they want to support inject / service throughout the entirety of the v6 series.

Copy link
Member

Choose a reason for hiding this comment

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

If we implement deprecation staging, we can enable the deprecation sooner even if it targets 7.0.


## Alternatives
Copy link
Member

Choose a reason for hiding this comment

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

a reasonable alternative would be to add a lint against it in the default lint set, essentially nudging people towards not needing it 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, that could happen now, actually, without an RFC


n/a

## Unresolved questions

n/a
Loading