minor #530 Add some checks to the 'pattern' option of Encore.copyFiles() (Lyrkan)

This PR was merged into the master branch.

Discussion
----------

Add some checks to the 'pattern' option of Encore.copyFiles()

While reviewing FriendsOfSymfony/FOSCKEditorBundle#174 I was a bit surprised that calling `copyFiles` with a string as a `pattern` (instead of a `RegExp`) worked fine.

It turns out that's not an issue since the `copyFiles` method causes the creation of a temporary `.js` file in which the `RegExp` is casted to a string anyway.

However, what *is* an issue is that using a string that does not contain a valid regular expression can break that temporary file.

For instance:

```js
Encore.copyFiles({
  from: './assets',
  pattern: 'foo;'
});
```

Will generate something like:

```js
const context_0 = require.context(
    '!/* ... */',
    true,
    foo; // <= Syntax error here
);

context_0.keys().forEach(context_0);
```

This PR doesn't prevent using strings for that option because I don't think that would be the right choice (the unquoted `/regexp/` syntax may not be obvious for JS beginners) but instead officially allow them with some extra checks.

It also casts `includeSubdirectories` (2nd parameter of the `require.context` above) to a boolean when creating the JS file to avoid the same kind of syntax error.

Commits
-------

d07965c Add some checks to the 'pattern' option of Encore.copyFiles()

Author: weaverryan

index.js

@@ -508,8 +508,9 @@ class Encore {
      * Supported options:
      *      * {string} from (mandatory)
      *              The path of the source directory (mandatory)
-     *      * {RegExp} pattern (default: all files)
-     *              A pattern that the filenames must match in order to be copied
+     *      * {RegExp|string} pattern (default: all files)
+     *              A regular expression (or a string containing one) that
+     *              the filenames must match in order to be copied
      *      * {string} to (default: [path][name].[ext])
      *              Where the files must be copied to. You can add all the
      *              placeholders supported by the file-loader.

lib/WebpackConfig.js

@@ -471,6 +471,20 @@ class WebpackConfig {
                 }
             }
 
+            if (typeof config.pattern !== 'undefined' && !(config.pattern instanceof RegExp)) {
+                let validPattern = false;
+                if (typeof config.pattern === 'string') {
+                    const regexPattern = /^\/(.*)\/([a-z]*)?$/;
+                    if (regexPattern.test(config.pattern)) {
+                        validPattern = true;
+                    }
+                }
+
+                if (!validPattern) {
+                    throw new Error(`Invalid pattern "${config.pattern}" passed to copyFiles(). Make sure it contains a valid regular expression.`);
+                }
+            }
+
             this.copyFilesConfigs.push(
                 Object.assign({}, defaultConfig, config)
             );

lib/config-generator.js

@@ -189,7 +189,7 @@ class ConfigGenerator {
                     return buffer + `
                         const context_${index} = require.context(
                             '${stringEscaper(requireContextParam)}',
-                            ${entry.includeSubdirectories},
+                            ${!!entry.includeSubdirectories},
                             ${entry.pattern}
                         );
                         context_${index}.keys().forEach(context_${index});

test/WebpackConfig.js

@@ -423,7 +423,7 @@ describe('WebpackConfig object', () => {
             // With multiple config objects
             config.copyFiles([
                 { from: './foo', pattern: /.*/ },
-                { from: './bar', pattern: /abc/, to: 'bar', includeSubdirectories: false },
+                { from: './bar', pattern: '/abc/', to: 'bar', includeSubdirectories: false },
             ]);
 
             // With a single config object
@@ -437,7 +437,7 @@ describe('WebpackConfig object', () => {
                 context: null,
             }, {
                 from: './bar',
-                pattern: /abc/,
+                pattern: '/abc/',
                 to: 'bar',
                 includeSubdirectories: false,
                 context: null,
@@ -481,6 +481,18 @@ describe('WebpackConfig object', () => {
                 config.copyFiles({ from: 'images', foo: 'foo' });
             }).to.throw('Invalid config option "foo"');
         });
+
+        it('Calling it with an invalid "pattern" option', () => {
+            const config = createConfig();
+
+            expect(() => {
+                config.copyFiles({ from: 'images', pattern: true });
+            }).to.throw('Invalid pattern "true"');
+
+            expect(() => {
+                config.copyFiles({ from: 'images', pattern: 'foo' });
+            }).to.throw('Invalid pattern "foo"');
+        });
     });
 
     describe('autoProvideVariables', () => {