Node LTS 12

[sirreal]

Node v12 is now LTS. Use it.

Updates node-sass as well so there are available binaries.

Blockers

• FSE app needs to be upgraded (FSE: Update calypso-build to latest 4.2.1 #37058)
• WP Desktop (https://github.com/Automattic/wp-desktop/issues/686)

[matticbot]

Test live: https://calypso.live/?branch=update/node-lts-12

[sirreal]

This needs an update:

https://github.com/Automattic/wp-calypso/blob/eb8b53baf4bb033d75920eb6b6d339ca2b076be0/package.json#L214

Do we allow the last LTS for a while before requiring everyone to be on v12, or just make the switch?

https://docs.npmjs.com/files/package.json#engines

[matticbot]

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

[sirreal]

FSE fails because it's package-lock uses the old node because it depends on a pinned calypso-build version 🤔

We could run that build on the older node version, or release another version of calypso-build after #35898 lands.

[sirreal]

👋 @loremattei It looks like wp-desktop may have problems with Node 12.

electron-spellchecker isn't building.

[sirreal]

We could run that build on the older node version

Not working.

or release another version of calypso-build after #35898 lands.

Seems we'll need to do this.

[loremattei]

Hey @sirreal! Thanks for the ping!

This requires an update to the Electron version used in WPDesktop. @nsakaimbo has already started to work on this and will be able to continue in a couple of weeks, but it's a non trivial task that may require some time because we currently have a huge tech debt (we are on Electron 1.7).

So, we'll probably need a B-plan in order to be able to continue to build the app in the meantime.

cc/ @belcherj

[blowery]

So, we'll probably need a B-plan in order to be able to continue to build the app in the meantime.

That might just be to relax engines for a while to continue to allow the 10.x LTS line. I don't think we're doing anything with the upgrade that precludes the old line...

[nsakaimbo]

That might just be to relax engines for a while

That might be worth a try, although not sure if the current spellchecker will play nice with that. TL;DR - if we change the version of the global node environment that WP-Desktop is compiled in (from 10.x to 12.x), it will try to fetch prebuilt binaries for the spellchecker that correspond to that version of Node. The problem is, we found out, the prebuilt spellchecker repo hasn't been updated since 2017, so compilation will fail because the prebuilt spellchecker doesn't exist for Node 12.x (yikes!). This will take some effort to unwind (and we've already made some progress).

However perhaps I'm misunderstanding your suggestion, and there's some other way we can work around it in the mean time?

Worst case, can we hold back the SHA of the version of Calypso we're using in WP-Desktop until the Electron upgrade is complete and these issues are resolved? cc: @loremattei @belcherj

[blowery]

However perhaps I'm misunderstanding your suggestion, and there's some other way we can work around it in the mean time?

I think maybe? I'm just saying that we could relax engines so that we can continue to allow Node@10. Not sure how that might play out with the .nvmrc and the build tooling though...

[sirreal]

Automattic/wp-desktop#688 may let wp-desktop stay behind while wp-calypso moves ahead.

[gwwar]

Maybe also cc @Automattic/i18n to double-check translation scripts. I recall we had some issues in the past on major version bumps.

[gwwar]

the prebuilt spellchecker repo hasn't been updated since 2017

@nsakaimbo are there any other viable alternatives that are maintained?

[sirreal]

All tests are green here, time for testing and review 🚀

[belcherj]

@gwwar There really aren't any. It may be a situation that we have to fork and update the spellchecker.

[belcherj]

@nsakaimbo @gwwar This is good news though: atom/node-spellchecker#128

That's the package underneath that is failing.

[sirreal]

@belcherj @gwwar As far as I can see we're free to land this since Automattic/wp-desktop#688 was merged. Is that accurate or are there remaining blockers?

All tests are green but I won't land this without some explicit approval.

[akirk]

npm run translate seems to be working fine so good to go from our @Automattic/i18n perspective.

[nsakaimbo]

@sirreal Have you verified that this branch can be built with wp-desktop, at least locally? Even though Automattic/wp-desktop#688 has landed, would be great to confirm that things are continuing to build as expected.

@gwwar

(re: spellchecker) are there any other viable alternatives that are maintained?

From my research, it seems no one else has even attempted to maintain an up-to-date prebuild fork of the spellchecker. We'll have to switch to using the spellchecker source in order to move forward with the Electron upgrade. Using prebuilds has proven highly impractical to maintain, so it's no surprise the project being relied on was effectively dropped - a little more context in this gist here.

[sirreal]

@sirreal Have you verified that this branch can be built with wp-desktop, at least locally?

As part of Automattic/wp-desktop#688, I changed the Calypso submodule to use this branch (Automattic/wp-desktop@fd626ff) and the tests passed:

https://circleci.com/gh/Automattic/wp-desktop/47765

[blowery]

LGTM!

[simison]

FYI @Automattic/jetpack-crew would be good to update also on Jetpack because so many folks work on both and would need to be switching between versions each time.