Double NPM modules

[staeke]

The file .meteor/local/build/programs/web.browser/packages/modules.js includes all npm dependencies from Meteor packages. However, those npm dependencies may be referenced from your app code as well, leading to code duplication, which would be ok for some modules, disastrous for some.

Example:

• you have the Meteor package barbatus:angular2-polyfills which includes rxjs as a referenced npm package
• in your source you have import { Observable } from 'rxjs/Observable'
• now Observable will appear both in modules.js and in a webpack destination (e.g. your main app.js)

The way I see it, we could either go for

1. removing duplicate from modules.js (error prone)
2. tell webpack to resolve these npm modules by some mechanism that "looks into" modules.js.

I believe in 2 (unless I'm missing another option). How could this be done? Jacking into the webpack build process to provide modules? Separate build step? Manual config for those modules?

One way would be to parse modules.js (a little hacky?) to look for e.g. "// node_modules/[MODULE_NAME]" (those comments prefixes all npm imports in modules.js). Step 2 would then be to hook into webpack's build chain and add externals accordingly. Something like:

externals: {
     'rxjs/Observable': "Package['modules-runtime'].meteorInstall()('rxjs/Observable')"
}

What do you think?

[staeke]

Maybe do something like they've done here instead of adding externals to the config? https://github.com/Morhaus/webpack-externals-plugin/blob/master/index.js

[staeke]

I wrote a suggestion plugin. Can make a PR if you want. Things that I haven't addressed (lack of experience with webpack):

• I currently haven't indicated to webpack that I want a watch on modules.js to only read it when necessary (right now I'm just reading it once and storing it)
• I have not informed the build system that there's a dependency to modules.js. That should of course be done to have the dependency graph done right
• I now require an extra config for noParse on modules.js in the webpack.config.js. I guess this could be done somehow when resolving/adding the module originally

[luisherranz]

Where are the Meteor npm packages installed? Maybe a simple resolve.modulesDirectories pointing there would do the trick.

[staeke]

The meteor npm packages originally comes from standard node_modules, but they're compiled by Meteor (along with some Meteor code that cannot easily be excluded) into .meteor/local/build/programs/web.browser/packages/modules.js.

So, the way I see it it's hard to just use resolve.modulesDirectories since they're bundled with and wrapped in Meteor code

[luisherranz]

Ok, I understand. It's not that webpack needs to know where those are, it's that it shouldn't include them in the bundle because they are already included by Meteor. Is that the issue?

Is your solution working?

[staeke]

You got it. Yes, it's working, but with the downsides I mentioned. I suppose I can look into fixing those when I get some time. Or...if you want to help out?

[luisherranz]

Not certainly, no, but if you do that PR I will accept it :)

[staeke]

ok...i'll get back

[jedwards1211]

@staeke another option would be to merge everything we can from Meteor's package.json into the top-level package.json, and leave only things that were semver-incompatible. One thing that's tricky though is Isobuild frighteningly still puts some deps in bundle/programs/server/npm/node_modules/meteor/<meteor package>/node_modules for various packages...examine this screenshot to see more about how meteorInstall is used:

[jedwards1211]

I still think in the long run it would be best to process Meteor packages' package.js and use that information to build them to into regular npm packages (though still relying on webpack/babel plugins to resolve requires like meteor/alanning:roles into node_modules/meteor/alanning_roles or whatever, since : isn't a valid filename character. But it's all a matter of what I have time for :)

[staeke]

I principally agree @jedwards1211 but do you still think we should parse/replace modules.js?

[jedwards1211]

I haven't looked into it enough yet. We'll see if I ever find the time...