search

embroider-build / ember-auto-import

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

Improved layering between app and tests bundles #620

Closed mansona closed 5 months ago

mansona commented 7 months ago

Fixes https://github.com/embroider-build/ember-auto-import/issues/503 Closes https://github.com/embroider-build/ember-auto-import/pull/512

ef4 commented 5 months ago

@simonihmig and I pushed this forward and have passing tests. I published it as 2.7.3-alpha.0. @simonihmig is going to test it under real-app conditions before we merge.

We think this is a minimum set of changes to fix the bug, but it leaves undone a lot of cleanup refactoring that could eliminate unused functionality in ember-auto-import and simplify the codebase. Essentially now we are trusting webpack to manage which deps get into app vs tests, so all the places where ~embroider~ ember-auto-import is trying to manage that ahead of webpack are unnecessary now.

simonihmig commented 5 months ago

I tested this on our monorepo with many hundreds of (test-)apps and >150 instances of workarounds for failing tests due to this bug. After reverting those workarounds and running CI with the alpha release, >99% of apps were passing!

The remaining failures were related to the more rare case when an app does not have any eai-handled imports, only the tests have. In that case there wouldn't be any app script, and with the previous implementation no chunks would be inserted into the index.html. This was fixed with 730270c196defb86b0cf04c6fdf2ff6a65764b3a, which I just pushed into this branch.

:tada: With that I have a perfectly green build now! :tada: