fsevents doesn't compile under Node 12

[jnraine]

While bootstrapping, I ran into an error with fsevents, an optional node module.

$ script/bootstrap
# ...snip...
gyp ERR! build error 
gyp ERR! stack Error: `make` failed with exit code: 2
gyp ERR! stack     at ChildProcess.onExit (/usr/local/lib/node_modules/npm/node_modules/node-gyp/lib/build.js:262:23)
gyp ERR! stack     at ChildProcess.emit (events.js:203:13)
gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:272:12)
gyp ERR! System Darwin 18.7.0
gyp ERR! command "/usr/local/Cellar/node/12.6.0/bin/node" "/usr/local/lib/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js" "build" "-j" "8" "native_metrics"
gyp ERR! cwd /Users/jordan/projects/slack/node_modules/@newrelic/native-metrics
gyp ERR! node -v v12.6.0
gyp ERR! node-gyp -v v3.8.0
gyp ERR! not ok 

There's lots more output. Try the repro steps to get it all.

🥼Reproduction Steps

brew install node # or, if you already have node: brew reinstall node 
node -v # v12.6.0 <--- node must be 12.x
script/bootstrap

You can "fix" the problem by deleting package-lock.json running script/bootstrap again. Among other things, this bumps fsevents from 1.2.4/1.2.7 to 1.2.9.

💥 Why did it break?

fsevents didn't work with Node 12 until v1.2.9. A

Because it is an optional dependency, this isn't the end of the world. The tests still pass. However, since script/bootstrap installs node 12 (as of a few months ago), this noise will hit anyone new to the project.

❓Questions

• Any objection to updating fsevents to 1.2.9?
• What do folks think about adding a .node-version file to indicate expected node version?

[welcome]

Thanks for opening this issue! If you would like to help implement an improvement, read more about contributing and consider submitting a pull request.

[dennissivia]

Hey @jnraine 👋 ,
thanks for reporting this.
Currently, we are only focussing on the nodeJS version that the app is running on in production.
See engines in package.json:

slack/package.json

Lines 56 to 59 in e2ba204

	"engines": {
	"node": "8.x.x",
	"npm": "6.x.x"
	},

So node 8 should be the version you are using when contributing to this app to make sure everything works nicely in staging and production.

Answers

Regarding your questions:

1. I would do that only if it is necessary for the current node version (8)
2. Adding a .node-version would be helpful to make things easier for folks that have a node version manager.

Node updates

We have an issue for updating the node version: #837.
In addition, updating to version 12, seems too early, because we want to stay in the active/maintenance lifecycle to ensure stability.
At this point in time (until the end of 2019) NodeJS 12 is still current.

[dennissivia]

Hey @jnraine,
Since this was a bug remote/update request and we need some more things to do before we start updating dependencies, I am closing this for now.
We only support Node 10 ATM and any issue with dependencies not working on Node 10, deserve a bug report.

Feel free to add a feature request for adding a .node-version or nvm config, if that would improve the developer experience.