-
Notifications
You must be signed in to change notification settings - Fork 142
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
Adding Support for Node 10 #525
Conversation
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.
Changeset seems sensible. Let's do some manual testing!
No need to wait. Node Sass already supports node 12. |
@@ -7,6 +7,8 @@ addons: | |||
- github.com | |||
node_js: | |||
- 8 | |||
- 10 |
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.
Should we add 9 as well ?
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 don't think so, odd versions are only supported for a short time and I don't think we need to support node versions that aren't supported for cli. In the future I can see doing the leading version like 13 for early adopters and it helps us make sure we are in a good position for the next even/lts release.
Thoughts?
@@ -4,7 +4,7 @@ | |||
"description": "CLI tool to run BigCommerce Stores locally for theme development.", | |||
"main": "index.js", | |||
"engines": { | |||
"node": ">= 8.0.0 <9.0.0" | |||
"node": ">= 8.0.0 <11.0.0" |
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.
Also can we also update the readme to reflect node version support.
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.
Good catch!
@@ -6,7 +6,7 @@ | |||
The BigCommerce server emulator for local theme development. | |||
|
|||
## Install | |||
_Note: Stencil requires the Node.js runtime environment, version 8.x. We do not yet have support for Node 10 or greater._ | |||
_Note: Stencil requires the Node.js runtime environment, version 8.x or 10.x We do not yet have support for Node 12 or greater._ | |||
|
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.
@junedkazi @bookernath Let me know if we want different messaging here.
What?
Updating stencil styles dependency which allows node 10 support.
Tested