Apply .meteorignore to node_modules in PackageSource#_findSources.#9800
Merged
Apply .meteorignore to node_modules in PackageSource#_findSources.#9800
Conversation
Although there was a comment in this code about not applying .meteorignore files to the contents of node_modules directories, I'm pretty sure that was the wrong decision, because .meteorignore merely limits what Meteor tries to compile as application code, and does not actually modify or hide node_modules files from other parts of Meteor (or Node). In other words, there's no harm in letting .meteorignore apply to node_modules, and there may be a LOT of benefit, because it allows the developer to fight back when compilation descends unexpectedly into an npm package that contains non-.js[on] files for which a compiler plugin has been registered, an obscure but not uncommon behavior originally intended to allow importing CSS assets from npm packages: * #6037 * 43659ff * a073280 * #5242 * #6846 * #7406 However, we now have a much more powerful tool for selectively compiling specific npm packages: #9771. In light of this new approach, we should probably remove the promiscuous node_modules compilation behavior altogether, as it might speed up rebuild times for many applications whose developers don't know or care that this behavior exists. For example, abandoning this behavior would prevent the problem reported here: #6950 In the meantime, this commit makes .meteorignore work for node_modules.
benjamn
pushed a commit
that referenced
this pull request
Apr 18, 2018
This functionality was originally intended to allow importing compiled-to-JS modules from `node_modules`, by precompiling any modules found in top-level npm packages, as long as a Meteor compiler plugin was registered for the module's file extension. As discussed in #9800, this extra compilation burden can be non-trivial, especially if you happen to install an npm package such as `less`, which contains hundreds of `.less` files in the `node_modules/less/test/` directory. More generally, this functionality was an early attempt to enable selective compilation of `node_modules` directories, but it was not a good solution to that problem, because in almost all cases the extra compilation was unwanted. Meteor 1.7 (formerly known as 1.6.2) will give full control over selective compilation of `node_modules` back to the application developer (#9771), which should afford a much better solution to this problem. If you've installed some `.less` or `.scss` or `.ts` files from npm into your `node_modules` directory, just create a symlink to the package directory within your application, and those modules will be compiled and become importable by other JS modules, as if they were part of the application.
benjamn
added a commit
that referenced
this pull request
Apr 18, 2018
…#9825) This functionality was originally intended to allow importing compiled-to-JS modules from `node_modules`, by precompiling any modules found in top-level npm packages, as long as a Meteor compiler plugin was registered for the module's file extension. As discussed in #9800, this extra compilation burden can be non-trivial, especially if you happen to install an npm package such as `less`, which contains hundreds of `.less` files in the `node_modules/less/test/` directory. More generally, this functionality was an early attempt to enable selective compilation of `node_modules` directories, but it was not a good solution to that problem, because in almost all cases the extra compilation was unwanted. Meteor 1.7 (formerly known as 1.6.2) will give full control over selective compilation of `node_modules` back to the application developer (#9771), which should afford a much better solution to this problem. If you've installed some `.less` or `.scss` or `.ts` files from npm into your `node_modules` directory, just create a symlink to the package directory within your application, and those modules will be compiled and become importable by other JS modules, as if they were part of the application.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Although there was a comment in this code about not applying
.meteorignorefiles to the contents ofnode_modulesdirectories, I'm pretty sure that was the wrong decision, because.meteorignoremerely limits what Meteor tries to compile as application code, and does not actually modify or hidenode_modulesfiles from other parts of Meteor (or Node).In other words, there's no harm in letting
.meteorignoreapply tonode_modules, and there may be a LOT of benefit, because it allows the developer to fight back when compilation descends unexpectedly into an npm package that contains non-.js[on] files for which a compiler plugin has been registered, an obscure but not uncommon behavior originally intended to allow importing CSS assets from npm packages:However, we now have a much more powerful tool for selectively compiling specific npm packages: #9771. In light of this new approach, we should probably remove the promiscuous
node_modulescompilation behavior altogether, as it might speed up rebuild times for many applications whose developers don't know or care that this behavior exists. For example, abandoning this behavior would prevent the problem reported here: #6950In the meantime, this commit makes
.meteorignorework fornode_modules.