Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Packager]bugfix: require in comment still effect sometimes on Windows. #2812

Closed
wants to merge 4 commits into from
Closed

[Packager]bugfix: require in comment still effect sometimes on Windows. #2812

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
960eb5d
bugfix: require in comment still effect sometimes on Windows. (When t…
tdzl2003 Sep 17, 2015
76869b7
Update lineCommentRe with a better one.
tdzl2003 Sep 17, 2015
a1dd682
Merge branch 'master' of github.com:facebook/react-native into master…
tdzl2003 Sep 18, 2015
e55ca0f
update comment regexp.
tdzl2003 Sep 18, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packager/react-packager/src/DependencyResolver/Module.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,8 @@ class Module {
/**
* Extract all required modules from a `code` string.
*/
const blockCommentRe = /\/\*(.|\n)*?\*\//g;
const lineCommentRe = /\/\/.+(\n|$)/g;
const blockCommentRe = /\/\*(.|\n|\r)*?\*\//g;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a nit: Should this be \r\n (Windows line ending) instead of just \r?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the intent is to match any character, use [\s\S] instead.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets go with @ide suggestion

const lineCommentRe = /\/\/.*(\r|\n|$)/g;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a nit: Should this be \r\n (Windows line ending) instead of just \r?

Can you make this consistent with the change above? Above you used \n|\r and here?

function extractRequires(code /*: string*/) /*: Array<string>*/ {
var deps = {
sync: [],
Expand Down