Generate per-package implicit-modules imports #1536
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, all implicit-modules got imported by their containing engine (normally the app). This makes some sense, because they are getting AMD-defined into one flat namespace anyway for compatibility reasons. If there are multiple versions of a package in the transitive deps, only one can be present in the AMD loader. However, this assumes there will be resolvability from the app all the way to the transitive deps, which is not reliably true. And it also means that if there is a mismatch between the list of implicit-modules provided by different copies of a package you could run into failures where you try to find them in the wrong copy. So instead, this PR moves the importing of implicit modules into per-package virtual modules. Each package imports its own direct dependencies' implicit modules, as well as the virtual implicit-module entrypoints for those dependencies, so the system recurses.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, all implicit-modules got imported by their containing engine (normally the app). This makes some sense, because they are getting AMD-defined into one flat namespace anyway for compatibility reasons. If there are multiple versions of a package in the transitive deps, only one can be present in the AMD loader.
However, this assumes there will be resolvability from the app all the way to the transitive deps, which is not true in general. And it also means that if there is a mismatch between the list of implicit-modules provided by different copies of a package you could run into failures where you try to find them in the wrong copy.
So instead, this PR moves the importing of implicit modules into per-package virtual modules. Each package imports its own direct dependencies' implicit modules, as well as the virtual implicit-module entrypoints for those dependencies.
If you have vast webs of addons, this change might make your code bigger. But the solution to that is enabling
staticAddonTrees, which cuts the set of implicit-modules down to approximately zero.