Module concatenation produces invalid output due to name collision

[Knagis]

Bug report

What is the current behavior?

import * as cn from "classnames";

export function foo() {
    const classnames = cn({a: 1});
    console.log(classnames);
    return classnames;
}

with module concatenation this is compiled as

// EXTERNAL MODULE: ./node_modules/classnames/index.js
var classnames = __webpack_require__("./node_modules/classnames/index.js");
// CONCATENATED MODULE: ./src/comp.js


function foo() {
    const classnames = classnames({a: 1});
    console.log(classnames);
    return classnames;
}

This reproduces in development mode, with concatenateModules: true and in full production mode. The code listed above has to be in a imported file, not directly in the entry.

If the current behavior is a bug, please provide the steps to reproduce.

• Clone https://github.com/Knagis/webpack-concat-issue/
• yarn
• yarn webpack

What is the expected behavior?

imported variable to have some sort of prefix/suffix to avoid name collisions or it preserves the original name as specified in the source

Other relevant information:
webpack version: 5.1.3
Node.js version: 12.13.1
Operating System: Win10
Additional tools:

[code1x1]

// EXTERNAL MODULE: leads to the assumption that you are using classnames as external module? is this correct?
could you provide a full example repository?
ok just checked out the example repo. it looks like a bug

[code1x1]

im still not sure if this is a bug or the type of classnames npm module type is not valid thus it becomes type external
webpack marks the npm module as external

webpack/lib/optimize/ConcatenatedModule.js

Line 832 in 9342540

if (modulesSet.has(module)) {

you could try to use webpack externals configuration to rename the module.

module.exports = {
    mode: "development",
    devtool: false,
    optimization: {
        concatenateModules: true,
    },
    externals: {
        classnames: 'cn'
    }
};

which will lead to the following output

// CONCATENATED MODULE: external "cn"
const external_cn_namespaceObject = cn;
// CONCATENATED MODULE: ./src/comp.js


function foo() {
    const classnames = external_cn_namespaceObject({a: 1});

[alexander-akait]

I think it is bug

[code1x1]

repo: https://github.com/code1x1/webpack-concat-issue

[code1x1]

i created a branch with actual working example
https://github.com/code1x1/webpack-concat-issue/tree/how-to-use-the-module
use the suggested usage from the npm readme

suggested usage from the npm author

var classNames = require('classnames');
classNames('foo', 'bar'); // => 'foo bar'

[Knagis]

@code1x1 do you suggest that I go and list every single dependency that I might have as external in webpack config?

classnames dependency is just an example, the issue does not seem to be limited to it. This dependency also doesn't have anything particular in its package.json to suggest it is incorrect by itself.

Also how is switching from import to require a valid suggestion in this day? It might be a workaround, yes, but in no way it proves that this is not an issue.

[sokra]

It's external to the concatenated module. No necessary external to the whole bundle. Anyway that's a bug.

[Knagis]

just an example that it is not limited to classnames package:

import * as _ from "lodash";
export function foo() {
    const lodash = _(123);
    console.log(lodash);
    return lodash;
}

var lodash = __webpack_require__("./node_modules/lodash/lodash.js");
function foo() {
    const lodash = lodash(123);
    console.log(lodash);
    return lodash;
}

[code1x1]

import _ from "lodash";
export function foo() {
    const lodash = _(123);
    console.log(lodash);
    return lodash;
}

should work 🙂

but your right the import should be prefixed.
I will open a pr for that soon :)

[code1x1]

Here is what i know so far:

currentConfiguration.warnings.stack[0].optimizationBailout
is set to

get optimizationBailout:ƒ optimizationBailout() {\n\t\treturn ModuleGraph.getModuleGraphForModule(\n\t\t\tthis,\n\t\t\t"Module.optimizationBailout",\n\t\t\t"DEP_WEBPACK_MODULE_OPTIMIZATION_BAILOUT"\n\t\t).getOptimizationBailout(this);\n\t}
Array(3)
0:'CommonJS bailout: module.exports is used directly at 41:38-52'
1:'CommonJS bailout: module.exports is used directly at 43:2-16'
2:'ModuleConcatenation bailout: Module is not an ECMAScript module'
length:3
__proto__:Array(0)

because this if statement is executed

webpack/lib/optimize/ModuleConcatenationPlugin.js

Line 509 in b5b423e

if (!possibleModules.has(module)) {

which results in this if statement being executed

webpack/lib/optimize/ModuleConcatenationPlugin.js

Line 253 in b5b423e

if (problem) {

after that the currentConfiguration.modules rollbacks.
classnames disappears and the above mentioned warning will be added

ModuleConcatenation bailout: Module is not an ECMAScript module

webpack/lib/optimize/ModuleConcatenationPlugin.js

Line 258 in b5b423e

currentConfiguration.rollback(backup);

im still not sure how to fix the problem without breaking a lot of other cases..

[Knagis]

This is probably not a very good hack but still - perhaps until a more proper solution is found, the workaround could be to add suffix for every external in concatenated modules?

webpack/lib/optimize/ConcatenatedModule.js

Line 1128 in b5b423e

info.name = externalName;

				case "external": {
					const externalName = this.findNewName(
						"",
						allUsedNames,
						namespaceObjectUsedNames,
						info.module.readableIdentifier(requestShortener)
					);
					allUsedNames.add(externalName);
-					info.name = externalName;
+					info.name = externalName + "__WEBPACK_CONCAT_EXTERNAL_";
					break;
				}

[code1x1]

type external probably has to stay that name because external could also reference an external library which this maybe required to reference it.
if that case wont break then it is a valid solution 👍

[Knagis]

Created a PR with the change mentioned above. The issue reproduced nicely in the tests, which is nice :)