Skip to content

Commit 356999f

Browse files
fsoncpojer
authored andcommitted
Fix blacklist regex syntax errors (#468)
Summary: **Summary** On Windows with Node.js v12.x.x, Metro crashes with ``` SyntaxError: Invalid regular expression: /(.*\\__fixtures__\\.*|node_modules[\\\]react[\\\]dist[\\\].*|website\\node_modules\\.*|heapCapture\\bundle\.js|.*\\__tests__\\.*)$/: Unterminated character class ``` This has been reported in #453, facebook/react-native#26829, facebook/react-native#26969, facebook/react-native#26878, facebook/react-native#26598, expo/expo-cli#1147 and expo/expo-cli#1074. There are a few open pull requests attempting to fix this same issue: * #464 * #461 * #458 * #454 However, none of the existing PRs address the *root cause* of this error: the `escapeRegExp` function in `blacklist.js` tries to convert regular expressions to be agnostic to the path separator ("/" or "\\"), but turns some valid regular expressions to invalid syntax. The error was is this line: https://github.com/facebook/metro/blob/142348f5345e40ce2075fc7f9dfa30c5d31fee2a/packages/metro-config/src/defaults/blacklist.js#L28 When given a regular expression, such as `/node_modules[/\\]react[/\\]dist[/\\].*/`, on Windows where `path.sep` is `\` (which is also an escape character in regular expressions), this gets turned into `/node_modules[\\\]react[\\\]dist[\\\].*/`, resulting in the `Unterminated character class` error. Automatically replacing `[/]` with `[\]` is an error, as is replacing `[\/]` with `[\\]`, because in both of these cases the backslash before the end of character class "]" escapes it, and the character class becomes unterminated. Therefore, this PR changes the code to look for both escaped forward slash `\/` and forward slash `/`, and always replace them with the escaped version (`\/` or `\\`, depending on the platform). This fixes #453. **Test plan** Added a test case that exercises the code with both `\` and `/` as path separators. Pull Request resolved: #468 Differential Revision: D18201730 Pulled By: cpojer fbshipit-source-id: 6bb694178314c39d4d6a0fd9f8547bfa2c36f894
1 parent 498194f commit 356999f

File tree

3 files changed

+41
-6
lines changed

3 files changed

+41
-6
lines changed

packages/metro-config/src/__tests__/__snapshots__/loadConfig-test.js.snap

+2-2
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ Object {
3939
"ttf",
4040
"zip",
4141
],
42-
"blacklistRE": /\\(node_modules\\[\\\\/\\\\\\\\\\]react\\[\\\\/\\\\\\\\\\]dist\\[\\\\/\\\\\\\\\\]\\.\\*\\|website\\\\/node_modules\\\\/\\.\\*\\|heapCapture\\\\/bundle\\\\\\.js\\|\\.\\*\\\\/__tests__\\\\/\\.\\*\\)\\$/,
42+
"blacklistRE": /\\(node_modules\\\\/react\\\\/dist\\\\/\\.\\*\\|website\\\\/node_modules\\\\/\\.\\*\\|heapCapture\\\\/bundle\\\\\\.js\\|\\.\\*\\\\/__tests__\\\\/\\.\\*\\)\\$/,
4343
"extraNodeModules": Object {},
4444
"hasteImplModulePath": undefined,
4545
"platforms": Array [
@@ -171,7 +171,7 @@ Object {
171171
"ttf",
172172
"zip",
173173
],
174-
"blacklistRE": /\\(node_modules\\[\\\\/\\\\\\\\\\]react\\[\\\\/\\\\\\\\\\]dist\\[\\\\/\\\\\\\\\\]\\.\\*\\|website\\\\/node_modules\\\\/\\.\\*\\|heapCapture\\\\/bundle\\\\\\.js\\|\\.\\*\\\\/__tests__\\\\/\\.\\*\\)\\$/,
174+
"blacklistRE": /\\(node_modules\\\\/react\\\\/dist\\\\/\\.\\*\\|website\\\\/node_modules\\\\/\\.\\*\\|heapCapture\\\\/bundle\\\\\\.js\\|\\.\\*\\\\/__tests__\\\\/\\.\\*\\)\\$/,
175175
"extraNodeModules": Object {},
176176
"hasteImplModulePath": undefined,
177177
"platforms": Array [
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/**
2+
* Copyright (c) Facebook, Inc. and its affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*
7+
* @emails oncall+metro_bundler
8+
* @flow
9+
* @format
10+
*/
11+
12+
'use strict';
13+
14+
const path = require('path');
15+
const blacklist = require('../blacklist');
16+
17+
describe('blacklist', () => {
18+
let originalSeparator;
19+
beforeEach(() => {
20+
originalSeparator = path.sep;
21+
});
22+
23+
afterEach(() => {
24+
// $FlowFixMe: property sep is not writable.
25+
path.sep = originalSeparator;
26+
});
27+
28+
it('converts forward slashes in the RegExp to the OS specific path separator', () => {
29+
// $FlowFixMe: property sep is not writable.
30+
path.sep = '/';
31+
expect('a/b/c').toMatch(blacklist([new RegExp('a/b/c')]));
32+
33+
// $FlowFixMe: property sep is not writable.
34+
path.sep = '\\';
35+
expect(require('path').sep).toBe('\\');
36+
expect('a\\b\\c').toMatch(blacklist([new RegExp('a/b/c')]));
37+
});
38+
});

packages/metro-config/src/defaults/blacklist.js

+1-4
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,9 @@ var path = require('path');
1313
// Don't forget to everything listed here to `package.json`
1414
// modulePathIgnorePatterns.
1515
var sharedBlacklist = [
16-
/node_modules[/\\]react[/\\]dist[/\\].*/,
17-
16+
/node_modules\/react\/dist\/.*/,
1817
/website\/node_modules\/.*/,
19-
2018
/heapCapture\/bundle\.js/,
21-
2219
/.*\/__tests__\/.*/,
2320
];
2421

0 commit comments

Comments
 (0)