Fix regex scape #453 #454
Closed
Fix regex scape #453 #454
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In the first index of sharedBlacklist variable the regex writed was without scape it is a path with purpose to exclude react/dist of metro process
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed. If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
Codecov Report
@@ Coverage Diff @@
## master #454 +/- ##
=======================================
Coverage 84.43% 84.43%
=======================================
Files 173 173
Lines 5757 5757
Branches 946 946
=======================================
Hits 4861 4861
Misses 794 794
Partials 102 102
Continue to review full report at Codecov.
|
|
Thank you for your PR! Fixed in #468. |
facebook-github-bot
pushed a commit
that referenced
this pull request
Oct 29, 2019
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
cpojer
pushed a commit
that referenced
this pull request
Jan 8, 2020
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
In the first index of sharedBlacklist variable the regex writed was without scape, it is a path with purpose to exclude react/dist of metro process.
Test plan
Adding the slash scape "\" before slash used in the path
executing
react-native starti got it normally