From 7e6fbf2c26ec1db79d64c859863b4dbf8e15dafe Mon Sep 17 00:00:00 2001 From: goganchic Date: Fri, 13 Sep 2019 12:07:54 +0300 Subject: [PATCH 1/8] refactor: remove excessive constants --- package-lock.json | 41 +++++++++++------------------------------ src/index.js | 43 +++++++++++++++++++++++-------------------- 2 files changed, 34 insertions(+), 50 deletions(-) diff --git a/package-lock.json b/package-lock.json index d40571e2..e5032931 100644 --- a/package-lock.json +++ b/package-lock.json @@ -5773,8 +5773,7 @@ "ansi-regex": { "version": "2.1.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "aproba": { "version": "1.2.0", @@ -5795,14 +5794,12 @@ "balanced-match": { "version": "1.0.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "brace-expansion": { "version": "1.1.11", "bundled": true, "dev": true, - "optional": true, "requires": { "balanced-match": "^1.0.0", "concat-map": "0.0.1" @@ -5817,20 +5814,17 @@ "code-point-at": { "version": "1.1.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "concat-map": { "version": "0.0.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "console-control-strings": { "version": "1.1.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "core-util-is": { "version": "1.0.2", @@ -5947,8 +5941,7 @@ "inherits": { "version": "2.0.3", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "ini": { "version": "1.3.5", @@ -5960,7 +5953,6 @@ "version": "1.0.0", "bundled": true, "dev": true, - "optional": true, "requires": { "number-is-nan": "^1.0.0" } @@ -5975,7 +5967,6 @@ "version": "3.0.4", "bundled": true, "dev": true, - "optional": true, "requires": { "brace-expansion": "^1.1.7" } @@ -5983,14 +5974,12 @@ "minimist": { "version": "0.0.8", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "minipass": { "version": "2.3.5", "bundled": true, "dev": true, - "optional": true, "requires": { "safe-buffer": "^5.1.2", "yallist": "^3.0.0" @@ -6009,7 +5998,6 @@ "version": "0.5.1", "bundled": true, "dev": true, - "optional": true, "requires": { "minimist": "0.0.8" } @@ -6090,8 +6078,7 @@ "number-is-nan": { "version": "1.0.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "object-assign": { "version": "4.1.1", @@ -6103,7 +6090,6 @@ "version": "1.4.0", "bundled": true, "dev": true, - "optional": true, "requires": { "wrappy": "1" } @@ -6189,8 +6175,7 @@ "safe-buffer": { "version": "5.1.2", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "safer-buffer": { "version": "2.1.2", @@ -6226,7 +6211,6 @@ "version": "1.0.2", "bundled": true, "dev": true, - "optional": true, "requires": { "code-point-at": "^1.0.0", "is-fullwidth-code-point": "^1.0.0", @@ -6246,7 +6230,6 @@ "version": "3.0.1", "bundled": true, "dev": true, - "optional": true, "requires": { "ansi-regex": "^2.0.0" } @@ -6290,14 +6273,12 @@ "wrappy": { "version": "1.0.2", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "yallist": { "version": "3.0.3", "bundled": true, - "dev": true, - "optional": true + "dev": true } } }, diff --git a/src/index.js b/src/index.js index 1ce31fda..8fad3d1b 100644 --- a/src/index.js +++ b/src/index.js @@ -148,29 +148,32 @@ class MiniCssExtractPlugin { apply(compiler) { compiler.hooks.thisCompilation.tap(pluginName, (compilation) => { - compilation.hooks.normalModuleLoader.tap(pluginName, (lc, m) => { - const loaderContext = lc; - const module = m; - - loaderContext[MODULE_TYPE] = (content) => { - if (!Array.isArray(content) && content != null) { - throw new Error( - `Exported value was not extracted as an array: ${JSON.stringify( - content - )}` - ); - } + compilation.hooks.normalModuleLoader.tap( + pluginName, + (loaderContext, module) => { + // eslint-disable-next-line no-param-reassign + loaderContext[MODULE_TYPE] = (content) => { + if (!Array.isArray(content) && content != null) { + throw new Error( + `Exported value was not extracted as an array: ${JSON.stringify( + content + )}` + ); + } - const identifierCountMap = new Map(); + const identifierCountMap = new Map(); - for (const line of content) { - const count = identifierCountMap.get(line.identifier) || 0; + for (const line of content) { + const count = identifierCountMap.get(line.identifier) || 0; - module.addDependency(new CssDependency(line, m.context, count)); - identifierCountMap.set(line.identifier, count + 1); - } - }; - }); + module.addDependency( + new CssDependency(line, module.context, count) + ); + identifierCountMap.set(line.identifier, count + 1); + } + }; + } + ); compilation.dependencyFactories.set( CssDependency, From 49199bff2c38d25634ca8ad8da94a93ec10286ac Mon Sep 17 00:00:00 2001 From: goganchic Date: Fri, 13 Sep 2019 12:11:21 +0300 Subject: [PATCH 2/8] refactor: remove outdated comment --- src/loader.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/loader.js b/src/loader.js index a91dd787..3dfe9890 100644 --- a/src/loader.js +++ b/src/loader.js @@ -91,8 +91,6 @@ export function pitch(request) { ); new LimitChunkCountPlugin({ maxChunks: 1 }).apply(childCompiler); - // We set loaderContext[MODULE_TYPE] = false to indicate we already in - // a child compiler so we don't spawn another child compilers from there. childCompiler.hooks.thisCompilation.tap( `${pluginName} loader`, (compilation) => { @@ -101,7 +99,6 @@ export function pitch(request) { (loaderContext, module) => { // eslint-disable-next-line no-param-reassign loaderContext.emitFile = this.emitFile; - loaderContext[MODULE_TYPE] = false; // eslint-disable-line no-param-reassign if (module.request === request) { // eslint-disable-next-line no-param-reassign From 93724d0aef4d6121e0128a3b7cd8f541388161d5 Mon Sep 17 00:00:00 2001 From: goganchic Date: Fri, 13 Sep 2019 12:27:29 +0300 Subject: [PATCH 3/8] refactor: move dependency adding function into loader --- src/CssDependency.js | 22 +++++++++++++++++++ src/index.js | 50 ++------------------------------------------ src/loader.js | 26 +++++++++++++++++++++-- 3 files changed, 48 insertions(+), 50 deletions(-) create mode 100644 src/CssDependency.js diff --git a/src/CssDependency.js b/src/CssDependency.js new file mode 100644 index 00000000..3e2ff817 --- /dev/null +++ b/src/CssDependency.js @@ -0,0 +1,22 @@ +import webpack from 'webpack'; + +export default class CssDependency extends webpack.Dependency { + constructor( + { identifier, content, media, sourceMap }, + context, + identifierIndex + ) { + super(); + + this.identifier = identifier; + this.identifierIndex = identifierIndex; + this.content = content; + this.media = media; + this.sourceMap = sourceMap; + this.context = context; + } + + getResourceIdentifier() { + return `css-module-${this.identifier}-${this.identifierIndex}`; + } +} diff --git a/src/index.js b/src/index.js index 8fad3d1b..cec08e21 100644 --- a/src/index.js +++ b/src/index.js @@ -3,6 +3,8 @@ import webpack from 'webpack'; import sources from 'webpack-sources'; +import CssDependency from './CssDependency'; + const { ConcatSource, SourceMapSource, OriginalSource } = sources; const { Template, @@ -19,27 +21,6 @@ const REGEXP_NAME = /\[name\]/i; const REGEXP_PLACEHOLDERS = /\[(name|id|chunkhash)\]/g; const DEFAULT_FILENAME = '[name].css'; -class CssDependency extends webpack.Dependency { - constructor( - { identifier, content, media, sourceMap }, - context, - identifierIndex - ) { - super(); - - this.identifier = identifier; - this.identifierIndex = identifierIndex; - this.content = content; - this.media = media; - this.sourceMap = sourceMap; - this.context = context; - } - - getResourceIdentifier() { - return `css-module-${this.identifier}-${this.identifierIndex}`; - } -} - class CssDependencyTemplate { apply() {} } @@ -148,33 +129,6 @@ class MiniCssExtractPlugin { apply(compiler) { compiler.hooks.thisCompilation.tap(pluginName, (compilation) => { - compilation.hooks.normalModuleLoader.tap( - pluginName, - (loaderContext, module) => { - // eslint-disable-next-line no-param-reassign - loaderContext[MODULE_TYPE] = (content) => { - if (!Array.isArray(content) && content != null) { - throw new Error( - `Exported value was not extracted as an array: ${JSON.stringify( - content - )}` - ); - } - - const identifierCountMap = new Map(); - - for (const line of content) { - const count = identifierCountMap.get(line.identifier) || 0; - - module.addDependency( - new CssDependency(line, module.context, count) - ); - identifierCountMap.set(line.identifier, count + 1); - } - }; - } - ); - compilation.dependencyFactories.set( CssDependency, new CssModuleFactory() diff --git a/src/loader.js b/src/loader.js index 3dfe9890..0ed0f55c 100644 --- a/src/loader.js +++ b/src/loader.js @@ -10,9 +10,10 @@ import SingleEntryPlugin from 'webpack/lib/SingleEntryPlugin'; import LimitChunkCountPlugin from 'webpack/lib/optimize/LimitChunkCountPlugin'; import validateOptions from 'schema-utils'; +import CssDependency from './CssDependency'; + import schema from './options.json'; -const MODULE_TYPE = 'css/mini-extract'; const pluginName = 'mini-css-extract-plugin'; function hotLoader(content, context) { @@ -133,6 +134,27 @@ export function pitch(request) { const callback = this.async(); childCompiler.runAsChild((err, entries, compilation) => { + const addDependencies = (content) => { + if (!Array.isArray(content) && content != null) { + throw new Error( + `Exported value was not extracted as an array: ${JSON.stringify( + content + )}` + ); + } + + const identifierCountMap = new Map(); + + for (const line of content) { + const count = identifierCountMap.get(line.identifier) || 0; + + this._module.addDependency( + new CssDependency(line, module.context, count) + ); + identifierCountMap.set(line.identifier, count + 1); + } + }; + if (err) { return callback(err); } @@ -173,7 +195,7 @@ export function pitch(request) { }; }); } - this[MODULE_TYPE](text); + addDependencies(text); } catch (e) { return callback(e); } From 0929269b8d97440e1683468b95a48e47341f1fcc Mon Sep 17 00:00:00 2001 From: goganchic Date: Fri, 13 Sep 2019 12:34:39 +0300 Subject: [PATCH 4/8] refactor: functions style unification --- src/loader.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/loader.js b/src/loader.js index 0ed0f55c..ccc83d25 100644 --- a/src/loader.js +++ b/src/loader.js @@ -37,7 +37,7 @@ function hotLoader(content, context) { `; } -const exec = (loaderContext, code, filename) => { +function exec(loaderContext, code, filename) { const module = new NativeModule(filename, loaderContext); module.paths = NativeModule._nodeModulePaths(loaderContext.context); // eslint-disable-line no-underscore-dangle @@ -45,9 +45,9 @@ const exec = (loaderContext, code, filename) => { module._compile(code, filename); // eslint-disable-line no-underscore-dangle return module.exports; -}; +} -const findModuleById = (modules, id) => { +function findModuleById(modules, id) { for (const module of modules) { if (module.id === id) { return module; @@ -55,7 +55,7 @@ const findModuleById = (modules, id) => { } return null; -}; +} export function pitch(request) { const options = loaderUtils.getOptions(this) || {}; From fc647160b1b8727c688fe893f03be695a4b4671a Mon Sep 17 00:00:00 2001 From: goganchic Date: Fri, 13 Sep 2019 13:16:26 +0300 Subject: [PATCH 5/8] refactor: rename variables names to reduce misunderstanding --- src/loader.js | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/loader.js b/src/loader.js index ccc83d25..60659c23 100644 --- a/src/loader.js +++ b/src/loader.js @@ -37,7 +37,7 @@ function hotLoader(content, context) { `; } -function exec(loaderContext, code, filename) { +function evalModuleCode(loaderContext, code, filename) { const module = new NativeModule(filename, loaderContext); module.paths = NativeModule._nodeModulePaths(loaderContext.context); // eslint-disable-line no-underscore-dangle @@ -175,27 +175,27 @@ export function pitch(request) { return callback(new Error("Didn't get a result from child compiler")); } - let text; let locals; try { - text = exec(this, source, request); - locals = text && text.locals; - if (!Array.isArray(text)) { - text = [[null, text]]; + let dependencies; + const exports = evalModuleCode(this, source, request); + locals = exports && exports.locals; + if (!Array.isArray(exports)) { + dependencies = [[null, exports]]; } else { - text = text.map((line) => { - const module = findModuleById(compilation.modules, line[0]); + dependencies = exports.map(([id, content, media, sourceMap]) => { + const module = findModuleById(compilation.modules, id); return { identifier: module.identifier(), - content: line[1], - media: line[2], - sourceMap: line[3], + content, + media, + sourceMap, }; }); } - addDependencies(text); + addDependencies(dependencies); } catch (e) { return callback(e); } From 53aeda89c6edd27afad5bd85c310fb97741ffdfa Mon Sep 17 00:00:00 2001 From: goganchic Date: Fri, 13 Sep 2019 15:58:18 +0300 Subject: [PATCH 6/8] refactor: remove unnecessary webpack comment --- src/loader.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/loader.js b/src/loader.js index 60659c23..0b81f2d7 100644 --- a/src/loader.js +++ b/src/loader.js @@ -66,7 +66,7 @@ export function pitch(request) { this.addDependency(this.resourcePath); - const childFilename = '*'; // eslint-disable-line no-path-concat + const childFilename = '*'; const publicPath = typeof options.publicPath === 'string' ? options.publicPath === '' || options.publicPath.endsWith('/') From 26fbea0fa99c752e62fb781d34d801f72184d6c0 Mon Sep 17 00:00:00 2001 From: goganchic Date: Fri, 13 Sep 2019 16:19:52 +0300 Subject: [PATCH 7/8] refactor: more clear names for local variables of addDependencies func --- src/loader.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/loader.js b/src/loader.js index 0b81f2d7..8e5ac5a5 100644 --- a/src/loader.js +++ b/src/loader.js @@ -134,24 +134,24 @@ export function pitch(request) { const callback = this.async(); childCompiler.runAsChild((err, entries, compilation) => { - const addDependencies = (content) => { - if (!Array.isArray(content) && content != null) { + const addDependencies = (dependencies) => { + if (!Array.isArray(dependencies) && dependencies != null) { throw new Error( `Exported value was not extracted as an array: ${JSON.stringify( - content + dependencies )}` ); } const identifierCountMap = new Map(); - for (const line of content) { - const count = identifierCountMap.get(line.identifier) || 0; + for (const dependency of dependencies) { + const count = identifierCountMap.get(dependency.identifier) || 0; this._module.addDependency( - new CssDependency(line, module.context, count) + new CssDependency(dependency, module.context, count) ); - identifierCountMap.set(line.identifier, count + 1); + identifierCountMap.set(dependency.identifier, count + 1); } }; From b134f00a939e716867a1906a241492a52aa86f4c Mon Sep 17 00:00:00 2001 From: goganchic Date: Sat, 14 Sep 2019 12:41:14 +0300 Subject: [PATCH 8/8] refactor: update eslint-utils --- package-lock.json | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/package-lock.json b/package-lock.json index e5032931..ea27369a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -5023,10 +5023,9 @@ } }, "eslint-utils": { - "version": "1.4.0", - "resolved": "https://registry.npmjs.org/eslint-utils/-/eslint-utils-1.4.0.tgz", - "integrity": "sha512-7ehnzPaP5IIEh1r1tkjuIrxqhNkzUJa9z3R92tLJdZIVdWaczEhr3EbhGtsMrVxi1KeR8qA7Off6SWc5WNQqyQ==", - "dev": true, + "version": "1.4.2", + "resolved": "https://registry.npmjs.org/eslint-utils/-/eslint-utils-1.4.2.tgz", + "integrity": "sha512-eAZS2sEUMlIeCjBeubdj45dmBHQwPHWyBcT1VSYB7o9x9WRRqKxyUoiXlRjyAwzN7YEzHJlYg0NmzDRWx6GP4Q==", "requires": { "eslint-visitor-keys": "^1.0.0" } @@ -5034,8 +5033,7 @@ "eslint-visitor-keys": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/eslint-visitor-keys/-/eslint-visitor-keys-1.0.0.tgz", - "integrity": "sha512-qzm/XxIbxm/FHyH341ZrbnMUpe+5Bocte9xkmFMzPMjRaZMcXww+MpBptFvtU+79L362nqiLhekCxCxDPaUMBQ==", - "dev": true + "integrity": "sha512-qzm/XxIbxm/FHyH341ZrbnMUpe+5Bocte9xkmFMzPMjRaZMcXww+MpBptFvtU+79L362nqiLhekCxCxDPaUMBQ==" }, "espree": { "version": "6.0.0",