NamedModulePlugins breaks global lookups, such as jquery

[Judahmeek]

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
Adding NamedModulesPlugin to a project that uses certain global lookups results in what are effectively reference errors.

If the current behavior is a bug, please provide the steps to reproduce.
Add NamedModulePlugins to https://github.com/shakacode/bootstrap-loader/blob/master/examples/basic/webpack.dev.config.js (which uses jquery & bootstrap. Resulting Error).

https://github.com/eggheadio/egghead-ui/blob/master/src/components/Icon/index.js#L65 also breaks with keys being declared not a function.

What is the expected behavior?
That global lookups work.

If this is a feature request, what is motivation or use case for changing the behavior?
Compatibility with other libraries.

Please mention other relevant information such as the browser version, Node.js version, webpack version and Operating System.
webpack: 2.2.1

[sokra]

duplicate of shakacode/bootstrap-loader#288

[Judahmeek]

@sokra bootstrap-loader is not the only project where I've had things break just by adding NamedModulePlugin. Shouldn't it be shakacode/bootstrap-loader#288 that's the duplicate or irrelevant issue?

[NervosaX]

Surely this is a webpack issue, not an issue of bootstrap-loader. If I add this plugin, it breaks jquery and velocity. I do not use bootstrap-loader. @sokra

[harperj1029]

I'm seeing the same issue. Why is this closed?

I have these two modules rules (note the dependence the first has on the exported [upper-case Q for jQuery] jQuery:
{ test: path.resolve(__dirname, "lib/jquery.filedownload.js"), use: [ "imports-loader?jQuery=jquery", "exports-loader?jqueryFileDownload" ] }, { test: path.resolve(__dirname, "scripts/lib/jquery-3.1.1.js"), use: [ "expose-loader?jQuery", "expose-loader?$" ] }
When I add the NamedModulesPlugin, then the globals $ and jQuery are no longer in the global.

[sokra]

Could you file a new issue?

[NervosaX]

I've filed this under the expose-loader project, as I believe that is the culprit.

webpack-contrib/expose-loader#49

[alangpierce]

I tracked this down and I believe it's a bug in NamedModulesPlugin, not expose-loader. When I looked in the bundle file causing errors, I saw that there were two modules with the same ID (the path to jquery on the filesystem). NamedModulesPlugin works by calling module.libIdent on each module (code) and using that as the ID, but it looks like in some cases (e.g. modules created by expose-loader), two different modules return the same libIdent return value, resulting in an ID collision. I hacked the code to keep track of all generated IDs and skip any cases where the same ID has been generated before, and it fixed the issue.

I can try coming up with a PR, although I'm fairly new to webpack, especially the internals. But it seems like it should be an easy fix. @sokra do you want a new issue filed, or do you think it makes sense to re-open this issue.

[cansin]

This problem is preventing us to use NamedModulesPlugin in conjunction with expose-loader as explained at webpack-contrib/expose-loader#55 . Looks like @sokra said

That's a bug in libIdent webpack/webpack.

on the issue linked above. So I am surprised he closed this one.

[cansin]

I have zero experience with Webpack's source code (except for reading it to figure things out when docs fall short). So I probably don't know what I am talking about. Take it with a grain of salt:

So a patch like what @alangpierce described would make NamedModulesPlugin look like:

/*
    MIT License http://www.opensource.org/licenses/mit-license.php
    Author Tobias Koppers @sokra
*/
"use strict";

class NamedModulesPlugin {
    constructor(options) {
        this.options = options || {};
    }

    apply(compiler) {
        compiler.plugin("compilation", (compilation) => {
            compilation.plugin("before-module-ids", (modules) => {
                let usedIds = [];
                modules.forEach((module) => {
                    let proposedId = null;
                    if (module.id === null && module.libIdent) {
                        proposedId = module.libIdent({
                            context: this.options.context || compiler.options.context
                        });
                        if (!usedIds.includes(proposedId)) {
                            module.id = proposedId;
                            usedIds.push(proposedId);
                        }
                    }
                });
            });
        });
    }
}

module.exports = NamedModulesPlugin;

This is, in fact, easy and fixes the expose-loader issue described at webpack-contrib/expose-loader#55 . But I sense that skipping setting module ids might break consistent hashing for long-term caching. Since we ended up skipping setting module ids for certain modules, I assume the ids for such modules became non-deterministic.

Let's wait for @sokra to weigh in.