update npm

[aminya]

Description of the change

This updates npm to 6.14.8.

Related: atom-community/atom#102

Verification

The CI passes. npm versions are Node version agnostic, so it will work on Node version without changing the behavior.

This is tested on Atom too: atom-community/atom#103

Drawbacks

none

Release Notes

• Update npm to 6.14.8

Closes #813

[aminya]

@sadick254 consider this as well. This solved the Linux issues for me.
atom/atom#21229

[DeeDeeG]

Unfortunately, (sorry to say it) this did not completely get rid of the Linux build failures either. We had a few failed builds even after merging the updated apm with this npm bump.

https://dev.azure.com/atomcommunity/atomcommunity/_build/results?buildId=640&view=logs&j=0da5d1d9-276d-5173-c4c4-9d4d4ed14fdb&t=cccdf131-0893-5d7b-ccea-9ccfc8953f95

https://dev.azure.com/atomcommunity/atomcommunity/_build/results?buildId=641&view=logs&j=0da5d1d9-276d-5173-c4c4-9d4d4ed14fdb&t=cccdf131-0893-5d7b-ccea-9ccfc8953f95&s=b15d9194-8f26-5328-b47f-5968c76b37e7

https://dev.azure.com/atomcommunity/atomcommunity/_build/results?buildId=644&view=logs&j=0da5d1d9-276d-5173-c4c4-9d4d4ed14fdb&t=cccdf131-0893-5d7b-ccea-9ccfc8953f95

It is an intermittent failure, so random chance will have it go away for several runs only to come back again.

[sadick254]

@aminya and @DeeDeeG The linux build failure seem to have started after the electron upgrade atom/atom#21079. I have not yet been able to pin point the commit that caused the flakiness.

[aminya]

@sadick254 Here I did some research: atom-community/atom#88 (comment)

[DeeDeeG]

@sadick254 @aminya Another possible solution: During the Electron 6 PR, CI's Node version was set back, starting from Node 12.13.1 and downgrading to Node 12.40. Maybe this older Node has a bug that got fixed in newer Node 12.

I have collected lots of evidence (including many passing CI runs) that show the Node version in CI doesn't really matter very much. It can be Node 10 (10.12.0 or newer) or Node 12 (12.16 or older). I suggest we should try to use the latest Node 12.16 (12.16.3) or the latest Node 10 (something like 10.22.0). If the Linux build failure goes away for a week or more of CI runs, then I'd call it "probably fixed."

I can trial this at my personal fork if folks want more evidence to go off of.

---

Update: Here's the branch, I'll try to remember to push to it every time I notice upstream has updated. So I'll try to run this CI as often as upstream master updates, for a while.

• Branch: https://github.com/DeeDeeG/atom/tree/upstream-with-node-12.16
  • CI runs: https://dev.azure.com/DeeDeeG/b/_build?definitionId=17&_a=summary&branchFilter=79

Bonus: Doing this for the fork as well, just to collect some more CI runs.

• Branch: https://github.com/DeeDeeG/atom/tree/fork-with-node-12.16
  • CI runs: https://dev.azure.com/DeeDeeG/b/_build?definitionId=17&branchFilter=80%2C80

[aminya]

This is a N-api issue. One of the Native modules (or Electron itself) does not use thread-safe promises. Using a Node version other than the one which Electron uses is not an appropriate solution to this (we have discussed this many times atom-community/atom#111).

For example: electron/electron#22843 fixed one missing handle scope in Electron 10, but this is not back ported to 6.

[DeeDeeG]

@aminya We have discussed many times and disagreed many times.

If I present a solution that works, that upstream is interested in, it is ultimately upstream's choice whether to go with it.

More evidence: Before the electron 6 PR, Node in CI did not match Electron's Node version. Only after they were put in sync did Linux builds start intermittently failing. Seriously, I'm not going to change my view on that. I'm not trying to re-hash that with you. I'm presenting an option to the upstream maintainers. They can choose.

---

Also: if upstream uses newer Node and you want to stay on Node 12.4.0 at our fork, then I have no problem with that. We can try to solve this from multiple angles at the same time.

[sadick254]

@aminya and @DeeDeeG Thank you for all your pointers.

Using a Node version other than the one which Electron uses is not an appropriate solution for this.

My concern with this would be if the node module version of the newer node was different from what electron has. In this case its not. Node 12.4.0 and 12.18.3 both use NODE_MODULE_VERSION: 72. I have updated the node version on my personal fork and here are the results .

I am planning to run more builds just to make sure the issue does not resurface.

[aminya]

This is a notice for all Atom developers (@sadick254, @darangi, etc)

This week the version 7 of npm was released. It has some breaking changes. If you use npm@7 and run npm install, it will regenerate the package-lock.json. That means using apm ci (which uses the old package-lock system) on that new package-lock.json will not work. In other words, npm 7 is able to install the old package-lock.json not the other way around.

My suggestion is to merge this PR soon and update npm to npm@7 which supports both types of package-lock.json files

For now, make sure that you use npm@6:

npm i -g npm@6