Repository: webpack-contrib/copy-webpack-plugin
Pull request #476 (2020-05-15 12:28:03)

Refactor code
<!--
  HOLY CRAP a Pull Request. We ❤️ those!

  If you remove or skip this template, you'll make the 🐼 sad and the mighty god
  of Github will appear and pile-drive the close button from a great height
  while making animal noises.

  Please place an x (no spaces!) in all [ ] that apply
-->

This PR contains a:

- [ ] **bugfix**
- [ ] new **feature**
- [x] **code refactor**
- [x] **test update** <!-- if bug or feature is checked, this should be too -->
- [ ] **typo fix**
- [ ] **metadata update**

### Motivation / Use-Case

refactor code base

### Breaking Changes

Yes, missing file is not an error

### Additional Info

No

codecov[bot]: # [Codecov](https://codecov.io/gh/webpack-contrib/copy-webpack-plugin/pull/476?src=pr&el=h1) Report
> Merging [#476](https://codecov.io/gh/webpack-contrib/copy-webpack-plugin/pull/476?src=pr&el=desc) into [master](https://codecov.io/gh/webpack-contrib/copy-webpack-plugin/commit/e3803ceffe93361184efc9b799be4c9dfb4eb467&el=desc) will **increase** coverage by `0.26%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/webpack-contrib/copy-webpack-plugin/pull/476/graphs/tree.svg?width=650&height=150&src=pr&token=fEoU9iBFGL)](https://codecov.io/gh/webpack-contrib/copy-webpack-plugin/pull/476?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #476      +/-   ##
==========================================
+ Coverage   94.90%   95.16%   +0.26%     
==========================================
  Files           8        8              
  Lines         216      207       -9     
  Branches       58       57       -1     
==========================================
- Hits          205      197       -8     
+ Misses         11       10       -1     
```


| [Impacted Files](https://codecov.io/gh/webpack-contrib/copy-webpack-plugin/pull/476?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/index.js](https://codecov.io/gh/webpack-contrib/copy-webpack-plugin/pull/476/diff?src=pr&el=tree#diff-c3JjL2luZGV4Lmpz) | `100.00% <100.00%> (ø)` | |
| [src/postProcessPattern.js](https://codecov.io/gh/webpack-contrib/copy-webpack-plugin/pull/476/diff?src=pr&el=tree#diff-c3JjL3Bvc3RQcm9jZXNzUGF0dGVybi5qcw==) | `85.24% <100.00%> (-0.70%)` | :arrow_down: |
| [src/preProcessPattern.js](https://codecov.io/gh/webpack-contrib/copy-webpack-plugin/pull/476/diff?src=pr&el=tree#diff-c3JjL3ByZVByb2Nlc3NQYXR0ZXJuLmpz) | `100.00% <100.00%> (ø)` | |
| [src/processPattern.js](https://codecov.io/gh/webpack-contrib/copy-webpack-plugin/pull/476/diff?src=pr&el=tree#diff-c3JjL3Byb2Nlc3NQYXR0ZXJuLmpz) | `100.00% <100.00%> (+3.22%)` | :arrow_up: |
| [src/utils/createPatternGlob.js](https://codecov.io/gh/webpack-contrib/copy-webpack-plugin/pull/476/diff?src=pr&el=tree#diff-c3JjL3V0aWxzL2NyZWF0ZVBhdHRlcm5HbG9iLmpz) | `100.00% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/webpack-contrib/copy-webpack-plugin/pull/476?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/webpack-contrib/copy-webpack-plugin/pull/476?src=pr&el=footer). Last update [e3803ce...664093f](https://codecov.io/gh/webpack-contrib/copy-webpack-plugin/pull/476?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/src/index.js b/src/index.js
index c13c577..219050d 100644
--- a/src/index.js
+++ b/src/index.js
@@ -27,7 +27,7 @@ class CopyPlugin {
       compilation.hooks.additionalAssets.tapAsync(
         'copy-webpack-plugin',
         async (callback) => {
-          logger.debug('start to adding additionalAssets');
+          logger.debug('start to adding additional assets');
 
           const globalRef = {
             context: compiler.options.context,
@@ -70,7 +70,7 @@ class CopyPlugin {
               )
             );
 
-            logger.debug('end to adding additionalAssets');
+            logger.debug('end to adding additional assets');
 
             callback();
           } catch (error) {
diff --git a/src/postProcessPattern.js b/src/postProcessPattern.js
index 265f0b2..6f33e6a 100644
--- a/src/postProcessPattern.js
+++ b/src/postProcessPattern.js
@@ -43,6 +43,7 @@ export default async function postProcessPattern(globalRef, pattern, file) {
   // If this came from a glob, add it to the file watchlist
   if (pattern.fromType === 'glob') {
     logger.debug(`add ${file.absoluteFrom} as fileDependencies`);
+
     compilation.fileDependencies.add(file.absoluteFrom);
   }
 
@@ -54,16 +55,13 @@ export default async function postProcessPattern(globalRef, pattern, file) {
     content = await readFile(inputFileSystem, file.absoluteFrom);
   } catch (error) {
     compilation.errors.push(error);
+
     return;
   }
 
   if (pattern.transform) {
     logger.log(`transforming content for '${file.absoluteFrom}'`);
 
-    // eslint-disable-next-line no-shadow
-    const transform = (content, absoluteFrom) =>
-      pattern.transform(content, absoluteFrom);
-
     if (pattern.cacheTransform) {
       if (!globalRef.cacheDir) {
         globalRef.cacheDir =
@@ -87,8 +85,8 @@ export default async function postProcessPattern(globalRef, pattern, file) {
         );
 
         content = result.data;
-      } catch (e) {
-        content = await transform(content, file.absoluteFrom);
+      } catch (_ignoreError) {
+        content = await pattern.transform(content, file.absoluteFrom);
 
         logger.debug(`caching transformation for '${file.absoluteFrom}'`);
 
@@ -97,7 +95,7 @@ export default async function postProcessPattern(globalRef, pattern, file) {
           .then(() => content);
       }
     } else {
-      content = await transform(content, file.absoluteFrom);
+      content = await pattern.transform(content, file.absoluteFrom);
     }
   }
 
@@ -109,7 +107,7 @@ export default async function postProcessPattern(globalRef, pattern, file) {
     // If it doesn't have an extension, remove it from the pattern
     // ie. [name].[ext] or [name][ext] both become [name]
     if (!path.extname(file.relativeFrom)) {
-      file.webpackTo = file.webpackTo.replace(/\.?\[ext\]/g, '');
+      file.webpackTo = file.webpackTo.replace(/\.?\[ext]/g, '');
     }
 
     file.webpackTo = loaderUtils.interpolateName(
@@ -137,13 +135,10 @@ export default async function postProcessPattern(globalRef, pattern, file) {
   }
 
   const targetPath = normalizePath(file.webpackTo);
-
   const source = new RawSource(content);
 
-  const hasAssetsAPI = typeof compilation.emitAsset === 'function';
-
   // For old version webpack 4
-  if (!hasAssetsAPI) {
+  if (typeof compilation.emitAsset !== 'function') {
     compilation.assets[targetPath] = source;
 
     return;
@@ -156,10 +151,12 @@ export default async function postProcessPattern(globalRef, pattern, file) {
       );
 
       compilation.updateAsset(targetPath, source);
+
       return;
     }
 
     logger.log(`skipping '${file.webpackTo}', because it already exists`);
+
     return;
   }
 
diff --git a/src/preProcessPattern.js b/src/preProcessPattern.js
index 126b172..9bba1b8 100644
--- a/src/preProcessPattern.js
+++ b/src/preProcessPattern.js
@@ -8,31 +8,25 @@ import { stat } from './utils/promisify';
 export default async function preProcessPattern(globalRef, pattern) {
   const { context, logger, inputFileSystem } = globalRef;
 
-  pattern =
-    typeof pattern === 'string'
-      ? { from: pattern }
-      : Object.assign({}, pattern);
-
-  pattern.to = pattern.to || '';
-  pattern.context = pattern.context || context;
-
-  if (!path.isAbsolute(pattern.context)) {
-    pattern.context = path.join(context, pattern.context);
-  }
-
-  pattern.noErrorOnMissing = pattern.noErrorOnMissing || false;
-
-  // Todo remove this in next major
-  const isToDirectory =
-    path.extname(pattern.to) === '' || pattern.to.slice(-1) === path.sep;
-
+  pattern = typeof pattern === 'string' ? { from: pattern } : { ...pattern };
   pattern.fromOrigin = pattern.from;
   pattern.from = path.normalize(pattern.from);
-  pattern.context = path.normalize(pattern.context);
-  pattern.to = path.normalize(pattern.to);
+  pattern.to = path.normalize(
+    typeof pattern.to !== 'undefined' ? pattern.to : ''
+  );
+  pattern.context = path.normalize(
+    typeof pattern.context !== 'undefined'
+      ? !path.isAbsolute(pattern.context)
+        ? path.join(context, pattern.context)
+        : pattern.context
+      : context
+  );
 
   logger.debug(`processing from: '${pattern.from}' to: '${pattern.to}'`);
 
+  const isToDirectory =
+    path.extname(pattern.to) === '' || pattern.to.slice(-1) === path.sep;
+
   switch (true) {
     // if toType already exists
     case !!pattern.toType:
@@ -67,10 +61,7 @@ export default async function preProcessPattern(globalRef, pattern) {
 
   if (stats.isDirectory()) {
     pattern.fromType = 'dir';
-    return pattern;
-  }
-
-  if (stats.isFile()) {
+  } else if (stats.isFile()) {
     pattern.fromType = 'file';
     pattern.stats = stats;
   }
diff --git a/src/processPattern.js b/src/processPattern.js
index 3f4faa5..83f571c 100644
--- a/src/processPattern.js
+++ b/src/processPattern.js
@@ -4,8 +4,6 @@ import globby from 'globby';
 
 import createPatternGlob from './utils/createPatternGlob';
 
-/* eslint-disable no-param-reassign */
-
 export default async function processPattern(globalRef, pattern) {
   const { logger, output, compilation } = globalRef;
 
@@ -22,20 +20,13 @@ export default async function processPattern(globalRef, pattern) {
       return Promise.resolve();
     }
 
-    const newWarning = new Error(
+    const missingError = new Error(
       `unable to locate '${pattern.from}' at '${pattern.absoluteFrom}'`
     );
-    const hasWarning = compilation.warnings.some(
-      // eslint-disable-next-line no-shadow
-      (warning) => warning.message === newWarning.message
-    );
 
-    // Only display the same message once
-    if (!hasWarning) {
-      logger.warn(newWarning.message);
+    logger.error(missingError.message);
 
-      compilation.warnings.push(newWarning);
-    }
+    compilation.errors.push(missingError);
 
     return Promise.resolve();
   }
diff --git a/src/utils/createPatternGlob.js b/src/utils/createPatternGlob.js
index ca8a9fd..ead002e 100644
--- a/src/utils/createPatternGlob.js
+++ b/src/utils/createPatternGlob.js
@@ -62,11 +62,9 @@ function createPatternGlob(pattern, globalRef) {
       /* eslint-enable no-param-reassign */
 
       break;
-
-    default:
+    default: {
       logger.debug(`determined '${pattern.absoluteFrom}' is a glob`);
 
-      // eslint-disable-next-line no-case-declarations
       const contextDependencies = path.normalize(
         globParent(pattern.absoluteFrom)
       );
@@ -83,7 +81,8 @@ function createPatternGlob(pattern, globalRef) {
             getAbsoluteContext(pattern.context),
             pattern.fromOrigin
           );
-    /* eslint-enable no-param-reassign */
+      /* eslint-enable no-param-reassign */
+    }
   }
 
   return pattern;


Test patch:
diff --git a/test/CopyPlugin.test.js b/test/CopyPlugin.test.js
index bb6eded..107ae61 100644
--- a/test/CopyPlugin.test.js
+++ b/test/CopyPlugin.test.js
@@ -6,7 +6,7 @@ import { readAssets } from './helpers';
 
 const FIXTURES_DIR = path.join(__dirname, 'fixtures');
 
-describe('apply function', () => {
+describe('CopyPlugin', () => {
   describe('basic', () => {
     it('should copy a file', (done) => {
       runEmit({
@@ -221,9 +221,7 @@ describe('apply function', () => {
         .then(done)
         .catch(done);
     });
-  });
 
-  describe('difference path segment separation', () => {
     it('should work with linux path segment separation path when "from" is glob', (done) => {
       runEmit({
         expectedAssetKeys: ['directory/nested/nestedfile.txt'],
@@ -237,34 +235,6 @@ describe('apply function', () => {
         .catch(done);
     });
 
-    // Windows path segment (\\) can not use as path segment in glob, but can use as dirname at linux
-    it.skip('should work with windows path segment separation path when "from" is glob', (done) => {
-      runEmit({
-        expectedAssetKeys: ['directory/nested/nestedfile.txt'],
-        patterns: [
-          {
-            from: 'directory\\nested\\*',
-          },
-        ],
-      })
-        .then(done)
-        .catch(done);
-    });
-
-    // Windows path segment (\\) can not use as path segment in glob, but can use as dirname at linux
-    it.skip('should work with mixed path segment separation path when "from" is glob', (done) => {
-      runEmit({
-        expectedAssetKeys: ['directory/nested/nestedfile.txt'],
-        patterns: [
-          {
-            from: 'directory/nested\\*',
-          },
-        ],
-      })
-        .then(done)
-        .catch(done);
-    });
-
     it('should exclude path with linux path segment separators', (done) => {
       runEmit({
         expectedAssetKeys: [
@@ -285,28 +255,6 @@ describe('apply function', () => {
         .then(done)
         .catch(done);
     });
-
-    // Windows path segment (\\) can not use as path segment in glob, but can use as dirname at linux
-    it.skip('should exclude path with windows path segment separators', (done) => {
-      runEmit({
-        expectedAssetKeys: [
-          '[!]/hello.txt',
-          '[special?directory]/(special-*file).txt',
-          '[special?directory]/directoryfile.txt',
-          '[special?directory]/nested/nestedfile.txt',
-          'dir (86)/file.txt',
-          'dir (86)/nesteddir/deepnesteddir/deepnesteddir.txt',
-          'dir (86)/nesteddir/nestedfile.txt',
-        ],
-        patterns: [
-          {
-            from: '!(directory)\\**\\*.txt',
-          },
-        ],
-      })
-        .then(done)
-        .catch(done);
-    });
   });
 
   describe('watch mode', () => {
@@ -538,33 +486,33 @@ describe('apply function', () => {
         .catch(done);
     });
   });
-});
 
-describe('logging', () => {
-  it('should logging', (done) => {
-    const expectedAssetKeys = ['file.txt'];
-
-    run({
-      patterns: [
-        {
-          from: 'file.txt',
-        },
-      ],
-    })
-      .then(({ compiler, stats }) => {
-        const root = path.resolve(__dirname).replace(/\\/g, '/');
-        const logs = stats.compilation.logging
-          .get('copy-webpack-plugin')
-          .map((entry) =>
-            entry.args[0].replace(/\\/g, '/').split(root).join('.')
-          );
+  describe('logging', () => {
+    it('should logging', (done) => {
+      const expectedAssetKeys = ['file.txt'];
 
-        expect(
-          Array.from(Object.keys(readAssets(compiler, stats))).sort()
-        ).toEqual(expectedAssetKeys);
-        expect({ result: logs }).toMatchSnapshot({ result: logs });
+      run({
+        patterns: [
+          {
+            from: 'file.txt',
+          },
+        ],
       })
-      .then(done)
-      .catch(done);
+        .then(({ compiler, stats }) => {
+          const root = path.resolve(__dirname).replace(/\\/g, '/');
+          const logs = stats.compilation.logging
+            .get('copy-webpack-plugin')
+            .map((entry) =>
+              entry.args[0].replace(/\\/g, '/').split(root).join('.')
+            );
+
+          expect(
+            Array.from(Object.keys(readAssets(compiler, stats))).sort()
+          ).toEqual(expectedAssetKeys);
+          expect({ result: logs }).toMatchSnapshot({ result: logs });
+        })
+        .then(done)
+        .catch(done);
+    });
   });
 });
diff --git a/test/__snapshots__/CopyPlugin.test.js.snap b/test/__snapshots__/CopyPlugin.test.js.snap
index c9a8de6..5092e9b 100644
--- a/test/__snapshots__/CopyPlugin.test.js.snap
+++ b/test/__snapshots__/CopyPlugin.test.js.snap
@@ -1,9 +1,9 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`logging should logging 1`] = `
+exports[`CopyPlugin logging should logging 1`] = `
 Object {
   "result": Array [
-    "start to adding additionalAssets",
+    "start to adding additional assets",
     "processing from: 'file.txt' to: '.'",
     "getting stats for './fixtures/file.txt' to determinate 'fromType'",
     "determined './fixtures/file.txt' is a file",
@@ -14,7 +14,7 @@ Object {
     "getting stats for './fixtures/file.txt' to write to assets",
     "reading './fixtures/file.txt' to write to assets",
     "writing 'file.txt' to compilation assets from './fixtures/file.txt'",
-    "end to adding additionalAssets",
+    "end to adding additional assets",
   ],
 }
 `;
diff --git a/test/from-option.test.js b/test/from-option.test.js
index 46740f6..4faedd8 100644
--- a/test/from-option.test.js
+++ b/test/from-option.test.js
@@ -67,7 +67,7 @@ describe('from option', () => {
     it('should move a file (symbolic link)', (done) => {
       runEmit({
         symlink: true,
-        expectedWarnings:
+        expectedErrors:
           process.platform === 'win32'
             ? [
                 new Error(
@@ -86,10 +86,10 @@ describe('from option', () => {
         .catch(done);
     });
 
-    it('should warn when file not found', (done) => {
+    it('should throw an error on the missing file', (done) => {
       runEmit({
         expectedAssetKeys: [],
-        expectedWarnings: [
+        expectedErrors: [
           new Error(
             `unable to locate 'nonexistent.txt' at '${FIXTURES_DIR}${path.sep}nonexistent.txt'`
           ),
@@ -206,7 +206,7 @@ describe('from option', () => {
       runEmit({
         // Windows doesn't support symbolic link
         symlink: true,
-        expectedWarnings:
+        expectedErrors:
           process.platform === 'win32'
             ? [
                 new Error(
@@ -290,10 +290,10 @@ describe('from option', () => {
         .catch(done);
     });
 
-    it('should warn when directory not found', (done) => {
+    it('should throw an error on the missing directory', (done) => {
       runEmit({
         expectedAssetKeys: [],
-        expectedWarnings: [
+        expectedErrors: [
           new Error(
             `unable to locate 'nonexistent' at '${FIXTURES_DIR}${path.sep}nonexistent'`
           ),
@@ -471,7 +471,7 @@ describe('from option', () => {
       runEmit({
         // Windows doesn't support symbolic link
         symlink: true,
-        expectedWarnings:
+        expectedErrors:
           process.platform === 'win32'
             ? [
                 new Error(
@@ -499,5 +499,23 @@ describe('from option', () => {
         .then(done)
         .catch(done);
     });
+
+    it('should throw an error on the missing glob', (done) => {
+      runEmit({
+        expectedAssetKeys: [],
+        expectedErrors: [
+          new Error(
+            `unable to locate 'nonexistent${path.sep}**${path.sep}*' at '${FIXTURES_DIR}${path.sep}nonexistent${path.sep}**${path.sep}*'`
+          ),
+        ],
+        patterns: [
+          {
+            from: 'nonexistent/**/*',
+          },
+        ],
+      })
+        .then(done)
+        .catch(done);
+    });
   });
 });
diff --git a/test/globOptions-option.test.js b/test/globOptions-option.test.js
index 0f61196..4338ad5 100644
--- a/test/globOptions-option.test.js
+++ b/test/globOptions-option.test.js
@@ -41,7 +41,7 @@ describe('from option', () => {
 describe('globOptions ignore option', () => {
   it('should ignore files when "from" is a file', (done) => {
     runEmit({
-      expectedWarnings: [
+      expectedErrors: [
         new Error(
           `unable to locate 'file.txt' at '${FIXTURES_DIR}${path.sep}file.txt'`
         ),
@@ -218,7 +218,7 @@ describe('globOptions ignore option', () => {
 
   it('should ignores all files even if they start with a dot', (done) => {
     runEmit({
-      expectedWarnings: [
+      expectedErrors: [
         new Error(
           `unable to locate 'directory' at '${FIXTURES_DIR}${path.sep}directory${path.sep}**${path.sep}*'`
         ),
@@ -238,7 +238,7 @@ describe('globOptions ignore option', () => {
 
   it('should ignore files when "from" is a file (global ignore)', (done) => {
     runEmit({
-      expectedWarnings: [
+      expectedErrors: [
         new Error(
           `unable to locate 'file.txt' at '${FIXTURES_DIR}${path.sep}file.txt'`
         ),
diff --git a/test/toType-option.test.js b/test/toType-option.test.js
index c939de8..7940b92 100644
--- a/test/toType-option.test.js
+++ b/test/toType-option.test.js
@@ -86,7 +86,7 @@ describe('toType option', () => {
   it('should warn when file not found and stats is undefined', (done) => {
     runEmit({
       expectedAssetKeys: [],
-      expectedWarnings: [
+      expectedErrors: [
         new Error(
           `unable to locate 'nonexistent.txt' at '${FIXTURES_DIR}${path.sep}nonexistent.txt'`
         ),