Skip to content
This repository has been archived by the owner on Jan 18, 2024. It is now read-only.

Print warning when running on newer version of Node.js #1992

Merged
merged 2 commits into from
Apr 28, 2020

Conversation

LinusU
Copy link
Contributor

@LinusU LinusU commented Apr 25, 2020

Right now, both Node.js 13 and 14 is "Current release"

Screenshot 2020-04-25 at 13 25 13

@brentvatne
Copy link
Member

we haven't tried using it with node 14 yet, perhaps we should add a new class of warning that this is a new release and there may be issues, but let people proceed with using expo-cli with it. can you update the pr to do that?

@LinusU LinusU changed the title Add support for Node.js 14.x Print warning when running on newer version of Node.js Apr 26, 2020
@LinusU
Copy link
Contributor Author

LinusU commented Apr 26, 2020

@brentvatne does this look good?

btw, chalk doesn't even import on Node.js <8, so effectively the error is only printed for 8.x, 11.x. Maybe that isn't a problem though? I'm not really sure what the motivation behind the warning is ☺️

@brentvatne
Copy link
Member

@LinusU - i didn't know that :O

the purpose of this is that we don't want to have people creating issues about problems they encounter with unsupported versions of expo-cli, and we found that if we don't block it from starting people will just ignore the warnings and try anyways. we can be pretty sure that on node < 10 people are going to have problems, so we just save them the trouble of encountering an obscure error and stop them from starting it at all.

thanks for the pr!

@LinusU
Copy link
Contributor Author

LinusU commented Apr 28, 2020

the purpose of this is that we don't want to have people creating issues about problems they encounter with unsupported versions of expo-cli, and we found that if we don't block it from starting people will just ignore the warnings and try anyways.

Thanks for the explanation, that makes sense!

i didn't know that :O

semver also only supports Node.js 10 and up, so we probably need to do this without dependencies if you want it to work on any old version. We are doing something similar in StandardJS here. I'd be happy to submit a PR if you want to...

@brentvatne
Copy link
Member

brentvatne commented Apr 28, 2020

that would be great, thank you @LinusU! i'll land this for now and look forward to the follow up pr

@LinusU
Copy link
Contributor Author

LinusU commented Apr 28, 2020

Thanks for merging! Follow up PR is here: #2030

@LinusU LinusU deleted the patch-1 branch April 28, 2020 20:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants