Support Node 14 (again)#1112
Merged
ianstormtaylor merged 1 commit intoianstormtaylor:mainfrom Sep 22, 2022
Merged
Conversation
It appears that between 0.16.0 and 0.16.1, the minimum version of Node required to use this package changed, from 14.x to 16.x. This was not explicit but seems to have been caused by a couple of factors. But first, what changed. If you look at `src/error.ts` in 0.16.0 you will see [this line][1]: ``` return (cached ??= [failure, ...failures()]) ``` In the [published version of this file in 0.16.0][2] this gets transpiled to: ``` return (_cached = cached) != null ? _cached : cached = [failure, ...failures()]; ``` In 0.16.1, the [original line is unchanged][3], but in the [published version][4] it is transpiled to: ``` return cached ??= [failure, ...failures()]; ``` The `??=` syntax is not supported by Node 14, hence, developers are forced to upgrade to at least Node 16 if they want to use v0.16.1 or greater. After looking at the diff between these two versions and running some experiments, I believe that there are two reasons why this line shows up differently in these two published versions. 1. Different Node versions were used to build and publish these versions. It appears that Node 14 was used for the former whereas Node 16 was used for the latter. This assertion is supported by the fact that in the [Rollup configuration][5], `@babel/preset-env` is configured with `node: true`, which instructs Babel to [use the current version of Node as a target][6]. So if the current Node version changes, so does the Babel config. 2. Between 0.16.0 and 0.16.1, [`browserslist` was updated from 4.20.3, to 4.21.4][7] (you will probably need to expand `yarn.lock`; if so, Cmd-F for "browserslist"). In `browserslist` 4.21.0, [IE 11 was removed from the default set of browsers][8] (which is being used in this case, since no explicit list is provided). According to caniuse, [IE 11 does not support the `??=` syntax][9], so its removal means that Babel doesn't need to transpile this syntax any longer. To address this problem, this PR: * changes the Rollup configuration mentioned above to use `node: "14.0"` instead of `node: true`, so that Node 14 is always used to compute the transpilation rules regardless of the version of Node used locally to build and publish the package * updates the CI workflow to ensure that Node 14 is being tested (along with 16 and 18) * adds Node >= 14 to the `engines` field to communicate that it is supported --- One thing you may wonder is why this change is needed at all. Node 16 is the current LTS, so shouldn't that be enough? True, but Node 14 hasn't hit end-of-life yet, and many people are still using it, including my company. We think this package is really great, but it would be even better if we didn't have to have a workaround for our libraries that we still want to keep on Node 14. Thanks for considering :) [1]: https://github.com/ianstormtaylor/superstruct/blob/v0.16.0/src/error.ts#L44 [2]: https://unpkg.com/superstruct@0.16.0/lib/index.cjs [3]: https://github.com/ianstormtaylor/superstruct/blob/v0.16.1/src/error.ts#L44 [4]: https://unpkg.com/superstruct@0.16.1/lib/index.cjs.js [5]: https://github.com/ianstormtaylor/superstruct/blob/v0.16.4/config/rollup.js#L26 [6]: https://babel.dev/docs/en/options#targetsnode [7]: https://github.com/ianstormtaylor/superstruct/compare/v0.16.0...v0.16.1?diff=unified#diff-51e4f558fae534656963876761c95b83b6ef5da5103c4adef6768219ed76c2deL99 [8]: https://github.com/browserslist/browserslist/blob/main/CHANGELOG.md#421 [9]: https://caniuse.com/?search=%3F%3F%3D
97c5fd3 to
586a18e
Compare
Contributor
Author
|
This is a carryover from the comment I left in this issue: #1111 (comment). I know originally I was going to submit a PR that set |
Owner
|
@mcmire this is an absolutely beautiful description and pull request. Thank you! I completely agree with everything you said. And I'm sorry for the regression. |
Contributor
Author
|
Super, no problem. Thanks for the fast merge/release! |
mcmire
added a commit
to MetaMask/utils
that referenced
this pull request
Sep 22, 2022
Published versions of `superstruct` between 0.16.1 and 0.16.4 contained syntax (`??=`) that Node did not support until v15. This effectively changed the minimum required Node version for this package, and any other package using this package, to v15. A [fix for this issue][1] landed in `superstruct` 0.16.5, so this commit bumps that library accordingly so as to restore compatibility with Node 14. [1]: ianstormtaylor/superstruct#1112
mcmire
added a commit
to MetaMask/utils
that referenced
this pull request
Sep 22, 2022
Published versions of `superstruct` between 0.16.1 and 0.16.4 contained syntax (`??=`) that Node did not support until v15. This effectively changed the minimum required Node version for this package, and any other package using this package, to v15. A [fix for this issue][1] landed in `superstruct` 0.16.5, so this commit bumps that library accordingly so as to restore compatibility with Node 14. [1]: ianstormtaylor/superstruct#1112
mcmire
added a commit
to mcmire/core
that referenced
this pull request
Jul 17, 2023
Published versions of `superstruct` between 0.16.1 and 0.16.4 contained syntax (`??=`) that Node did not support until v15. This effectively changed the minimum required Node version for this package, and any other package using this package, to v15. A [fix for this issue][1] landed in `superstruct` 0.16.5, so this commit bumps that library accordingly so as to restore compatibility with Node 14. [1]: ianstormtaylor/superstruct#1112
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
It appears that between 0.16.0 and 0.16.1, the minimum version of Node required to use this package changed, from 14.x to 16.x. This was not explicit but seems to have been caused by a couple of factors.
But first, what changed. If you look at
src/error.tsin 0.16.0 you will see this line:In the published version of this file in 0.16.0 this gets transpiled to:
In 0.16.1, the original line is unchanged, but in the published version it is transpiled to:
The
??=syntax is not supported by Node 14, hence, developers are forced to upgrade to at least Node 16 if they want to use v0.16.1 or greater.After looking at the diff between these two versions and running some experiments, I believe that there are two reasons why this line shows up differently in these two published versions.
@babel/preset-envis configured withnode: true, which instructs Babel to use the current version of Node as a target. So if the current Node version changes, so does the Babel config.browserslistwas updated from 4.20.3, to 4.21.4 (you will probably need to expandyarn.lock; if so, Cmd-F for "browserslist"). Inbrowserslist4.21.0, IE 11 was removed from the default set of browsers (which is being used in this case, since no explicit list is provided). According to caniuse, IE 11 does not support the??=syntax, so its removal means that Babel doesn't need to transpile this syntax any longer.To address this problem, this PR:
node: "14.0"instead ofnode: true, so that Node 14 is always used to compute the transpilation rules regardless of the version of Node used locally to build and publish the packageenginesfield to communicate that it is supportedOne thing you may wonder is why this change is needed at all. Node 16 is the current LTS, so shouldn't that be enough? True, but Node 14 hasn't hit end-of-life yet, and many people are still using it, including my company. We think this package is really great, but it would be even better if we didn't have to have a workaround for our libraries that we still want to keep on Node 14.
Thanks for considering :)