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

Move @ember/test-waiters to dependencies #993

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

gorner
Copy link

@gorner gorner commented Dec 10, 2024

Resolves #992 by moving @ember/test-waiters from package.json devDependencies to dependencies.

Confirmed this was fixed in my Ember 5.8 app that also uses ember-sortable (which requires a peer dependency of "@ember/test-waiters": ">= 3.0.1",) as follows:

  • Removed package-lock.json and node_modules/
  • Re-ran npm install (which generated a new package-lock with @ember/test-waiters v4 for ember-sortable)
  • Booted ember serve and saw the same error logged in 992
  • Replaced the ember-attacher entry in my package.json with "ember-attacher": "gorner/ember-attacher#move-test-waiters-to-deps", (i.e. this branch)
  • Ran ember serve again and saw everything was working as expected

A few notes:

  • Although there is now a v4 of test-waiters, I've stuck with v3 as there are breaking changes that require dropping support for Ember <4.0, which would imply a major version bump to ember-attacher. This will probably be needed in the near future, but it doesn't appear to be needed to resolve the current issue.
  • The ember-try run for ember-release (Ember 6.0) fails due to breaking changes, specifically the requirement to switch to component template co-location. I see there is a separate issue for this already (Component Templates Co-location and Typescript #978) so for now I am marking the ember-release run as allowedToFail: true, adding a separate scenario for Ember 5.12, and indicating in the README that the addon is not yet compatible with Ember 6+.
  • In the issue I noted that it might make sense to use peerDependencies instead so app developers could specify the version of test-waiters that works for them. For now it doesn't really make a difference because this package only works with v3 of test-waiters anyway. It might make more sense once the Ember 6.x compatibility issues are cleared.

@gorner gorner marked this pull request as ready for review December 10, 2024 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

@ember/test-waiters should be a non-dev dependency or peerDependency
1 participant