fix: avoid mutations of options and config (#470)

webpack-contrib · Sep 7, 2020 · cad6f07 · cad6f07
1 parent 77449e1
commit cad6f07
Show file tree

Hide file tree

Showing 13 changed files with 114 additions and 79 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -44,6 +44,7 @@
   },
   "dependencies": {
     "cosmiconfig": "^7.0.0",
+    "klona": "^2.0.3",
     "loader-utils": "^2.0.0",
     "schema-utils": "^2.7.1",
     "semver": "^7.3.2"

diff --git a/src/index.js b/src/index.js
@@ -45,7 +45,8 @@ export default async function loader(content, sourceMap, meta) {
     typeof options.postcssOptions.config === 'undefined'
       ? true
       : options.postcssOptions.config;
-  let loadedConfig = {};
+
+  let loadedConfig;
 
   if (configOption) {
     try {

diff --git a/src/utils.js b/src/utils.js
@@ -1,6 +1,7 @@
 import path from 'path';
 import Module from 'module';
 
+import { klona } from 'klona/full';
 import { cosmiconfig } from 'cosmiconfig';
 
 const parentModule = module;
@@ -41,7 +42,7 @@ async function loadConfig(loaderContext, config) {
   try {
     stats = await stat(loaderContext.fs, searchPath);
   } catch (errorIgnore) {
-    throw new Error(`No PostCSS Config found in: ${searchPath}`);
+    throw new Error(`No PostCSS config found in: ${searchPath}`);
   }
 
   const explorer = cosmiconfig('postcss');
@@ -62,25 +63,26 @@ async function loadConfig(loaderContext, config) {
     return {};
   }
 
-  let resultConfig = result.config || {};
+  loaderContext.addDependency(result.filepath);
 
-  if (typeof resultConfig === 'function') {
+  if (result.isEmpty) {
+    return result;
+  }
+
+  if (typeof result.config === 'function') {
     const api = {
-      env: process.env.NODE_ENV,
       mode: loaderContext.mode,
       file: loaderContext.resourcePath,
       // For complex use
       webpackLoaderContext: loaderContext,
     };
 
-    resultConfig = resultConfig(api);
+    result.config = result.config(api);
   }
 
-  resultConfig.file = result.filepath;
-
-  loaderContext.addDependency(resultConfig.file);
+  result = klona(result);
 
-  return resultConfig;
+  return result;
 }
 
 function loadPlugin(plugin, options, file) {
@@ -160,7 +162,11 @@ function pluginFactory() {
   };
 }
 
-function getPostcssOptions(loaderContext, config, postcssOptions = {}) {
+function getPostcssOptions(
+  loaderContext,
+  loadedConfig = {},
+  postcssOptions = {}
+) {
   const file = loaderContext.resourcePath;
 
   let normalizedPostcssOptions = postcssOptions;
@@ -174,7 +180,10 @@ function getPostcssOptions(loaderContext, config, postcssOptions = {}) {
   try {
     const factory = pluginFactory();
 
-    factory(config.plugins);
+    if (loadedConfig.config && loadedConfig.config.plugins) {
+      factory(loadedConfig.config.plugins);
+    }
+
     factory(normalizedPostcssOptions.plugins);
 
     plugins = [...factory()].map((item) => {
@@ -190,27 +199,26 @@ function getPostcssOptions(loaderContext, config, postcssOptions = {}) {
     loaderContext.emitError(error);
   }
 
-  const processOptionsFromConfig = { ...config };
+  const processOptionsFromConfig = loadedConfig.config || {};
 
   if (processOptionsFromConfig.from) {
     processOptionsFromConfig.from = path.resolve(
-      path.dirname(config.file),
+      path.dirname(loadedConfig.filepath),
       processOptionsFromConfig.from
     );
   }
 
   if (processOptionsFromConfig.to) {
     processOptionsFromConfig.to = path.resolve(
-      path.dirname(config.file),
+      path.dirname(loadedConfig.filepath),
       processOptionsFromConfig.to
     );
   }
 
   // No need them for processOptions
   delete processOptionsFromConfig.plugins;
-  delete processOptionsFromConfig.file;
 
-  const processOptionsFromOptions = { ...normalizedPostcssOptions };
+  const processOptionsFromOptions = klona(normalizedPostcssOptions);
 
   if (processOptionsFromOptions.from) {
     processOptionsFromOptions.from = path.resolve(

diff --git a/test/__snapshots__/postcssOptins.test.js.snap b/test/__snapshots__/postcssOptins.test.js.snap
@@ -23,7 +23,7 @@ exports[`"postcssOptions" option should throw an error with the "config" option
 exports[`"postcssOptions" option should throw an error with the "config" option on the unresolved config: errors 1`] = `
 Array [
   "ModuleBuildError: Module build failed (from \`replaced original path\`):
-Error: No PostCSS Config found in: /test/fixtures/config-scope/css/unresolve.js",
+Error: No PostCSS config found in: /test/fixtures/config-scope/css/unresolve.js",
 ]
 `;
 

diff --git a/test/config-autoload.test.js b/test/config-autoload.test.js
@@ -12,96 +12,93 @@ const loaderContext = {
 
 describe('autoload config', () => {
   it('should load ".postcssrc"', async () => {
-    const expected = (config) => {
-      expect(config.map).toEqual(false);
-      expect(config.from).toEqual('./test/rc/fixtures/index.css');
-      expect(config.to).toEqual('./test/rc/expect/index.css');
-      expect(Object.keys(config.plugins).length).toEqual(2);
-      expect(config.file).toEqual(
-        path.resolve(testDirectory, 'rc', '.postcssrc')
-      );
-    };
-
-    const config = await loadConfig(
+    const loadedConfig = await loadConfig(
       loaderContext,
       path.resolve(testDirectory, 'rc')
     );
 
-    expected(config);
+    expect(loadedConfig.config.map).toEqual(false);
+    expect(loadedConfig.config.from).toEqual('./test/rc/fixtures/index.css');
+    expect(loadedConfig.config.to).toEqual('./test/rc/expect/index.css');
+    expect(Object.keys(loadedConfig.config.plugins).length).toEqual(2);
+    expect(loadedConfig.filepath).toEqual(
+      path.resolve(testDirectory, 'rc', '.postcssrc')
+    );
   });
 
   it('should load "package.json"', async () => {
-    const expected = (config) => {
-      expect(config.parser).toEqual(false);
-      expect(config.syntax).toEqual(false);
-      expect(config.map).toEqual(false);
-      expect(config.from).toEqual('./index.css');
-      expect(config.to).toEqual('./index.css');
-      expect(Object.keys(config.plugins).length).toEqual(2);
-      expect(config.file).toEqual(
-        path.resolve(testDirectory, 'pkg', 'package.json')
-      );
-    };
-
-    const config = await loadConfig(
+    const loadedConfig = await loadConfig(
       loaderContext,
       path.resolve(testDirectory, 'pkg')
     );
 
-    expected(config);
+    expect(loadedConfig.config.parser).toEqual(false);
+    expect(loadedConfig.config.syntax).toEqual(false);
+    expect(loadedConfig.config.map).toEqual(false);
+    expect(loadedConfig.config.from).toEqual('./index.css');
+    expect(loadedConfig.config.to).toEqual('./index.css');
+    expect(Object.keys(loadedConfig.config.plugins).length).toEqual(2);
+    expect(loadedConfig.filepath).toEqual(
+      path.resolve(testDirectory, 'pkg', 'package.json')
+    );
   });
 
   it('should load "postcss.config.js" with "Object" syntax of plugins', async () => {
-    const expected = (config) => {
-      expect(config.map).toEqual(false);
-      expect(config.from).toEqual(
-        './test/fixtures/config-autoload/js/object/index.css'
-      );
-      expect(config.to).toEqual(
-        './test/fixtures/config-autoload/js/object/expect/index.css'
-      );
-      expect(Object.keys(config.plugins).length).toEqual(2);
-      expect(config.file).toEqual(
-        path.resolve(testDirectory, 'js/object', 'postcss.config.js')
-      );
-    };
-
-    const config = await loadConfig(
+    const loadedConfig = await loadConfig(
       loaderContext,
       path.resolve(testDirectory, 'js/object')
     );
 
-    expected(config);
+    expect(loadedConfig.config.map).toEqual(false);
+    expect(loadedConfig.config.from).toEqual(
+      './test/fixtures/config-autoload/js/object/index.css'
+    );
+    expect(loadedConfig.config.to).toEqual(
+      './test/fixtures/config-autoload/js/object/expect/index.css'
+    );
+    expect(Object.keys(loadedConfig.config.plugins).length).toEqual(2);
+    expect(loadedConfig.filepath).toEqual(
+      path.resolve(testDirectory, 'js/object', 'postcss.config.js')
+    );
   });
 
   it('should load "postcss.config.js" with "Array" syntax of plugins', async () => {
-    const expected = (config) => {
-      expect(config.map).toEqual(false);
-      expect(config.from).toEqual(
-        './test/fixtures/config-autoload/js/object/index.css'
-      );
-      expect(config.to).toEqual(
-        './test/fixtures/config-autoload/js/object/expect/index.css'
-      );
-      expect(Object.keys(config.plugins).length).toEqual(2);
-      expect(config.file).toEqual(
-        path.resolve(testDirectory, 'js/array', 'postcss.config.js')
-      );
-    };
-
-    const config = await loadConfig(
+    const loadedConfig = await loadConfig(
       loaderContext,
       path.resolve(testDirectory, 'js/array')
     );
 
-    expected(config);
+    expect(loadedConfig.config.map).toEqual(false);
+    expect(loadedConfig.config.from).toEqual(
+      './test/fixtures/config-autoload/js/object/index.css'
+    );
+    expect(loadedConfig.config.to).toEqual(
+      './test/fixtures/config-autoload/js/object/expect/index.css'
+    );
+    expect(Object.keys(loadedConfig.config.plugins).length).toEqual(2);
+    expect(loadedConfig.filepath).toEqual(
+      path.resolve(testDirectory, 'js/array', 'postcss.config.js')
+    );
+  });
+
+  it('should load empty ".postcssrc"', async () => {
+    const loadedConfig = await loadConfig(
+      loaderContext,
+      path.resolve(testDirectory, 'empty/.postcssrc')
+    );
+
+    // eslint-disable-next-line no-undefined
+    expect(loadedConfig.config).toEqual(undefined);
+    expect(loadedConfig.filepath).toEqual(
+      path.resolve(testDirectory, 'empty/.postcssrc')
+    );
   });
 
   it('should throw an error on "unresolved" config', async () => {
     try {
       await loadConfig(loaderContext, path.resolve('unresolved'));
     } catch (error) {
-      expect(error.message).toMatch(/^No PostCSS Config found in: (.*)$/);
+      expect(error.message).toMatch(/^No PostCSS config found in: (.*)$/);
     }
   });
 });
diff --git a/test/fixtures/config-autoload/empty/.postcssrc b/test/fixtures/config-autoload/empty/.postcssrc
@@ -0,0 +1 @@
+
diff --git a/test/fixtures/config-autoload/empty/expect/index.css b/test/fixtures/config-autoload/empty/expect/index.css
@@ -0,0 +1,7 @@
+.import {
+  color: red;
+}
+
+.test {
+  color: blue;
+}
diff --git a/test/fixtures/config-autoload/empty/expect/index.sss b/test/fixtures/config-autoload/empty/expect/index.sss
@@ -0,0 +1,7 @@
+.import {
+  color: red
+}
+
+.test {
+  color: blue
+}
diff --git a/test/fixtures/config-autoload/empty/fixtures/imports/section.css b/test/fixtures/config-autoload/empty/fixtures/imports/section.css
@@ -0,0 +1,3 @@
+.import {
+  color: red;
+}
diff --git a/test/fixtures/config-autoload/empty/fixtures/imports/section.sss b/test/fixtures/config-autoload/empty/fixtures/imports/section.sss
@@ -0,0 +1,2 @@
+.import
+  color: red
diff --git a/test/fixtures/config-autoload/empty/fixtures/index.css b/test/fixtures/config-autoload/empty/fixtures/index.css
@@ -0,0 +1,5 @@
+@import "imports/section.css";
+
+.test {
+  color: blue;
+}
diff --git a/test/fixtures/config-autoload/empty/fixtures/index.sss b/test/fixtures/config-autoload/empty/fixtures/index.sss
@@ -0,0 +1,4 @@
+@import "imports/section.sss"
+
+.test
+  color: blue