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";

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
module.exports = [
2+
[/system_undef/, /System\.import\(\) is deprecated/]
3+
];

‎test/configCases/parsing/system.import/webpack.config.js

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
const webpack = require("../../../../");
2+
3+
function createConfig(system) {
4+
const systemString = "" + system;
5+
return {
6+
name: `system_${systemString}`,
7+
module: {
8+
rules: [
9+
{
10+
test: /\.js$/,
11+
parser: {
12+
system
13+
}
14+
}
15+
]
16+
},
17+
plugins: [
18+
new webpack.DefinePlugin({
19+
__SYSTEM__: systemString
20+
})
21+
]
22+
};
23+
}
24+
25+
module.exports = [
26+
createConfig(undefined),
27+
createConfig(true),
28+
createConfig(false)
29+
];

‎test/configCases/target/node-dynamic-import/index.js

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,9 @@ it("should be able to use expressions in lazy-once import", function(done) {
3838
testCase(load, done);
3939
});
4040

41-
it("should be able to use expressions in System.import", function(done) {
41+
it("should be able to use expressions in import", function(done) {
4242
function load(name, expected, callback) {
43-
System.import("./dir2/" + name).then((result) => {
43+
import("./dir2/" + name).then((result) => {
4444
result.should.be.eql({ default: expected });
4545
callback();
4646
}).catch((err) => {
@@ -62,13 +62,3 @@ it("should be able to use import", function(done) {
6262
done(err);
6363
});
6464
});
65-
66-
it("should be able to use System.import", function(done) {
67-
System.import("./two").then((two) => {
68-
two.should.be.eql({ default: 2 });
69-
done();
70-
}).catch(function(err) {
71-
done(err);
72-
});
73-
});
74-

‎test/hotCases/chunks/accept-system-import/index.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
it("should import a changed chunk", function(done) {
2-
System.import("./chunk").then(function(chunk) {
2+
import("./chunk").then(function(chunk) {
33
chunk.value.should.be.eql(1);
4-
System.import("./chunk2").then(function(chunk2) {
4+
import("./chunk2").then(function(chunk2) {
55
chunk2.value.should.be.eql(1);
66
NEXT(require("../../update")(done));
77
module.hot.accept(["./chunk", "./chunk2"], function() {
8-
System.import("./chunk").then(function(chunk) {
8+
import("./chunk").then(function(chunk) {
99
chunk.value.should.be.eql(2);
10-
System.import("./chunk2").then(function(chunk2) {
10+
import("./chunk2").then(function(chunk2) {
1111
chunk2.value.should.be.eql(2);
1212
done();
1313
}).catch(done);

‎test/hotCases/chunks/dynamic-system-import/index.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
it("should import a changed chunk (dynamic import)", function(done) {
22
function load(name) {
3-
return System.import("./chunk" + name);
3+
return import("./chunk" + name);
44
}
55
load(1).then(function(chunk) {
66
chunk.value.should.be.eql(1);

‎test/hotCases/chunks/system-import/index.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
it("should import a changed chunk", function(done) {
2-
System.import("./chunk").then(function(chunk) {
2+
import("./chunk").then(function(chunk) {
33
chunk.value.should.be.eql(1);
44
chunk.value2.should.be.eql(3);
55
chunk.counter.should.be.eql(0);
66
NEXT(require("../../update")(done, true, function() {
77
chunk.value.should.be.eql(2);
88
chunk.value2.should.be.eql(4);
99
chunk.counter.should.be.eql(1);
10-
System.import("./chunk2").then(function(chunk2) {
10+
import("./chunk2").then(function(chunk2) {
1111
chunk2.value.should.be.eql(2);
1212
chunk2.value2.should.be.eql(4);
1313
chunk2.counter.should.be.eql(0);

‎test/hotCases/runtime/dispose-removed-chunk/module.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
export default System.import("./a");
1+
export default import("./a");
22
---
3-
export default System.import("./b");
3+
export default import("./b");
44
---
5-
export default System.import("./a");
5+
export default import("./a");

0 commit comments

Comments
 (0)