Eliminate node_modules rewriting #1435
Conversation
It was unused because the code to emit the index doesn't exist, but I'm about to add it, and I'd like to focus on one thihng at a time.
ef4
force-pushed
the
done-rewrite-node-modules2
branch
from
11f723b
to
8bccbd7
Compare
It existed primarily to power our node modules rewriting.
ef4
force-pushed
the
done-rewrite-node-modules2
branch
from
bc32c93
to
9425b06
Compare
The original intent here was a guess about what part we'd want to swap out for a v2 app. But it assumed incorrectly that there would still be broccoli driving the v2 app.
This is going to come back, but in a different implementation
…odules2 # Conflicts: # packages/compat/src/v1-app.ts
mansona
force-pushed
the
done-rewrite-node-modules2
branch
from
ec433a3
to
09efde9
Compare
This is a side-quest for #1435. The engines-related tests fail there because we still have app-tree-merging support for dumping addon files into engines, which requires us to manage two copies of each engine. Since our module-resolver can now handle all the app-tree-merging virtually, we can drop that whole part of AppDiffer (and maybe *all* of AppDiffer) to solve the problem. And reforming AppDiffer makes this a good time to remove the Stage concept.. I'm doing this on a separate PR because this is going to take a step backard in terms of passing tests before it moves forward again. I expect the first commit breaks some fastboot-specific features in apps (we already handle those same features in addons via the module-resolver).
|
This is getting really close. The remaining failures are because the new uses of tree-sync that are introduced turn out to be too aggressive (tree-sync wants to control its entire output directory and will delete anything it doesn't recognize, whereas we're trying to blend several outputs into the same directory). Things like the public assets are getting deleted, and the test are picking up on that. |
|
Congrats! Must feel great to finally have this done! |
ef4
changed the title
|
This afternoon after the 3.1.0 release we started seeing |
|
@jrjohnson I was about to comment the same thing, after updating to embroider 3.1.0 - same I'm using pnpm if that matters - I get the error even after a fresh pnpm - |
|
@jrjohnson @Techn1x we debugged this today and @mansona is going to prepare the fix. Thanks for reporting. |
|
Oh boy, when I consume the updates I get past that issue with percy/sdk-utils (thanks so much!!), but end up with a whole different error on build.. (note: using pnpm) Looks like this issue is related to ember-modal-dialog somehow. When I remove that addon from my app, and all references to it, I get past the error and my app builds and runs. I'm not sure if maybe I have embroider configured incorrectly or someting - it looks very similar to the error stacks encountered here #1371 (comment) and here #1483 but they could be a red herring. |
This is picking up the work explored in #1374. I'm going to probably land those same code changes, but in a different order so we can more easily keep the tests green through the refactor.
My plan here: