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

Use the types published from Ember itself; require Ember.js v4.8.4 or above #473

Merged
merged 3 commits into from
Dec 22, 2022

Conversation

chriskrycho
Copy link
Collaborator

  • Specify a peerDependencies requirement of Ember 4.8.
  • Drop the corresponding @types packages.

@chriskrycho chriskrycho added enhancement New feature or request breaking labels Dec 22, 2022
@chriskrycho chriskrycho mentioned this pull request Dec 22, 2022
6 tasks
- Specify a `peerDependencies` requirement of Ember 4.8.
- Drop the corresponding `@types` packages.
@@ -67,8 +67,7 @@ jobs:
fail-fast: false
matrix:
try-scenario:
- ember-lts-3.28
- ember-lts-4.4
Copy link
Contributor

@simonihmig simonihmig Jun 2, 2023

Choose a reason for hiding this comment

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

Sorry for hijacking this older PR for a new discussion, but I was just trying to figure out if upgrading to the new major version in another addon (cc @NullVoxPopuli) can be done without a breaking change there. And also I wanted to see how addons can consume the new native types while supporting older Ember versions...

So, the Readme states that Ember 3.28 is still supported. While this change suggests otherwise. You could argue that it is supported but not tested, in which case I would argue anything that is not tested is not supported! So what is true? 😏

Unrelated to this specific project, is there any other way addons can start to consume the preview types, while still having test coverage for older Ember versions that do not have them? I guess we could make ember-try add all those @types packages, but then import 'ember-source/types' is likely to break, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just saw this as I was clearing out my inbox! There are two things to note here—in order of importance:

  1. The only way to test that kind of interop correctly is to have two CI passes with their own dependencies. The cleanest/easiest way to do that is with two separate test apps which just have their own dependencies, but you could also imagine doing it by specifying a list of dependencies to install and doing a yarn add -D <that list of deps> in separate CI runs. The key is that they need some degree of isolation from each other so they don’t stomp on each other.

  2. It actually turns out that you don’t have to worry about the side-effect imports: import 'ember-source/types'; is actually always allowed as long as ember-source exists. Yes, this is a footgun: it will totally silently fail if that is not present, with the surfacing symptom being type checking errors about not finding @ember/<whatever> if the types are not there. That means that you could have a single test app that covers both… but that would be pretty unclear, so I would not recommend it. 😂

As for compatibility with native types for this add-on in particular without coupling it to a breaking change, it is likely possible in principle by taking advantage of (1), so I'd be happy to merge PRs that supported it for versions as far back as 0.5.x.

SergeAstapov added a commit that referenced this pull request Sep 2, 2023
This was made as part of #473 and was part of v1.0.0 release but README.md update was missed
@SergeAstapov SergeAstapov changed the title Use the types published from Ember itself Use the types published from Ember itself; require Ember.js v4.8.4 or above Sep 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants