From 4e808cb6137ef2640b691db3e1867d202c2faaa2 Mon Sep 17 00:00:00 2001 From: evilebottnawi Date: Thu, 21 Nov 2019 16:36:31 +0300 Subject: [PATCH 1/3] feat: getOptions util for loader --- lib/NormalModule.js | 47 +++++++++++ test/configCases/loaders/options/a.js | 0 test/configCases/loaders/options/b.js | 0 test/configCases/loaders/options/c.js | 0 test/configCases/loaders/options/d.js | 0 test/configCases/loaders/options/e.js | 0 test/configCases/loaders/options/f.js | 0 test/configCases/loaders/options/g.js | 0 test/configCases/loaders/options/h.js | 0 test/configCases/loaders/options/i.js | 0 test/configCases/loaders/options/index.js | 45 +++++++++++ test/configCases/loaders/options/loader-1.js | 12 +++ .../loaders/options/loader-1.options.json | 43 ++++++++++ test/configCases/loaders/options/loader.js | 9 +++ .../loaders/options/webpack.config.js | 78 +++++++++++++++++++ 15 files changed, 234 insertions(+) create mode 100644 test/configCases/loaders/options/a.js create mode 100644 test/configCases/loaders/options/b.js create mode 100644 test/configCases/loaders/options/c.js create mode 100644 test/configCases/loaders/options/d.js create mode 100644 test/configCases/loaders/options/e.js create mode 100644 test/configCases/loaders/options/f.js create mode 100644 test/configCases/loaders/options/g.js create mode 100644 test/configCases/loaders/options/h.js create mode 100644 test/configCases/loaders/options/i.js create mode 100644 test/configCases/loaders/options/index.js create mode 100644 test/configCases/loaders/options/loader-1.js create mode 100644 test/configCases/loaders/options/loader-1.options.json create mode 100644 test/configCases/loaders/options/loader.js create mode 100644 test/configCases/loaders/options/webpack.config.js diff --git a/lib/NormalModule.js b/lib/NormalModule.js index 4fafd5c53ad..1f8ea181866 100644 --- a/lib/NormalModule.js +++ b/lib/NormalModule.js @@ -5,7 +5,10 @@ "use strict"; +const parseJson = require("json-parse-better-errors"); const { getContext, runLoaders } = require("loader-runner"); +const querystring = require("querystring"); +const validateOptions = require("schema-utils"); const { SyncHook } = require("tapable"); const { CachedSource, @@ -350,6 +353,50 @@ class NormalModule extends Module { }; const loaderContext = { version: 2, + getOptions: (loaderName, schema) => { + const loader = loaderContext.loaders[loaderContext.loaderIndex]; + let options = {}; + + if (loader.options && typeof loader.options === "object") { + ({ options } = loader); + } else if (loader.query) { + let { query } = loader; + + if (query.substr(0, 1) !== "?") { + throw new WebpackError( + "A valid query string should begin with '?'" + ); + } + + query = query.substr(1); + + // Allow to use `?foo=bar` in `options` + if (query.substr(0, 1) === "?") { + query = query.substr(1); + } + + if (query.substr(0, 1) === "{" && query.substr(-1) === "}") { + try { + options = parseJson(query); + } catch (e) { + throw new WebpackError(`Cannot parse query string: ${e.message}`); + } + } else { + options = querystring.parse(query); + } + } + + if (!schema) { + return options; + } + + validateOptions(schema, options, { + name: loaderName || "Unknown Loader", + baseDataPath: "options" + }); + + return options; + }, emitWarning: warning => { if (!(warning instanceof Error)) { warning = new NonErrorEmittedError(warning); diff --git a/test/configCases/loaders/options/a.js b/test/configCases/loaders/options/a.js new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/configCases/loaders/options/b.js b/test/configCases/loaders/options/b.js new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/configCases/loaders/options/c.js b/test/configCases/loaders/options/c.js new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/configCases/loaders/options/d.js b/test/configCases/loaders/options/d.js new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/configCases/loaders/options/e.js b/test/configCases/loaders/options/e.js new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/configCases/loaders/options/f.js b/test/configCases/loaders/options/f.js new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/configCases/loaders/options/g.js b/test/configCases/loaders/options/g.js new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/configCases/loaders/options/h.js b/test/configCases/loaders/options/h.js new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/configCases/loaders/options/i.js b/test/configCases/loaders/options/i.js new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/configCases/loaders/options/index.js b/test/configCases/loaders/options/index.js new file mode 100644 index 00000000000..57ddc511038 --- /dev/null +++ b/test/configCases/loaders/options/index.js @@ -0,0 +1,45 @@ +it("should get options", function() { + expect(require("./a")).toStrictEqual({ + arg: true, + arg1: null, + arg3: 1234567890, + arg4: "string", + arg5: [1, 2, 3], + arg6: { foo: "value", bar: { baz: "other-value" } } + }); + expect(require("./b")).toStrictEqual({ + arg: true, + arg1: null, + arg3: 1234567890, + arg4: "string", + arg5: [1, 2, 3], + arg6: { foo: "value", bar: { baz: "other-value" } } + }); + expect(require("./c")).toStrictEqual({ + arg: true, + arg1: null, + arg3: 1234567890, + arg4: "string", + arg5: [1, 2, 3], + arg6: { foo: "value", bar: { baz: "other-value" } } + }); + expect(require("./d")).toStrictEqual({ + arg4: "text" + }); + expect(require("./e")).toStrictEqual({}); + expect(require("./f")).toStrictEqual({ + delicious: "", + name: "cheesecake", + slices: "8", + warm: "false" + }); + expect(require("./g")).toStrictEqual({ + "=": "=" + }); + expect(require("./h")).toStrictEqual({ + "foo": "bar" + }); + expect(require("./i")).toStrictEqual({ + "foo": "bar" + }); +}); diff --git a/test/configCases/loaders/options/loader-1.js b/test/configCases/loaders/options/loader-1.js new file mode 100644 index 00000000000..5c8b11efac1 --- /dev/null +++ b/test/configCases/loaders/options/loader-1.js @@ -0,0 +1,12 @@ +const schema = require('./loader-1.options'); + +module.exports = function() { + const options = this.getOptions('Loader Name', schema); + + const json = JSON.stringify(options) + .replace(/\u2028/g, '\\u2028') + .replace(/\u2029/g, '\\u2029'); + + + return `module.exports = ${json}`; +}; diff --git a/test/configCases/loaders/options/loader-1.options.json b/test/configCases/loaders/options/loader-1.options.json new file mode 100644 index 00000000000..3c86ba01025 --- /dev/null +++ b/test/configCases/loaders/options/loader-1.options.json @@ -0,0 +1,43 @@ +{ + "additionalProperties": false, + "properties": { + "arg": { + "type": "boolean" + }, + "arg1": { + "type": "null" + }, + "arg2": {}, + "arg3": { + "type": "number" + }, + "arg4": { + "type": "string" + }, + "arg5": { + "type": "array", + "items": { + "type": "number" + } + }, + "arg6": { + "type": "object", + "properties": { + "foo": { + "type": "string" + }, + "bar": { + "type": "object", + "properties": { + "baz": { + "type": "string" + } + }, + "additionalProperties": false + } + }, + "additionalProperties": false + } + }, + "type": "object" +} diff --git a/test/configCases/loaders/options/loader.js b/test/configCases/loaders/options/loader.js new file mode 100644 index 00000000000..5b9386651be --- /dev/null +++ b/test/configCases/loaders/options/loader.js @@ -0,0 +1,9 @@ +module.exports = function() { + const options = this.getOptions(); + + const json = JSON.stringify(options) + .replace(/\u2028/g, '\\u2028') + .replace(/\u2029/g, '\\u2029'); + + return `module.exports = ${json}`; +}; diff --git a/test/configCases/loaders/options/webpack.config.js b/test/configCases/loaders/options/webpack.config.js new file mode 100644 index 00000000000..0f343bd7b0f --- /dev/null +++ b/test/configCases/loaders/options/webpack.config.js @@ -0,0 +1,78 @@ +module.exports = { + mode: "none", + module: { + rules: [ + { + test: /a\.js$/, + loader: "./loader", + options: { + arg: true, + arg1: null, + arg2: undefined, + arg3: 1234567890, + arg4: "string", + arg5: [1, 2, 3], + arg6: { foo: "value", bar: { baz: "other-value" } } + } + }, + { + test: /b\.js$/, + loader: "./loader-1", + options: { + arg: true, + arg1: null, + arg2: undefined, + arg3: 1234567890, + arg4: "string", + arg5: [1, 2, 3], + arg6: { foo: "value", bar: { baz: "other-value" } } + } + }, + { + test: /c\.js$/, + loader: "./loader-1", + options: JSON.stringify({ + arg: true, + arg1: null, + arg2: undefined, + arg3: 1234567890, + arg4: "string", + arg5: [1, 2, 3], + arg6: { foo: "value", bar: { baz: "other-value" } } + }) + }, + { + test: /d\.js$/, + loader: "./loader-1", + options: "arg4=text" + }, + { + test: /d\.js$/, + loader: "./loader", + options: "?" + }, + { + test: /f\.js$/, + loader: "./loader", + options: "name=cheesecake&slices=8&delicious&warm=false" + }, + { + test: /g\.js$/, + loader: "./loader", + options: "%3d=%3D" + }, + { + test: /h\.js$/, + loader: "./loader", + options: "?foo=bar" + }, + { + test: /i\.js$/, + loader: "./loader", + options: `?${JSON.stringify({ + foo: "bar" + })}` + } + ] + } +}; From f9846f1f91f1827fa351febad788f0d38b8e6115 Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Thu, 16 Jan 2020 14:48:52 +0100 Subject: [PATCH 2/3] improve code, remove unneeded old stuff --- lib/NormalModule.js | 59 +++++++++---------- .../loaders/options/deprecations.js | 9 +++ test/configCases/loaders/options/loader-1.js | 9 ++- .../loaders/options/webpack.config.js | 6 +- 4 files changed, 43 insertions(+), 40 deletions(-) create mode 100644 test/configCases/loaders/options/deprecations.js diff --git a/lib/NormalModule.js b/lib/NormalModule.js index 1f8ea181866..3152d76d2ed 100644 --- a/lib/NormalModule.js +++ b/lib/NormalModule.js @@ -52,6 +52,13 @@ const makeSerializable = require("./util/makeSerializable"); /** @typedef {import("./util/Hash")} Hash */ /** @typedef {import("./util/fs").InputFileSystem} InputFileSystem */ +/** + * @typedef {Object} LoaderItem + * @property {string} loader + * @property {any} options + * @property {string?} ident + */ + /** * @param {string} context absolute context path * @param {string} source a source path @@ -169,7 +176,7 @@ class NormalModule extends Module { * @param {string} options.request request string * @param {string} options.userRequest request intented by user (without loaders from config) * @param {string} options.rawRequest request without resolving - * @param {TODO[]} options.loaders list of loaders + * @param {LoaderItem[]} options.loaders list of loaders * @param {string} options.resource path + query of the real resource * @param {string | undefined} options.matchResource path + query of the matched resource (virtuel * @param {Parser} options.parser the parser used @@ -207,7 +214,7 @@ class NormalModule extends Module { this.resource = resource; /** @type {string | undefined} */ this.matchResource = matchResource; - /** @type {TODO[]} */ + /** @type {LoaderItem[]} */ this.loaders = loaders; if (resolveOptions !== undefined) { // already declared in super class @@ -353,47 +360,35 @@ class NormalModule extends Module { }; const loaderContext = { version: 2, - getOptions: (loaderName, schema) => { - const loader = loaderContext.loaders[loaderContext.loaderIndex]; - let options = {}; - - if (loader.options && typeof loader.options === "object") { - ({ options } = loader); - } else if (loader.query) { - let { query } = loader; - - if (query.substr(0, 1) !== "?") { - throw new WebpackError( - "A valid query string should begin with '?'" - ); - } + getOptions: schema => { + const loader = this.getCurrentLoader(loaderContext); - query = query.substr(1); + let { options } = loader; - // Allow to use `?foo=bar` in `options` - if (query.substr(0, 1) === "?") { - query = query.substr(1); - } - - if (query.substr(0, 1) === "{" && query.substr(-1) === "}") { + if (typeof options === "string") { + if (options.substr(0, 1) === "{" && options.substr(-1) === "}") { try { - options = parseJson(query); + options = parseJson(options); } catch (e) { - throw new WebpackError(`Cannot parse query string: ${e.message}`); + throw new Error(`Cannot parse string options: ${e.message}`); } } else { - options = querystring.parse(query); + options = querystring.parse(options, "&", "=", { + maxKeys: 0 + }); } } - if (!schema) { - return options; + if (options === null || options === undefined) { + options = {}; } - validateOptions(schema, options, { - name: loaderName || "Unknown Loader", - baseDataPath: "options" - }); + if (schema) { + validateOptions(schema, options, { + name: getCurrentLoaderName(), + baseDataPath: "options" + }); + } return options; }, diff --git a/test/configCases/loaders/options/deprecations.js b/test/configCases/loaders/options/deprecations.js new file mode 100644 index 00000000000..6c3c0c2f1b2 --- /dev/null +++ b/test/configCases/loaders/options/deprecations.js @@ -0,0 +1,9 @@ +module.exports = [ + { code: /DEP_WEBPACK_RULE_LOADER_OPTIONS_STRING/ }, + { code: /DEP_WEBPACK_RULE_LOADER_OPTIONS_STRING/ }, + { code: /DEP_WEBPACK_RULE_LOADER_OPTIONS_STRING/ }, + { code: /DEP_WEBPACK_RULE_LOADER_OPTIONS_STRING/ }, + { code: /DEP_WEBPACK_RULE_LOADER_OPTIONS_STRING/ }, + { code: /DEP_WEBPACK_RULE_LOADER_OPTIONS_STRING/ }, + { code: /DEP_WEBPACK_RULE_LOADER_OPTIONS_STRING/ } +]; diff --git a/test/configCases/loaders/options/loader-1.js b/test/configCases/loaders/options/loader-1.js index 5c8b11efac1..f27763418ab 100644 --- a/test/configCases/loaders/options/loader-1.js +++ b/test/configCases/loaders/options/loader-1.js @@ -1,12 +1,11 @@ -const schema = require('./loader-1.options'); +const schema = require("./loader-1.options"); module.exports = function() { - const options = this.getOptions('Loader Name', schema); + const options = this.getOptions(schema); const json = JSON.stringify(options) - .replace(/\u2028/g, '\\u2028') - .replace(/\u2029/g, '\\u2029'); - + .replace(/\u2028/g, "\\u2028") + .replace(/\u2029/g, "\\u2029"); return `module.exports = ${json}`; }; diff --git a/test/configCases/loaders/options/webpack.config.js b/test/configCases/loaders/options/webpack.config.js index 0f343bd7b0f..4b454014876 100644 --- a/test/configCases/loaders/options/webpack.config.js +++ b/test/configCases/loaders/options/webpack.config.js @@ -49,7 +49,7 @@ module.exports = { { test: /d\.js$/, loader: "./loader", - options: "?" + options: "" }, { test: /f\.js$/, @@ -64,12 +64,12 @@ module.exports = { { test: /h\.js$/, loader: "./loader", - options: "?foo=bar" + options: "foo=bar" }, { test: /i\.js$/, loader: "./loader", - options: `?${JSON.stringify({ + options: `${JSON.stringify({ foo: "bar" })}` } From d673e4179b42144683b79db9ae8244858aeda220 Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Fri, 17 Jan 2020 12:17:53 +0100 Subject: [PATCH 3/3] test errors message, allow custom names in schema --- lib/NormalModule.js | 10 ++++++++-- test/configCases/loaders/options/error1.js | 0 test/configCases/loaders/options/error2.js | 0 test/configCases/loaders/options/errors.js | 12 ++++++++++++ test/configCases/loaders/options/index.js | 10 ++++++++-- test/configCases/loaders/options/loader-2.js | 11 +++++++++++ .../loaders/options/loader-2.options.json | 10 ++++++++++ test/configCases/loaders/options/webpack.config.js | 14 ++++++++++++++ 8 files changed, 63 insertions(+), 4 deletions(-) create mode 100644 test/configCases/loaders/options/error1.js create mode 100644 test/configCases/loaders/options/error2.js create mode 100644 test/configCases/loaders/options/errors.js create mode 100644 test/configCases/loaders/options/loader-2.js create mode 100644 test/configCases/loaders/options/loader-2.options.json diff --git a/lib/NormalModule.js b/lib/NormalModule.js index 3152d76d2ed..e28bca68db1 100644 --- a/lib/NormalModule.js +++ b/lib/NormalModule.js @@ -384,9 +384,15 @@ class NormalModule extends Module { } if (schema) { + let name = "Loader"; + let baseDataPath = "options"; + let match; + if (schema.title && (match = /^(.+) (.+)$/.exec(schema.title))) { + [, name, baseDataPath] = match; + } validateOptions(schema, options, { - name: getCurrentLoaderName(), - baseDataPath: "options" + name, + baseDataPath }); } diff --git a/test/configCases/loaders/options/error1.js b/test/configCases/loaders/options/error1.js new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/configCases/loaders/options/error2.js b/test/configCases/loaders/options/error2.js new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/configCases/loaders/options/errors.js b/test/configCases/loaders/options/errors.js new file mode 100644 index 00000000000..3ea73741041 --- /dev/null +++ b/test/configCases/loaders/options/errors.js @@ -0,0 +1,12 @@ +module.exports = [ + [ + /\.\/loader-1\.js/, + /Loader has been/, + /options\.arg6\.bar\.baz should be a string/ + ], + [ + /\.\/loader-2\.js/, + /Custom Loader Name has been/, + /configuration\.arg should be true/ + ] +]; diff --git a/test/configCases/loaders/options/index.js b/test/configCases/loaders/options/index.js index 57ddc511038..56896b7c2ef 100644 --- a/test/configCases/loaders/options/index.js +++ b/test/configCases/loaders/options/index.js @@ -37,9 +37,15 @@ it("should get options", function() { "=": "=" }); expect(require("./h")).toStrictEqual({ - "foo": "bar" + foo: "bar" }); expect(require("./i")).toStrictEqual({ - "foo": "bar" + foo: "bar" }); }); + +const never = false; +if (never) { + require("./error1"); + require("./error2"); +} diff --git a/test/configCases/loaders/options/loader-2.js b/test/configCases/loaders/options/loader-2.js new file mode 100644 index 00000000000..b1690265227 --- /dev/null +++ b/test/configCases/loaders/options/loader-2.js @@ -0,0 +1,11 @@ +const schema = require("./loader-2.options"); + +module.exports = function() { + const options = this.getOptions(schema); + + const json = JSON.stringify(options) + .replace(/\u2028/g, "\\u2028") + .replace(/\u2029/g, "\\u2029"); + + return `module.exports = ${json}`; +}; diff --git a/test/configCases/loaders/options/loader-2.options.json b/test/configCases/loaders/options/loader-2.options.json new file mode 100644 index 00000000000..d17814f5fcf --- /dev/null +++ b/test/configCases/loaders/options/loader-2.options.json @@ -0,0 +1,10 @@ +{ + "title": "Custom Loader Name configuration", + "additionalProperties": false, + "properties": { + "arg": { + "enum": [true] + } + }, + "type": "object" +} diff --git a/test/configCases/loaders/options/webpack.config.js b/test/configCases/loaders/options/webpack.config.js index 4b454014876..c7870d8e6b6 100644 --- a/test/configCases/loaders/options/webpack.config.js +++ b/test/configCases/loaders/options/webpack.config.js @@ -72,6 +72,20 @@ module.exports = { options: `${JSON.stringify({ foo: "bar" })}` + }, + { + test: /error1\.js$/, + loader: "./loader-1", + options: { + arg6: { foo: "value", bar: { baz: 42 } } + } + }, + { + test: /error2\.js$/, + loader: "./loader-2", + options: { + arg: false + } } ] }