-
-
Notifications
You must be signed in to change notification settings - Fork 27k
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.
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
Remove code duplication #1241
Remove code duplication #1241
Conversation
packageName, | ||
'package.json' | ||
); | ||
checkNodeVersion(packageJsonPath); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not even sure if this line is still useful.
Looks like unreachable code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This lets us increase minimal version with time if react-scripts
imposes this but user doesn't update the CLI itself.
} | ||
} | ||
|
||
checkNodeVersion(path.resolve(__dirname, 'package.json')); | ||
|
||
var currentNodeVersion = process.versions.node | ||
if (currentNodeVersion.split('.')[0] < 4) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this check then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This allows us to check the minimal supported version asap for the whole cra to work.
The problem was that it took ~10 minutes with node v0.10 to get to an halt with error, using a lot of resources meanwhile as it still installs all the dependencies, even thou it warns about the problem.
0018096
to
125edbd
Compare
125edbd
to
c140012
Compare
This doesn't merge cleanly and I forgot what this is about. If it's still relevant please resubmit :-) |
I don't think this is still relevant 🤷♂️ |
I just realized that with #1195 I duplicated some code. This PR just removes it.