fix: add Node 18 deprecation notice #1506
Conversation
Pull Request Test Coverage Report for Build 16472859267Details
💛 - Coveralls |
There was a problem hiding this comment.
Pull Request Overview
This PR implements a deprecation notice for Node.js 18 as announced in the Supabase community discussions. The changes add runtime detection and warning for Node.js 18 usage while also improving the CI coverage reporting workflow.
- Adds runtime deprecation warning for Node.js 18 environments
- Updates CI workflow to support parallel coverage reporting with proper completion handling
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/index.ts | Adds Node.js 18 deprecation detection and console warning |
| .github/workflows/ci.yml | Updates coverage reporting to use parallel processing with finish step |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
grdsdev
force-pushed
the
guilherme/clibs-244-add-warning-in-js-lib-when-node-version-equals-18
branch
from
034be14 to
ec3c428
Compare
|
I just realized |
|
Great @soedirgo , didn't know that. I will add the engine field |
package.json
Outdated
| @@ -86,5 +86,8 @@ | |||
| }, | |||
| "jsdelivr": "dist/umd/supabase.js", | |||
| "unpkg": "dist/umd/supabase.js", | |||
| "engines": { | |||
| "node": ">=20" | |||
There was a problem hiding this comment.
Just sharing my experience here: Many many people see this as a breaking change as some package managers will outright fail if they encounter conflicts with the used node version which can cause breakage in CI.
There was a problem hiding this comment.
Should we at least add a note that it's a potential breaking change in some configurations then in this case?
There was a problem hiding this comment.
If that can be treated as breaking change by some package managers, then I'd rather remove it and keep only the console.warning.
There was a problem hiding this comment.
What is said here
won't get enforced unless
engine-strictis set
this narrows down the cases this can be a breaking change, but there still are such cases.
I think what you suggest is safer, have the warning, and roll out the engines.node setting after some time?
There was a problem hiding this comment.
I rollback a few changes, please review it again @mandarini
There was a problem hiding this comment.
@lforst good call; had a feeling that could be the case. Let's go ahead with the console.warning.
package.json
Outdated
| @@ -86,5 +86,8 @@ | |||
| }, | |||
| "jsdelivr": "dist/umd/supabase.js", | |||
| "unpkg": "dist/umd/supabase.js", | |||
| "engines": { | |||
| "node": ">=20" | |||
There was a problem hiding this comment.
Should we at least add a note that it's a potential breaking change in some configurations then in this case?
- Add browser environment check to only show Node.js deprecation warning in Node.js - Fix typo in types import path 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Fix typo in types import path - Improve formatting of Node.js 18 deprecation warning condition 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add deprecation warning for Node.js versions 18 and below - Extract shouldShowDeprecationWarning function with robust version parsing - Parse major version number from process.version using regex - Handle edge cases: null version, malformed strings, browser environments - Display informative warning message directing users to upgrade to Node.js 20+ - Include link to GitHub discussion for more information
grdsdev
force-pushed
the
guilherme/clibs-244-add-warning-in-js-lib-when-node-version-equals-18
branch
from
3fc7863 to
ae0ffbb
Compare
What kind of change does this PR introduce?
As announced in https://github.com/orgs/supabase/discussions/37217, Node 18 support has been deprecated.
This PR: