ember-auto-import Restructure entry file bundle logic to support "layering"

Currently, ember-auto-import has two conceptual buckets where it places the imports it has found (an app bucket and a tests bucket). While this works, it has two potential problems. The first is with imports from within lazy engines as they get placed inside of the "app" bucket. This means that instead of these asset loading lazily they are instead eagerly loaded as part of the main app bundle. The second problem is that modules can be "duplicated" in each bucket and can cause problems with module states' being different depending on the importer.

In order to fix this, this PR creates an individual entry file (or bucket) for every addon that is doing an import. It then creates import links from each entry file to its rightful parent (ie a nested addon from a lazy engine would only be imported from the lazy engine's entry file and then the lazy engine's entry file is import from the app's entry file dynamically). Additionally, the tests entry file now imports the app entry file which creates a conceptual graph for webpack to properly traverse such that modules will not be duplicated.

Fixes: #503

Mar 29 '22 22:03 thoov

@ef4: When you have a chance could you review this?

Mar 31 '22 16:03 thoov

@thoov the above commit adds the failure case I mentioned. It does indeed break.

Mar 31 '22 19:03 ef4

@ef4 could you review this again. I got the test case you added to pass using dependsOn.

Apr 02 '22 23:04 thoov

@ef4 Could you review this again?

Apr 17 '22 08:04 thoov

@ef4 friendly ping again on this

Apr 21 '22 20:04 thoov

This will need to land first: https://github.com/ember-engines/ember-asset-loader/pull/127

Apr 25 '22 21:04 thoov

ember-asset-loader is released, but it is a transitive dependency of ember-engines. I tried to use NPM overrides here to get CI passing, but apparently NPM overrides doesn't actually work when you're also using workspaces. 😭https://github.com/npm/cli/issues/4834

May 16 '22 14:05 ef4

I am happy to land this but somebody needs to get on ember-engines to release.

Jun 22 '22 13:06 ef4

@ef4 I'm looking to get this pushed forward as we need this resolved for our application, I can create PRs as necessary to push this forward from my side.

I just want to confirm my understanding of what needs to happen:

The ember-asset-loader PR was merged and has landed in 1.0.0 of that repo. ✅
ember-engines needs to bump its dependency on ember-asset-loader to 1.0.0 since it is still on 0.6.1
We can then check in this PR

Sep 30 '22 18:09 lbdm44

I can handle an ember-engines PR/release cc @SergeAstapov. I'll probably PR this over night tonight if you might have time to review tomorrow?

Oct 20 '22 22:10 runspired

I can handle an ember-engines PR/release cc @SergeAstapov. I'll probably PR this over night tonight if you might have time to review tomorrow?

There is PR already, need also get https://github.com/ember-engines/ember-engines/pull/818 which I assume no brained but would require v0.9.0 (due to Node.js req version bump)

Oct 20 '22 23:10 SergeAstapov

looks like we're ready to move forward? 🎉

Oct 21 '22 16:10 NullVoxPopuli

@NullVoxPopuli yes, after we solve test failures in https://github.com/ember-engines/ember-engines/pull/820

Oct 21 '22 16:10 SergeAstapov

Looks like ember-engines/ember-engines#820 has gone in, any ideas on what needs to be done to get this ✅?

Nov 07 '22 19:11 lbdm44

the blocker in EmberEngines has been fixed and released as 0.9 https://github.com/ember-engines/ember-engines/releases/tag/v0.9.0

Nov 17 '22 18:11 runspired

should this be rebased? the diff seems sus:

esp as this PR didn't initially change any dependenices

Nov 23 '22 23:11 NullVoxPopuli

@NullVoxPopuli probably deps need recalculated but a diff is expected: https://github.com/ef4/ember-auto-import/pull/512/files#diff-fbd8a1b3659adac1c6d2d635ea65aa1cf0682939f6f767a03039c63ad5ca4f7bR39

Nov 23 '22 23:11 runspired

@rwjblue / @ef4 I rebased and get the tests suite to pass again. Any chance to land this and get a version released?

Feb 13 '23 18:02 thoov

@ef4 can we get eyes on this - I hear you are the only one that can release this change once merged?

Feb 14 '23 20:02 Kaceykaso

+1 This is blocking multiple projects at our company.

Feb 15 '23 18:02 lbdm44

+1 would be great to have this merged and published it is blocking our consumption of v2 addons.

Feb 16 '23 00:02 gabrielcsapo

@ef4 this causes issues in engines, specially engine service dependencies we might need to test that for service injections.

Mar 22 '23 19:03 gabrielcsapo

By this do you mean my last commit that turns off the removeAvailableModules optimization in development?

Mar 23 '23 13:03 ef4

I've been picking up this work, and the implementation here is still not correct.

We shouldn't need to do any manual deduplication between test and app, because that's the point of the layering.

And we shouldn't be inserting any app chunks into the tests entrypoint, because again that's the point of layering them together inside code that webpack manages.

Hopefully I will have some time in the next couple weeks to try to fix this.

Mar 30 '23 13:03 ef4

@ef4 any progress here?

Aug 14 '23 05:08 runspired

Putting this on the embroider team agenda for tomorrow, with the intention of identifying who can pick this up and how soon. It's still a priority to get this fixed.

Aug 14 '23 14:08 ef4