Skip to content

Conversation

@ecraig12345
Copy link
Member

@ecraig12345 ecraig12345 commented Nov 30, 2018

Pull request checklist

  • Addresses an existing issue: Fixes Support Node 10 #7184
  • Include a change request file using $ npm run change

Description of changes

Node 10 is now LTS, so we should support it in Fabric. We will also continue to support Node 8 for now.

Scripts to test

Checked items work; unchecked items are broken or untested.

  • build (also includes basic tests)
  • start
  • start-exp
  • code-style
  • bundlesize
  • create-component
  • create-package
  • update-api
  • update-snapshots
  • check-for-changed-files
  • prettier
  • generate-version-files
  • codepen
  • precommit
  • (apps/vr-tests) screener
  • (apps/vr-tests) screener:local
  • (apps/vr-tests) start
Microsoft Reviewers: Open in CodeFlow

@ecraig12345 ecraig12345 requested a review from dzearing November 30, 2018 02:35
* for your repo.
*/
"nodeSupportedVersionRange": ">=8.0.0 <10.0.0",
"nodeSupportedVersionRange": ">=8.0.0 <9.0.0 || >=10.0.0 <11.0.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

For node 10, maybe restrict it to more recent versions...

Copy link
Member

Choose a reason for hiding this comment

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

what do you mean?

Copy link
Member

Choose a reason for hiding this comment

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

or... do you know of problems with some versions?

Copy link
Member Author

Choose a reason for hiding this comment

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

What do you think a good start version would be? I'm not sure how to determine that.

Copy link
Contributor

Choose a reason for hiding this comment

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

A good version to start is the version that Node 10 first went LTS on. But I otherwise think 10.9.0 is a good starting place.

@dzearing I don't know of a specific problem, but @pgonzal really did not like to be on Node 10 initially because his perception (I believe) is that the very early versions of Node 10 are quite unstable ... and hence we generally stay at LTS.

Copy link
Member

Choose a reason for hiding this comment

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

gotcha; maybe just start at the current LTS then? (10.14.1) as we're just starting to test now.

There were a few security updates recommending minbar 10.14.0, and the .1 patch was specifcally for windows msi issues.

https://nodejs.org/en/blog/vulnerability/november-2018-security-releases/

Choose a reason for hiding this comment

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

What do you think a good start version would be? I'm not sure how to determine that.

(1) an LTS version, and (2) something you've actually tested yourself.

It is super frustrating to waste an hour investigating somebody's issue, only to find out that it was just a buggy build of NodeJS. We got burned by this repeatedly until we finally started insisting on LTS versions.

Copy link
Member Author

Choose a reason for hiding this comment

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

Looked up when 8 and 10 became LTS, and it was at 8.9.0 and 10.9.0. (Didn't know until today that each even-numbered major version is out for about 6 months before becoming officially LTS.)

@pgonzal, I'm curious what types of problems you've run into due to buggy Node builds? I don't remember encountering any issues like that myself, though I'm sure it's possible.

Copy link
Member

Choose a reason for hiding this comment

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

@ecraig12345 ecraig12345 merged commit 087b0b6 into microsoft:master Nov 30, 2018
@ecraig12345 ecraig12345 deleted the node10 branch November 30, 2018 03:32
@microsoft microsoft locked as resolved and limited conversation to collaborators Aug 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support Node 10

5 participants