search

emberjs / ember-test-helpers

Test-framework-agnostic helpers for testing Ember.js applications
Apache License 2.0
188 stars 254 forks source link

@ember/test-helpers v4 suggests itself to be in dependencies even when only used for test-support files #1490

Closed elwayman02 closed 1 week ago

elwayman02 commented 2 weeks ago

As of the v4 release, this package now throws an error if used in the addon-test-support/ directory as a devDependency:

Build Error (WebpackBundler)

ember-scroll-modifiers tried to import "@ember/test-helpers" in "ember-scroll-modifiers/test-support/did-intersect-mock.js" from addon code, but "@ember/test-helpers" is a devDependency. You may need to move it into dependencies.

The correct pattern here is to add @ember/test-helpers to peerDependencies for the package that utilizes it in addon-test-support/, because we do not want to cause @ember/test-helpers to be included in production bundles for consuming applications, but we do want to ensure that @ember/test-helpers is properly installed as a devDependency for any consumers of the package providing the test-support util, since the util imports @ember/test-helpers and will fail if the consumer doesn't have it installed.

The error message itself is incorrect: addon-test-support is, by definition, not "addon code", but rather code for supporting tests only. This will confuse developers who will try to move it to dependencies and erroneously add it to production builds for their consumers. The error message should be updated to be more correct and suggest adding it to peerDependencies, since we know where it's being used is not actually addon code but is(maybe) going to be used by consumers.

NullVoxPopuli commented 2 weeks ago

Looks like you solved it? https://github.com/elwayman02/ember-scroll-modifiers/pull/1106/files

in a v1 addon you need both peer and devDep

elwayman02 commented 2 weeks ago

@NullVoxPopuli I updated the issue a bit ago to be more clear what the problem is - peerDep is working but the error message does not correctly suggest that as a solution, even though it has enough information to do so.

NullVoxPopuli commented 2 weeks ago

gotchya -- for an issue with the error message, I think you'd want to open an issue on auto-import or embroider

NullVoxPopuli commented 1 week ago

Just had some successful upgrades here:

I'm going to close issue, because it seems like the issue you're encountering is elsewhere. Please let me know if you want help debugging or anything.