Bump required Node.js version from 16 to 20 #723
Conversation
🦋 Changeset detectedLatest commit: 061de23 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This comment was marked as resolved.
This comment was marked as resolved.
|
Updated dependencies detected. Learn more about Socket for GitHub ↗︎
|
|
What is the reason for this change? |
|
Since #616 and #710 are breaking changes I'm taking the opportunity to bump the minimum Node.js version. Node.js 16 has been EOL since September 11, 2023. Also, since we now rely more on web APIs, such as Request/Response and web streams, these have only become stable since Node.js 18. In general I want to do as little major releases as possible and be able to use newer features if it improves the server/stores. That being said, I don't think we need Node.js >=20 for anything in particular right now, although web APIs were further improved in 20 compared to 18, particularly streams. Hence I bumped it to 20 to be on the safe side for a long time. But I could consider doing 18 instead. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/file-store/README.md (1)
116-119: Fix code example formatting.The example code has inconsistent formatting and is missing semicolons.
-import {MemoryConfigstore} from './MemoryConfigstore' +import { MemoryConfigstore } from "./MemoryConfigstore"; -const store = new FileStore({directory: './some/path', configstore: MemoryConfigstore}), +const store = new FileStore({ directory: "./some/path", configstore: MemoryConfigstore });README.md (1)
8-8: Fix typo in the documentation.There's a typo in the word "by": "bn accident" should be "by accident".
-or bn accident in case of an network issue or server outage. +or by accident in case of an network issue or server outage.🧰 Tools
🪛 LanguageTool
[misspelling] ~8-~8: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...s to pause, > or bn accident in case of an network issue or server outage. tus-no...(EN_A_VS_AN)
packages/s3-store/README.md (1)
106-107: Clean up unused link references.The following link references are defined but not used in the document:
[cleanUpExpiredUploads][kvstores]Remove these unused references to keep the documentation clean.
packages/azure-store/README.md (1)
106-107: Clean up unused link references.The following link references are defined but not used in the document:
[cleanUpExpiredUploads][kvstores]Remove these unused references to keep the documentation clean.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
106-106: Link and image reference definitions should be needed
Unused link or image reference definition: "cleanupexpireduploads"(MD053, link-image-reference-definitions)
107-107: Link and image reference definitions should be needed
Unused link or image reference definition: "kvstores"(MD053, link-image-reference-definitions)
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.github/workflows/ci.yml(2 hunks)README.md(3 hunks)packages/azure-store/README.md(4 hunks)packages/file-store/README.md(5 hunks)packages/gcs-store/README.md(4 hunks)packages/s3-store/README.md(8 hunks)packages/server/README.md(11 hunks)packages/utils/package.json(1 hunks)tsconfig.base.json(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
packages/azure-store/README.md
106-106: Link and image reference definitions should be needed
Unused link or image reference definition: "cleanupexpireduploads"
(MD053, link-image-reference-definitions)
107-107: Link and image reference definitions should be needed
Unused link or image reference definition: "kvstores"
(MD053, link-image-reference-definitions)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Node.js 20
🔇 Additional comments (11)
tsconfig.base.json (1)
6-6: Verify TypeScript module resolution compatibility.The change from
"node16"to"NodeNext"aligns with the Node.js version upgrade but requires verification of module resolution behavior.✅ Verification successful
🌐 Web query:
What are the differences between "node16" and "NodeNext" module resolution in TypeScript?💡 Result:
TypeScript's
node16andNodeNextmodule resolution strategies are closely related but serve different purposes:Key Differences
Target Versions
node16: Specifically mimics Node.js v12+ module resolution behavior (ESM + CommonJS)[1][2]NodeNext: Designed to track latest Node.js features, currently equivalent tonode16but intended to evolve with future Node.js versions[2][4]Future Compatibility
node16remains fixed to Node 16-era resolution rulesNodeNextwill automatically adopt new resolution behaviors as Node.js evolves[4][5]Configuration Pairing
Both require matchingmodulecompiler option:{ "compilerOptions": { "module": "NodeNext", "moduleResolution": "NodeNext" } }Using mismatched values (like
module: commonjs) causes errors in TS 5.2+[5]Shared Features
Both strategies:
- Support modern ESM/CommonJS hybrid resolution[1]
- Require explicit file extensions in relative imports (
.js/.cjs/.mjs)[8]- Respect
package.jsonexports/importsfields[2][4]- Enable strict ESM compliance checks[7]
When to Use Each
Scenario Recommendation Targeting Node 16 node16Future-proof setup NodeNext(default)Library development NodeNext[9]Bundler workflows Use bundlerinstead[3][5]As of TypeScript 5.2+,
NodeNextis recommended for new Node.js projects as it provides automatic compatibility with future ECMAScript Module updates in Node.js[5][9]. Thenode16option remains available for teams needing explicit version pinning.Citations:
- 1: https://www.typescriptlang.org/tsconfig/moduleResolution.html
- 2: https://github.com/IanVS/ts-module-resolution-examples/blob/main/moduleResolution/Node16/README.md
- 3: https://www.totaltypescript.com/workshops/typescript-pro-essentials/configuring-typescript/the-moduleresolution-option-in-tsconfigjson
- 4: https://github.com/IanVS/ts-module-resolution-examples
- 5: https://www.totaltypescript.com/concepts/option-module-must-be-set-to-nodenext-when-option-moduleresolution-is-set-to-nodenext
- 6: https://youtrack.jetbrains.com/issue/WEB-59941/WebStorm-and-Typescript-fighting-over-Node16-module-resolution-absolute-imports
- 7: "moduleResolution": "NodeNext" throws ESM related errors microsoft/TypeScript#50058
- 8: https://www.totaltypescript.com/relative-import-paths-need-explicit-file-extensions-in-ecmascript-imports
- 9: https://blog.andrewbran.ch/is-nodenext-right-for-libraries-that-dont-target-node-js/
- 10: https://community.sonarsource.com/t/can-we-support-node16-instead-of-node12-in-module-for-typescript/64000
Module Resolution Compatibility Confirmed
The change to
"module": "NodeNext"is in line with the latest recommendations for new Node.js projects using TypeScript 5.2+. As noted in the web results,"NodeNext"is designed to automatically track evolving Node.js ESM resolution behaviors, whereas"node16"is fixed to the Node 16-era rules. Ensure that themoduleResolutionoption is also set to"NodeNext"(or appropriately aligned) to avoid any mismatches.packages/utils/package.json (1)
31-31: LGTM! Node.js version requirement updated appropriately.The update to require Node.js 20+ is justified as:
- Node.js 16 reached EOL on September 11, 2023
- Project needs web APIs that are stable from Node.js 18+
- Node.js 20 provides improved web API features
.github/workflows/ci.yml (1)
19-19: LGTM! CI workflow updated to use Node.js 20.The changes to use Node.js 20 in CI align with the updated requirements in package.json files.
Also applies to: 36-36
packages/gcs-store/README.md (2)
20-20: LGTM! Documentation updated to reflect new Node.js version requirement.The README has been consistently updated to indicate Node.js 20+ requirement.
Also applies to: 77-77
29-41: Consistent code formatting improvements.The example code has been reformatted to use double quotes and consistent spacing, improving readability.
packages/file-store/README.md (2)
23-23: LGTM! Documentation updated to reflect new Node.js version requirement.The README has been consistently updated to indicate Node.js 20+ requirement.
Also applies to: 127-127
90-110: Improved TypeScript example formatting.The TypeScript example has been updated with consistent semicolons and proper type imports, enhancing code clarity.
README.md (1)
127-127: LGTM! Node.js version requirement updated.The Node.js version requirement has been correctly updated to 20+, which aligns with the project's needs for stable web APIs and long-term support.
packages/s3-store/README.md (1)
25-25: LGTM! Node.js version requirement updated.The Node.js version requirement has been correctly updated to 20+ in both the install section and compatibility section.
Also applies to: 87-87
packages/server/README.md (1)
29-29: LGTM! Node.js version requirement updated.The Node.js version requirement has been correctly updated to 20+ in both the install section and compatibility section. The documentation is comprehensive and well-maintained.
Also applies to: 634-634
packages/azure-store/README.md (1)
19-19: LGTM! Node.js version requirement updated.The Node.js version requirement has been correctly updated to 20+ in both the install section and compatibility section.
Also applies to: 87-87
* main: [ci] release (#721) @tus/s3-store: fix unhandled promise rejection (#728) Bump @aws-sdk/client-s3 from 3.717.0 to 3.758.0 (#733) Bump @types/node from 22.10.1 to 22.13.7 (#734) Bump typescript from 5.6.2 to 5.8.2 (#731) @tus/azure-store: fix non-ASCII characters error on metadata (#725) Fix package-lock.json
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/azure-store/package.json (1)
4-4: Major Version Bump: Confirm Documentation Updates.
The update from"version": "0.1.3"to"version": "1.0.0"correctly signals a major release and any potential breaking changes. Please ensure your release notes or documentation clearly highlight these changes for end users.
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
.changeset/perfect-rats-type.md(1 hunks).github/workflows/ci.yml(2 hunks)README.md(1 hunks)packages/azure-store/README.md(2 hunks)packages/azure-store/package.json(2 hunks)packages/file-store/README.md(2 hunks)packages/file-store/package.json(1 hunks)packages/gcs-store/README.md(2 hunks)packages/gcs-store/package.json(1 hunks)packages/s3-store/README.md(2 hunks)packages/s3-store/package.json(1 hunks)packages/server/README.md(2 hunks)packages/server/package.json(1 hunks)packages/utils/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (13)
- packages/gcs-store/README.md
- packages/azure-store/README.md
- packages/file-store/package.json
- packages/gcs-store/package.json
- packages/file-store/README.md
- .github/workflows/ci.yml
- packages/s3-store/package.json
- packages/s3-store/README.md
- packages/server/README.md
- README.md
- packages/server/package.json
- .changeset/perfect-rats-type.md
- packages/utils/package.json
🔇 Additional comments (1)
packages/azure-store/package.json (1)
33-35: Update Node.js Engine Requirement: Verify Compatibility Across the Project.
The"engines"field is now set to"node": ">=20.19.0", which aligns with the PR objective to bump the Node.js version to 20. This change ensures compatibility with newer web APIs. Confirm that all related documentation, CI configurations, and downstream dependencies reflect and support this new requirement.
Note
I manually bumped the version of
@tus/azure-storeto1.0.0so that changesets will release it as2.0.0so it's aligned with the other packages.require(esm), and TS type stripping, and the test runner.