From 3aef0e5844231f54dc83125bfabeb5fb499db209 Mon Sep 17 00:00:00 2001 From: Ruben Verborgh Date: Mon, 17 Sep 2018 15:19:04 +0200 Subject: [PATCH 1/3] Allow array as value in externals object. Fixes https://github.com/webpack/webpack/issues/8041. --- schemas/WebpackOptions.json | 3 +++ .../externals/optional-externals-array/index.js | 10 ++++++++++ .../optional-externals-array/webpack.config.js | 5 +++++ 3 files changed, 18 insertions(+) create mode 100644 test/configCases/externals/optional-externals-array/index.js create mode 100644 test/configCases/externals/optional-externals-array/webpack.config.js diff --git a/schemas/WebpackOptions.json b/schemas/WebpackOptions.json index 01513196134..ac11bfcdd3d 100644 --- a/schemas/WebpackOptions.json +++ b/schemas/WebpackOptions.json @@ -123,6 +123,9 @@ { "type": "object" }, + { + "$ref": "#/definitions/common.arrayOfStringValues" + }, { "type": "boolean" } diff --git a/test/configCases/externals/optional-externals-array/index.js b/test/configCases/externals/optional-externals-array/index.js new file mode 100644 index 00000000000..70700065cbe --- /dev/null +++ b/test/configCases/externals/optional-externals-array/index.js @@ -0,0 +1,10 @@ +it("should not fail on optional externals", function() { + try { + require("external"); + } catch(e) { + expect(e).toBeInstanceOf(Error); + expect(e.code).toBe("MODULE_NOT_FOUND"); + return; + } + throw new Error("It doesn't fail"); +}); diff --git a/test/configCases/externals/optional-externals-array/webpack.config.js b/test/configCases/externals/optional-externals-array/webpack.config.js new file mode 100644 index 00000000000..8563233c547 --- /dev/null +++ b/test/configCases/externals/optional-externals-array/webpack.config.js @@ -0,0 +1,5 @@ +module.exports = { + externals: { + external: ["./math", "subtract"] + } +}; From f0271d93c6716af3d0d39420c89c9723416be625 Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Tue, 18 Sep 2018 09:10:14 +0200 Subject: [PATCH 2/3] fix ExternalModule and test case --- lib/ExternalModule.js | 20 +++++++++++--- .../externals/externals-array/index.js | 4 +++ .../externals-array/webpack.config.js | 26 +++++++++++++++++++ .../optional-externals-array/index.js | 10 ------- .../webpack.config.js | 5 ---- 5 files changed, 46 insertions(+), 19 deletions(-) create mode 100644 test/configCases/externals/externals-array/index.js create mode 100644 test/configCases/externals/externals-array/webpack.config.js delete mode 100644 test/configCases/externals/optional-externals-array/index.js delete mode 100644 test/configCases/externals/optional-externals-array/webpack.config.js diff --git a/lib/ExternalModule.js b/lib/ExternalModule.js index f306710e4fb..8cb5e071c0b 100644 --- a/lib/ExternalModule.js +++ b/lib/ExternalModule.js @@ -74,7 +74,9 @@ class ExternalModule extends Module { .slice(1) .map(r => `[${JSON.stringify(r)}]`) .join(""); - return `module.exports = require(${moduleName})${objectLookup};`; + return `module.exports = require(${JSON.stringify( + moduleName + )})${objectLookup};`; } checkExternalVariable(variableToCheck, request) { @@ -94,15 +96,25 @@ class ExternalModule extends Module { } getSourceForDefaultCase(optional, request) { + if (!Array.isArray(request)) { + // make it an array as the look up works the same basically + request = [request]; + } + + const variableName = request[0]; const missingModuleError = optional - ? this.checkExternalVariable(request, request) + ? this.checkExternalVariable(variableName, request.join(".")) : ""; - return `${missingModuleError}module.exports = ${request};`; + const objectLookup = request + .slice(1) + .map(r => `[${JSON.stringify(r)}]`) + .join(""); + return `${missingModuleError}module.exports = ${variableName}${objectLookup};`; } getSourceString(runtime) { const request = - typeof this.request === "object" + typeof this.request === "object" && !Array.isArray(this.request) ? this.request[this.externalType] : this.request; switch (this.externalType) { diff --git a/test/configCases/externals/externals-array/index.js b/test/configCases/externals/externals-array/index.js new file mode 100644 index 00000000000..a7dedba652b --- /dev/null +++ b/test/configCases/externals/externals-array/index.js @@ -0,0 +1,4 @@ +it("should not fail on optional externals", function() { + const external = require("external"); + expect(external).toBe(EXPECTED); +}); diff --git a/test/configCases/externals/externals-array/webpack.config.js b/test/configCases/externals/externals-array/webpack.config.js new file mode 100644 index 00000000000..af6b62d059c --- /dev/null +++ b/test/configCases/externals/externals-array/webpack.config.js @@ -0,0 +1,26 @@ +const webpack = require("../../../../"); +module.exports = [ + { + output: { + libraryTarget: "commonjs2" + }, + externals: { + external: ["webpack", "version"] + }, + plugins: [ + new webpack.DefinePlugin({ + EXPECTED: JSON.stringify(webpack.version) + }) + ] + }, + { + externals: { + external: ["Array", "isArray"] + }, + plugins: [ + new webpack.DefinePlugin({ + EXPECTED: "Array.isArray" + }) + ] + } +]; diff --git a/test/configCases/externals/optional-externals-array/index.js b/test/configCases/externals/optional-externals-array/index.js deleted file mode 100644 index 70700065cbe..00000000000 --- a/test/configCases/externals/optional-externals-array/index.js +++ /dev/null @@ -1,10 +0,0 @@ -it("should not fail on optional externals", function() { - try { - require("external"); - } catch(e) { - expect(e).toBeInstanceOf(Error); - expect(e.code).toBe("MODULE_NOT_FOUND"); - return; - } - throw new Error("It doesn't fail"); -}); diff --git a/test/configCases/externals/optional-externals-array/webpack.config.js b/test/configCases/externals/optional-externals-array/webpack.config.js deleted file mode 100644 index 8563233c547..00000000000 --- a/test/configCases/externals/optional-externals-array/webpack.config.js +++ /dev/null @@ -1,5 +0,0 @@ -module.exports = { - externals: { - external: ["./math", "subtract"] - } -}; From 9bda6295cac742f0a1c44425416cd68fc4f9601b Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Tue, 18 Sep 2018 09:16:26 +0200 Subject: [PATCH 3/3] remove bad unit test --- test/ExternalModule.unittest.js | 308 -------------------------------- 1 file changed, 308 deletions(-) delete mode 100644 test/ExternalModule.unittest.js diff --git a/test/ExternalModule.unittest.js b/test/ExternalModule.unittest.js deleted file mode 100644 index 075c4600919..00000000000 --- a/test/ExternalModule.unittest.js +++ /dev/null @@ -1,308 +0,0 @@ -/* globals describe, it, beforeEach */ -"use strict"; - -const ExternalModule = require("../lib/ExternalModule"); -const OriginalSource = require("webpack-sources").OriginalSource; -const RawSource = require("webpack-sources").RawSource; - -describe("ExternalModule", () => { - let externalModule; - let request; - let type; - beforeEach(() => { - request = "some/request"; - type = "some-type"; - externalModule = new ExternalModule(request, type, `${type} ${request}`); - }); - describe("#identifier", () => { - it("returns an identifier for this module", () => { - const expected = `external "${request}"`; - expect(externalModule.identifier()).toBe(expected); - }); - }); - - describe("#readableIdentifier", () => { - it("returns an identifier for this module", () => { - const expected = `external "${request}"`; - expect(externalModule.identifier()).toBe(expected); - }); - }); - - describe("#needRebuild", () => { - it("always returns false", () => { - expect(externalModule.needRebuild()).toBe(false); - }); - }); - - describe("#size", () => { - it("always returns 42", () => { - expect(externalModule.size()).toBe(42); - }); - }); - - describe("#source", () => { - it("calls getSource with the result of getSourceString", () => { - // set up - const expectedString = "something expected stringy"; - const expectedSource = "something expected source"; - externalModule.getSource = jest.fn(() => expectedSource); - externalModule.getSourceString = jest.fn(() => expectedString); - - // invoke - const result = externalModule.source(); - - // check - expect(externalModule.getSource.mock.calls.length).toBe(1); - expect(externalModule.getSourceString.mock.calls.length).toBe(1); - expect(externalModule.getSource.mock.calls[0][0]).toBe(expectedString); - expect(result).toEqual(expectedSource); - }); - }); - - describe("#getSource", () => { - describe("given it should use source maps", () => { - beforeEach(() => { - externalModule.useSourceMap = true; - }); - it("returns an instance of OriginalSource", () => { - // set up - const someSourceString = "some source string"; - - // invoke - const result = externalModule.getSource(someSourceString); - - // check - expect(result).toBeInstanceOf(OriginalSource); - }); - }); - describe("given it does not use source maps", () => { - beforeEach(() => { - externalModule.useSourceMap = false; - }); - it("returns an instance of RawSource", () => { - // set up - const someSourceString = "some source string"; - - // invoke - const result = externalModule.getSource(someSourceString); - - // check - expect(result).toBeInstanceOf(RawSource); - }); - }); - }); - - describe("#getSourceForGlobalVariableExternal", () => { - describe("given an array as variable name in the global namespace", () => { - it("use the array as lookup in the global object", () => { - // set up - const type = "window"; - const varName = ["foo", "bar"]; - const expected = - '(function() { module.exports = window["foo"]["bar"]; }());'; - - // invoke - const result = externalModule.getSourceForGlobalVariableExternal( - varName, - type - ); - - // check - expect(result).toEqual(expected); - }); - }); - describe("given an single variable name", () => { - it("look it up in the global namespace", () => { - // set up - const type = "window"; - const varName = "foo"; - const expected = '(function() { module.exports = window["foo"]; }());'; - - // invoke - const result = externalModule.getSourceForGlobalVariableExternal( - varName, - type - ); - - // check - expect(result).toEqual(expected); - }); - }); - }); - - describe("#getSourceForCommonJsExternal", () => { - describe("given an array as names in the global namespace", () => { - it("use the first to require a module and the rest as lookup on the required module", () => { - // set up - const varName = ["module", "look", "up"]; - const expected = 'module.exports = require(module)["look"]["up"];'; - - // invoke - const result = externalModule.getSourceForCommonJsExternal( - varName, - type - ); - - // check - expect(result).toEqual(expected); - }); - }); - describe("given an single variable name", () => { - it("require a module with that name", () => { - // set up - const type = "window"; - const varName = "foo"; - const expected = 'module.exports = require("foo");'; - - // invoke - const result = externalModule.getSourceForCommonJsExternal( - varName, - type - ); - - // check - expect(result).toEqual(expected); - }); - }); - }); - - describe("#checkExternalVariable", () => { - it("creates a check that fails if a variable does not exist", () => { - // set up - const variableToCheck = "foo"; - const request = "bar"; - const expected = `if(typeof foo === 'undefined') {var e = new Error("Cannot find module 'bar'"); e.code = 'MODULE_NOT_FOUND'; throw e;} -`; - - // invoke - const result = externalModule.checkExternalVariable( - variableToCheck, - request - ); - - // check - expect(result).toEqual(expected); - }); - }); - - describe("#getSourceForAmdOrUmdExternal", () => { - it("looks up a global variable as specified by the id", () => { - // set up - const id = "someId"; - const optional = false; - const expected = "module.exports = __WEBPACK_EXTERNAL_MODULE_someId__;"; - - // invoke - const result = externalModule.getSourceForAmdOrUmdExternal( - id, - optional, - request - ); - - // check - expect(result).toEqual(expected); - }); - describe("given an optional check is set", function() { - it("ads a check for the existence of the variable before looking it up", () => { - // set up - const id = "someId"; - const optional = true; - const expected = `if(typeof __WEBPACK_EXTERNAL_MODULE_someId__ === 'undefined') {var e = new Error("Cannot find module 'some/request'"); e.code = 'MODULE_NOT_FOUND'; throw e;} -module.exports = __WEBPACK_EXTERNAL_MODULE_someId__;`; - - // invoke - const result = externalModule.getSourceForAmdOrUmdExternal( - id, - optional, - request - ); - - // check - expect(result).toEqual(expected); - }); - }); - }); - - describe("#getSourceForDefaultCase", () => { - it("returns the given request as lookup", () => { - // set up - const optional = false; - const expected = "module.exports = some/request;"; - - // invoke - const result = externalModule.getSourceForDefaultCase(optional, request); - - // check - expect(result).toEqual(expected); - }); - describe("given an optional check is requested", function() { - it("checks for the existence of the request setting it", () => { - // set up - const optional = true; - const expected = `if(typeof some/request === 'undefined') {var e = new Error("Cannot find module 'some/request'"); e.code = 'MODULE_NOT_FOUND'; throw e;} -module.exports = some/request;`; - - // invoke - const result = externalModule.getSourceForDefaultCase( - optional, - request - ); - - // check - expect(result).toEqual(expected); - }); - }); - }); - - describe("#updateHash", () => { - let hashedText; - let hash; - beforeEach(() => { - hashedText = ""; - hash = { - update: text => { - hashedText += text; - } - }; - externalModule.id = 12345678; - externalModule.updateHash(hash); - }); - it("updates hash with request", () => { - expect(hashedText).toMatch("some/request"); - }); - it("updates hash with type", () => { - expect(hashedText).toMatch("some-type"); - }); - it("updates hash with module id", () => { - expect(hashedText).toMatch("12345678"); - }); - }); - - describe("#updateHash without optional", () => { - let hashedText; - let hash; - beforeEach(() => { - hashedText = ""; - hash = { - update: text => { - hashedText += text; - } - }; - // Note no set of `externalModule.optional`, which crashed externals in 3.7.0 - externalModule.id = 12345678; - externalModule.updateHash(hash); - }); - it("updates hash with request", () => { - expect(hashedText).toMatch("some/request"); - }); - it("updates hash with type", () => { - expect(hashedText).toMatch("some-type"); - }); - it("updates hash with optional flag", () => { - expect(hashedText).toMatch("false"); - }); - it("updates hash with module id", () => { - expect(hashedText).toMatch("12345678"); - }); - }); -});