Skip to content

Commit 3fb63f9

Browse files
ooflorentooflorent
committed
Deprecate System.import() parser plugin
- `system: undefined`: Warns if `System.import()` is used - `system: true`: Disable warning - `system: false`: Skip `System.import()` instrumentation
1 parent 114abee commit 3fb63f9

File tree

17 files changed

+118
-64
lines changed

17 files changed

+118
-64
lines changed

‎lib/dependencies/ImportParserPlugin.js‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,7 @@ class ImportParserPlugin {
1717
}
1818

1919
apply(parser) {
20-
const options = this.options;
21-
const handler = (expr) => {
20+
parser.hooks.importCall.tap("ImportParserPlugin", expr => {
2221
if(expr.arguments.length !== 1)
2322
throw new Error("Incorrect number of arguments provided to 'import(module: string) -> Promise'.");
2423

@@ -84,7 +83,7 @@ class ImportParserPlugin {
8483
if(mode === "weak") {
8584
mode = "async-weak";
8685
}
87-
const dep = ContextDependencyHelpers.create(ImportContextDependency, expr.range, param, expr, options, {
86+
const dep = ContextDependencyHelpers.create(ImportContextDependency, expr.range, param, expr, this.options, {
8887
chunkName,
8988
include,
9089
exclude,
@@ -97,10 +96,7 @@ class ImportParserPlugin {
9796
parser.state.current.addDependency(dep);
9897
return true;
9998
}
100-
};
101-
102-
parser.hooks.call.for("System.import").tap("ImportParserPlugin", handler);
103-
parser.hooks.importCall.tap("ImportParserPlugin", handler);
99+
});
104100
}
105101
}
106102
module.exports = ImportParserPlugin;

‎lib/dependencies/SystemPlugin.js‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
Author Tobias Koppers @sokra
44
*/
55
"use strict";
6+
67
const ParserHelpers = require("../ParserHelpers");
8+
const WebpackError = require("../WebpackError");
79

810
class SystemPlugin {
911
constructor(options) {
@@ -18,6 +20,8 @@ class SystemPlugin {
1820
if(typeof parserOptions.system !== "undefined" && !parserOptions.system)
1921
return;
2022

23+
const shouldWarn = typeof parserOptions.system === "undefined";
24+
2125
const setNotSupported = name => {
2226
parser.hooks.evaluateTypeof.for(name).tap("SystemPlugin", ParserHelpers.evaluateToString("undefined"));
2327
parser.hooks.expression.for(name).tap("SystemPlugin",
@@ -39,11 +43,35 @@ class SystemPlugin {
3943
parser.state.module.context, require.resolve("../../buildin/system.js"));
4044
return ParserHelpers.addParsedVariableToModule(parser, "System", systemPolyfillRequire);
4145
});
46+
47+
parser.hooks.call.for("System.import").tap("SystemPlugin", expr => {
48+
if(shouldWarn) {
49+
parser.state.module.warnings.push(new SystemImportDeprecationWarning(parser.state.module, expr.loc));
50+
}
51+
52+
return parser.hooks.importCall.call(expr);
53+
});
4254
};
4355

4456
normalModuleFactory.hooks.parser.for("javascript/auto").tap("SystemPlugin", handler);
4557
normalModuleFactory.hooks.parser.for("javascript/dynamic").tap("SystemPlugin", handler);
4658
});
4759
}
4860
}
61+
62+
class SystemImportDeprecationWarning extends WebpackError {
63+
constructor(module, loc) {
64+
super();
65+
66+
this.name = "SystemImportDeprecationWarning";
67+
this.message = "System.import() is deprecated and will be removed soon. Use import() instead.\n" +
68+
"For more info visit https://webpack.js.org/guides/code-splitting/";
69+
70+
this.origin = this.module = module;
71+
this.originLoc = loc;
72+
73+
Error.captureStackTrace(this, this.constructor);
74+
}
75+
}
76+
4977
module.exports = SystemPlugin;

‎lib/node/NodeMainTemplatePlugin.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ module.exports = class NodeMainTemplatePlugin {
3636
`${mainTemplate.requireFn}.oe = function(err) {`,
3737
Template.indent([
3838
"process.nextTick(function() {",
39-
Template.indent("throw err; // catch this error by using System.import().catch()"),
39+
Template.indent("throw err; // catch this error by using import().catch()"),
4040
"});"
4141
]),
4242
"};"

‎test/Parser.unittest.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -486,8 +486,8 @@ describe("Parser", () => {
486486
param: "y"
487487
}
488488
],
489-
"System.import": [
490-
"async function x() { const y = await System.import('z'); }", {
489+
"import": [
490+
"async function x() { const y = await import('z'); }", {
491491
param: "z"
492492
}
493493
]
@@ -498,7 +498,7 @@ describe("Parser", () => {
498498
const param = parser.evaluateExpression(expr.arguments[0]);
499499
parser.state.param = param.string;
500500
});
501-
parser.hooks.call.tap("System.import", "ParserTest", (expr) => {
501+
parser.hooks.importCall.tap("ParserTest", (expr) => {
502502
const param = parser.evaluateExpression(expr.arguments[0]);
503503
parser.state.param = param.string;
504504
});

‎test/browsertest/lib/index.web.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,12 +90,12 @@ describe("main", function() {
9090
it("should be able to handle chunk loading errors and try again", function(done) {
9191
var old = __webpack_public_path__;
9292
__webpack_public_path__ += "wrong/";
93-
System.import("./three").then(function() {
93+
import("./three").then(function() {
9494
done(new Error("Chunk shouldn't be loaded"));
9595
}).catch(function(err) {
9696
err.should.be.instanceOf(Error);
9797
__webpack_public_path__ = old;
98-
System.import("./three").then(function(three) {
98+
import("./three").then(function(three) {
9999
three.should.be.eql(3);
100100
done();
101101
}).catch(function(err) {

‎test/cases/chunks/import-context/index.js‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -31,15 +31,3 @@ it("should be able to use expressions in import", function(done) {
3131
}
3232
testCase(load, done);
3333
});
34-
35-
it("should be able to use expressions in System.import", function(done) {
36-
function load(name, expected, callback) {
37-
System.import("./dir2/" + name).then(function(result) {
38-
result.should.be.eql({ default: expected });
39-
callback();
40-
}).catch(function(err) {
41-
done(err);
42-
});
43-
}
44-
testCase(load, done);
45-
});

‎test/cases/chunks/import/index.js‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,3 @@ it("should be able to use import", function(done) {
66
done(err);
77
});
88
});
9-
10-
it("should be able to use System.import", function(done) {
11-
System.import("./two").then(function(two) {
12-
two.should.be.eql({ default: 2 });
13-
done();
14-
}).catch(function(err) {
15-
done(err);
16-
});
17-
});

‎test/cases/parsing/typeof/index.js‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,6 @@ it("should answer typeof require.ensure correctly", function() {
2929
it("should answer typeof require.resolve correctly", function() {
3030
(typeof require.resolve).should.be.eql("function");
3131
});
32-
it("should answer typeof System correctly", function() {
33-
(typeof System).should.be.eql("object");
34-
});
35-
it("should answer typeof System.import correctly", function() {
36-
(typeof System.import).should.be.eql("function");
37-
});
38-
3932

4033
it("should not parse filtered stuff", function() {
4134
if(typeof require != "function") require("fail");
@@ -50,7 +43,6 @@ it("should not parse filtered stuff", function() {
5043
if(typeof module != "object") module = require("fail");
5144
if(typeof exports == "undefined") exports = require("fail");
5245
if(typeof System !== "object") exports = require("fail");
53-
if(typeof System.import !== "function") exports = require("fail");
5446
if(typeof require.include !== "function") require.include("fail");
5547
if(typeof require.ensure !== "function") require.ensure(["fail"], function(){});
5648
});

‎test/configCases/parsing/system.import/index.js‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
it("should answer typeof System correctly", () => {
2+
if(__SYSTEM__ === false) {
3+
(typeof System).should.be.eql("undefined");
4+
} else {
5+
(typeof System).should.be.eql("object");
6+
}
7+
});
8+
9+
it("should answer typeof System.import correctly", () => {
10+
if(__SYSTEM__ === false) {
11+
(() => {
12+
typeof System.import;
13+
}).should.throw();
14+
} else {
15+
(typeof System.import).should.be.eql("function");
16+
}
17+
});
18+
19+
it("should be able to use System.import()", done => {
20+
try {
21+
System.import("./module").then(mod => {
22+
if(__SYSTEM__ === false) {
23+
done(new Error("System.import should not be parsed"));
24+
} else {
25+
mod.should.be.eql({ default: "ok" });
26+
done();
27+
}
28+
});
29+
} catch(e) {
30+
if(__SYSTEM__ === false) {
31+
done();
32+
} else {
33+
done(e);
34+
}
35+
}
36+
});

‎test/configCases/parsing/system.import/module.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export default "ok";

0 commit comments

Comments
 (0)