Blueprints and Client: update engine to node v20 on package.json
#591
Conversation
|
@sejas should we bump the version for all packages and also in the top-level I'm just wondering if there's any benefit to having different minimum versions in the different packages |
There was a problem hiding this comment.
there are some JS classes such as
CustomEventandFilethat are implemented in node v20.
Would it be possible to shim these classes so we can keep node 16/18 support? If node 18 is LTS, I'm concerned that we'll lose some number of users by requiring node 20. It might break the VS Code Extension too.
@dmsnell, the benefit would be that the rest of the packages will work in third party apps with lower node versions. @adamziel asked to upgrade just those packages.
Maybe I understood it too literally. I have doubts if that engine section will be overwritten in the next release. |
@danielbachhuber, Yes, it's possible.
I took this approach since @adamziel suggested it over writing polyfills.
This week, I can estimate the level of effort to pull the NodeJS code and support lower versions. And we can decide if it's worth it or not. Sounds good? |
@sejas Sure, that's fine. |
|
This is pure personal preference, but I'm fairly okay with aggressively updating the minimum version and even more so of pinning all the versions together. It could be nice indicating a given package doesn't rely on a certain version, but then once people use the packages together they would need to update anyway. Also if we can avoid polyfills by bumping the minimum version then that's code we don't have to write and schedule to remove later when it's no longer needed because of some actual version requirement bump. This is still a fairly new and rapid project. We can make some more pressing demands on versions, plus if someone really wants to they can still run it in old versions, but at their own risk. The version information in |
@dmsnell I think this is at odds with "most users" being able to use 
See some conversation in WordPress/playground-tools#83. I think we should support LTS as a minimum if we care about other users being able to use both WordPress Playground and |
|
@danielbachhuber that's fine, which is why I added the caveat stating my personal preference. there are two issues in the quoted comment though: minimum version and harmonizing the versions. for the sake of integrations there is still potential value in making all packages require the same base version. it's obviously not a problem to specify separate ones, but it's still a tradeoff I think we are free to consider. |
|
Found out that the current version of VS Code bundles Node.js
It's a somewhat old version that comes bundled with Electron, which lags behind until its Node.js runtime is merged from upstream. Currently, VS Code bundles Electron 22. Even the newest version of Electron 25 bundles Node.js https://releases.electronjs.org/ That means, as @danielbachhuber suggested earlier, the VS Code extension for the Playground needs to support Node.js 16/18 for the foreseeable future. As far as we know, the only thing necessary to meet that requirement is to polyfill the |
|
Oh yeah, I forgot about the VS Code Extension. @dmsnell and I didn't factor it into our conversation yesterday, whoops. Thanks for mentioning! |
|
Sounds good; let's just polyfill then. Thanks for noticing that @eliot-akira |
|
## Motivation for the change, related issues [Playground depends on crypto](https://github.com/WordPress/wordpress-playground/blob/07fb8bac24f02db0c40a2f465597a05021c01d7e/packages/playground/website/src/lib/site-metadata.ts#L82) which isn't available in Node 18, which [is preventing us from creating Playground CLI tests.](Automattic/wordpress-playground-private#39 (comment)) In the past, we kept the [Node version at 18 to ensure Playground works in Electron apps like VSCode and Studio](#591 (comment)). Today Electron includes Node 20, so we are safe to update. I chose Node 20.9.0 because it's the version of Node that Studio uses. ## Implementation details To update Playground I did the following: - Update the Node version in all package.json files - Updated `.nvmrc` - Update the Node version used in CI - Set a minimal Node version Emscripten should target - Update Node tyles - Ran `npm install` - Checked manually if there are any mentions of Node 18 or polyfills that we can remove. This PR also adds a global `vitest.setup.ts` file with some JSDOM function polyfills/replacements. This was required because JSDOM replaces the existing Node.js APIs with their versions of Web APIs, and sometimes these APIs are missing features already supported in Node.js. Read [the comment in vitest.setup.ts for more details.](https://github.com/Automattic/wordpress-playground-private/blob/update/min-node-version-to-20/vitest.setup.ts) ## Testing Instructions (or ideally a Blueprint) - CI
What is this PR doing?
Update the node version required for:
@wp-playground/blueprints@wp-playground/clientWhat problem is it solving?
CustomEventandFilethat are implemented in node v20.How is the problem addressed?
Update the node version defined in
package.json>engineTesting Instructions