search

embroider-build / ember-auto-import

Zero config import from npm packages
Other
360 stars 109 forks source link

Code imported from within addon-test-support dir of addon being included in production build #539

Open andrew-paterson opened 2 years ago

andrew-paterson commented 2 years ago

I'm using an Ember addon in several Ember apps. The addon has a non-Ember npm package listed as a dependency. This contains a large collection of mocked JSON responses as JavaScript exports, for use in tests.

The addon-test-support directory of the ember-addon, has several reusable test modules which import modules from npm-package in order to use as data stubs for tests.

These test modules are in turn imported by the test files in the consuming Ember apps.

My understanding is that any import that happens in a file in the addon-test-support directory of the addon should be added to the test-support file during a build, but this is not happening.

The dependency structure is as follows:

ember-addon has ember-auto-import v2.4.2 listed in dependencies and webpack v 5.74.0 in `devDependencies.

Currently, all of the modules imported from npm-modules are being included in the vendor file, even though they are only ever imported from inside the addon-test-support directory of ember-addon.

Build Error (WebpackBundler)

ember-addon tried to import "npm-module" in "ember-addon/test-support/test-functions/acceptance/ngs/full-results.js" from addon code, but "npm-module" is a devDependency. You may need to move it into dependencies.

Is this the expected behaviour, and if so, am I missing something in the docs?

ef4 commented 2 years ago

What precisely do you mean by "vendor file"? There is probably a test-specific vendor chunk, and that would be the correct place for those dependencies to appear. That file shouldn't be loaded by the app except when the app is running its tests.

andrew-paterson commented 2 years ago

I should have mentioned that this is when building for production.

Here is the result of ember build --environment=production with the above setup:

 - dist/assets/chunk.106.a7a13fd403b935d0df8b.js: 60.26 MB (3.96 MB gzipped)
 - dist/assets/chunk.143.0ad30597611569a437b9.js: 2.07 KB (987 B gzipped)
 - dist/assets/chunk.178.372c546b03cb64338a86.js: 1.96 KB (944 B gzipped)
 - dist/assets/chunk.777.7337689689dec2caee17.js: 170.19 KB (54.57 KB gzipped)
 - dist/assets/dynamic-endpoint.js: 37 B (57 B gzipped)
 - dist/assets/header.css: 1.46 KB (516 B gzipped)
 - dist/assets/ember-app-1-5f8593e29ada14fb56f9db321487c03c.js: 249.75 KB (16.09 KB gzipped)
 - dist/assets/ember-app-1-6ab06a4a2141cf5710bcdd66925211a6.css: 172.35 KB (40.73 KB gzipped)
 - dist/assets/ember-app-1-tests-5d0223cb6c9e087f98a462fe5bbb26a6.css: 213.33 KB (42.23 KB gzipped)
 - dist/assets/vendor-3b28780443d8e77e94b02e998f930278.css: 2.64 KB (798 B gzipped)
 - dist/assets/vendor-6941e5cbaaa96b48688d7c34cfb364d3.js: 2.67 MB (557.1 KB gzipped)

If I move all the contents of the npm package into the addon-test-support dir of the addon itself, remove the npm package as a dependency of the addon and update the imports accordingly (All of which are also in the addon-test-support dir) then the result of ember build --environment=production is as follows:

 - dist/assets/chunk.143.4e1b08f03204d8d11fea.js: 2.07 KB (988 B gzipped)
 - dist/assets/chunk.178.aee7978274d720d9ef74.js: 610 B (316 B gzipped)
 - dist/assets/chunk.777.7337689689dec2caee17.js: 170.19 KB (54.57 KB gzipped)
 - dist/assets/dynamic-endpoint.js: 37 B (57 B gzipped)
 - dist/assets/header.css: 1.46 KB (516 B gzipped)
 - dist/assets/ember-app-1-5f8593e29ada14fb56f9db321487c03c.js: 249.75 KB (16.09 KB gzipped)
 - dist/assets/ember-app-1-6ab06a4a2141cf5710bcdd66925211a6.css: 172.35 KB (40.73 KB gzipped)
 - dist/assets/ember-app-1-tests-5d0223cb6c9e087f98a462fe5bbb26a6.css: 213.33 KB (42.23 KB gzipped)
 - dist/assets/vendor-3b28780443d8e77e94b02e998f930278.css: 2.64 KB (798 B gzipped)
 - dist/assets/vendor-6941e5cbaaa96b48688d7c34cfb364d3.js: 2.67 MB (557.1 KB gzipped)

The first result includes a 60.26MB chunk which I have confirmed contains all of the test data, originally stored in the npm package.

That 60MB chunk is wreaking havoc with the build process, causing the build to give up due to an out-of-memory error, unless I include ie11 as one of the targets, which is weird.

I was under the impression that I should get the same result either way as I'm importing the data from files within the addon-test-support directory, but when they're being imported from a non-ember dependency they are included in the prod build.

andrew-paterson commented 2 years ago

In case it helps, I've reproduced the behaviour in this repo: git@github.com:andrew-paterson/ember-app.git.

The README outlines the two scenarios that occur with two different versions of ember-addon one which imports test stubs from a non ember package and one in which those stubs are stored in the addon-test-support directory of ember-addon.

I'm running the app in node v12.16.3

apellerano-pw commented 12 months ago

Regardless of how a project's tests and dependencies are structured, why does ember-auto-import generate a tests chunk when --environment=production is used? Ember itself won't generate the dist/tests/index.html file that would import it. So the production build process could skip bundling the test chunk entirely.

We have a similar situation where using --environment=production generates a large chunk containing only test data. We have to manually remove it from the dist folder to save considerably on deploy artifact size; but the build time has already been spent by then.

We are using the technique described in this issue to reliably know the name of the test chunk. With a default ember-auto-import configuration it would have a randomly assigned name and I'm not sure how you'd know which chunk is the test chunk without opening the files and parsing their contents. I guess you could also parse index.html and discover it via process of elimination, in some scenarios.

But if you pin the name of the file, you can do something like this at the end of ember-cli-build.js:


const exclude = environment !== 'production'
  ? []
  : [
    'assets/ember-auto-import.tests.js',
    'assets/ember-auto-import.tests.map', // probably
  ];
return new Funnel(app.toTree(), {
  exclude,
});