Skip to content

Commit 71d27d6

Browse files
sokrasokra
authored
Merge pull request webpack#6924 from Janpot/issue-4940
Don't error on "new require(...)"
2 parents 8b6092d + 199cd43 commit 71d27d6

7 files changed

Lines changed: 122 additions & 49 deletions

File tree

‎lib/dependencies/CommonJsRequireDependencyParserPlugin.js‎

Lines changed: 47 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -73,53 +73,58 @@ class CommonJsRequireDependencyParserPlugin {
7373
parser.state.current.addDependency(dep);
7474
return true;
7575
});
76-
parser.hooks.call
77-
.for("require")
78-
.tap("CommonJsRequireDependencyParserPlugin", expr => {
79-
if (expr.arguments.length !== 1) return;
80-
let localModule;
81-
const param = parser.evaluateExpression(expr.arguments[0]);
82-
if (param.isConditional()) {
83-
let isExpression = false;
84-
const prevLength = parser.state.current.dependencies.length;
85-
const dep = new RequireHeaderDependency(expr.callee.range);
86-
dep.loc = expr.loc;
87-
parser.state.current.addDependency(dep);
88-
for (const p of param.options) {
89-
const result = processItem(expr, p);
90-
if (result === undefined) {
91-
isExpression = true;
92-
}
93-
}
94-
if (isExpression) {
95-
parser.state.current.dependencies.length = prevLength;
96-
} else {
97-
return true;
76+
77+
const createHandler = callNew => expr => {
78+
if (expr.arguments.length !== 1) return;
79+
let localModule;
80+
const param = parser.evaluateExpression(expr.arguments[0]);
81+
if (param.isConditional()) {
82+
let isExpression = false;
83+
const prevLength = parser.state.current.dependencies.length;
84+
const dep = new RequireHeaderDependency(expr.callee.range);
85+
dep.loc = expr.loc;
86+
parser.state.current.addDependency(dep);
87+
for (const p of param.options) {
88+
const result = processItem(expr, p);
89+
if (result === undefined) {
90+
isExpression = true;
9891
}
9992
}
100-
if (
101-
param.isString() &&
102-
(localModule = LocalModulesHelpers.getLocalModule(
103-
parser.state,
104-
param.string
105-
))
106-
) {
107-
const dep = new LocalModuleDependency(localModule, expr.range);
108-
dep.loc = expr.loc;
109-
parser.state.current.addDependency(dep);
110-
return true;
93+
if (isExpression) {
94+
parser.state.current.dependencies.length = prevLength;
11195
} else {
112-
const result = processItem(expr, param);
113-
if (result === undefined) {
114-
processContext(expr, param);
115-
} else {
116-
const dep = new RequireHeaderDependency(expr.callee.range);
117-
dep.loc = expr.loc;
118-
parser.state.current.addDependency(dep);
119-
}
12096
return true;
12197
}
122-
});
98+
}
99+
if (
100+
param.isString() &&
101+
(localModule = LocalModulesHelpers.getLocalModule(
102+
parser.state,
103+
param.string
104+
))
105+
) {
106+
const dep = new LocalModuleDependency(localModule, expr.range, callNew);
107+
dep.loc = expr.loc;
108+
parser.state.current.addDependency(dep);
109+
return true;
110+
} else {
111+
const result = processItem(expr, param);
112+
if (result === undefined) {
113+
processContext(expr, param);
114+
} else {
115+
const dep = new RequireHeaderDependency(expr.callee.range);
116+
dep.loc = expr.loc;
117+
parser.state.current.addDependency(dep);
118+
}
119+
return true;
120+
}
121+
};
122+
parser.hooks.call
123+
.for("require")
124+
.tap("CommonJsRequireDependencyParserPlugin", createHandler(false));
125+
parser.hooks.new
126+
.for("require")
127+
.tap("CommonJsRequireDependencyParserPlugin", createHandler(true));
123128
}
124129
}
125130
module.exports = CommonJsRequireDependencyParserPlugin;

‎lib/dependencies/LocalModuleDependency.js‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,22 +6,22 @@
66
const NullDependency = require("./NullDependency");
77

88
class LocalModuleDependency extends NullDependency {
9-
constructor(localModule, range) {
9+
constructor(localModule, range, callNew) {
1010
super();
1111
localModule.flagUsed();
1212
this.localModule = localModule;
1313
this.range = range;
14+
this.callNew = callNew;
1415
}
1516
}
1617

1718
LocalModuleDependency.Template = class LocalModuleDependencyTemplate {
1819
apply(dep, source) {
1920
if (!dep.range) return;
20-
source.replace(
21-
dep.range[0],
22-
dep.range[1] - 1,
23-
dep.localModule.variableName()
24-
);
21+
const moduleInstance = dep.callNew
22+
? `new (function () { return ${dep.localModule.variableName()}; })()`
23+
: dep.localModule.variableName();
24+
source.replace(dep.range[0], dep.range[1] - 1, moduleInstance);
2525
}
2626
};
2727

‎package.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,8 @@
8989
],
9090
"scripts": {
9191
"setup": "node ./setup/setup.js",
92-
"test": "mocha test/*.test.js test/*.unittest.js --max-old-space-size=4096 --harmony --trace-deprecation",
92+
"test": "npm run mocha",
93+
"mocha": "mocha test/*.test.js test/*.unittest.js --max-old-space-size=4096 --harmony --trace-deprecation",
9394
"test:integration": "mocha test/*.test.js --max-old-space-size=4096 --harmony --trace-deprecation",
9495
"test:unit": "mocha test/*.unittest.js --max-old-space-size=4096 --harmony --trace-deprecation",
9596
"travis:integration": "yarn cover:init && yarn cover:integration && yarn cover:report-min",

‎test/cases/parsing/issue-4940/index.js‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
define("local-module-object", function () {
2+
return {
3+
foo: "bar"
4+
};
5+
});
6+
7+
define("local-side-effect", function () {
8+
return {
9+
foo: null
10+
};
11+
});
12+
13+
define("local-module-non-object", ["local-side-effect"], function (sideEffect) {
14+
sideEffect.foo = "bar"
15+
return 1;
16+
});
17+
18+
it("should create dependency when require is called with 'new' (object export)", function() {
19+
const result = new require("./object-export");
20+
result.foo.should.equal("bar");
21+
result.should.equal(require("./object-export"));
22+
});
23+
24+
it("should create dependency when require is called with 'new' (non-object export)", function() {
25+
const sideEffect = require("./sideEffect");
26+
const result = new require("./non-object-export");
27+
result.should.instanceof(__webpack_require__);
28+
sideEffect.foo.should.equal("bar");
29+
result.should.not.equal(require("./non-object-export"));
30+
});
31+
32+
it("should create dependency with 'new' on a local dependency (object export)", function() {
33+
const result = new require("local-module-object");
34+
result.foo.should.equal("bar");
35+
result.should.equal(require("local-module-object"));
36+
});
37+
38+
it("shouldn't fail with a local dependency (non-object export)", function() {
39+
const sideEffect = require("local-side-effect");
40+
const result = new require("local-module-non-object");
41+
result.should.not.equal(1);
42+
sideEffect.foo.should.equal("bar");
43+
result.should.not.equal(require("local-module-non-object"));
44+
});
45+
46+
it("should work with 'require' in parentheses", function () {
47+
const result = new (require)("./object-export");
48+
result.foo.should.equal("bar");
49+
});
50+
51+
it("should work with local module 'require' in parentheses", function () {
52+
const result = new (require)("local-module-object");
53+
result.foo.should.equal("bar");
54+
});
55+
56+
it("shouldn't fail with non-object local module 'require' in parentheses", function () {
57+
new (require)("local-module-non-object");
58+
});

‎test/cases/parsing/issue-4940/non-object-export.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
const sideEffect = require("./sideEffect");
2+
sideEffect.foo = "bar";
3+
module.exports = "foo";

‎test/cases/parsing/issue-4940/object-export.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
module.exports = {
2+
foo: "bar"
3+
};

‎test/cases/parsing/issue-4940/sideEffect.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
module.exports = {
2+
foo: null
3+
};

0 commit comments

Comments
 (0)