experiment: revert Node.js 24 upgrade to test e2e impact#8424
experiment: revert Node.js 24 upgrade to test e2e impact#8424jacekradko wants to merge 1 commit intomainfrom
Conversation
This reverts commit e0a63f9.
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
📝 WalkthroughWalkthroughThis PR downgrades the minimum supported Node.js version from 24.15.0 to 22.x across configuration files ( Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Review rate limit: 3/5 reviews remaining, refill in 19 minutes and 34 seconds. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/backend/src/tokens/clerkRequest.ts`:
- Line 29: The super call in the constructor of ClerkRequest uses the expression
init || typeof input === 'string' ? undefined : input which mis-parses due to
operator precedence and drops a truthy init; change the second argument to use
init as the primary value and only fall back to the ternary when init is falsy
by parenthesizing the ternary (i.e., pass init || (typeof input === 'string' ?
undefined : input)) so provided init is preserved; update the super(...) call
accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 27d8db00-7eec-4f6f-a813-17c8ea86ced1
📒 Files selected for processing (12)
.github/actions/init-blacksmith/action.yml.github/actions/init/action.yml.github/workflows/ci.yml.github/workflows/release.yml.nvmrcpackage.jsonpackages/backend/src/__tests__/proxy.test.tspackages/backend/src/proxy.tspackages/backend/src/tokens/clerkRequest.tspackages/react-router/src/server/utils.tspackages/tanstack-react-start/src/__tests__/patchRequest.test.tspackages/tanstack-react-start/src/server/utils/index.ts
| }) as unknown as RequestInit; | ||
| } | ||
| super(url, cloneInit); | ||
| super(url, init || typeof input === 'string' ? undefined : input); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's read the file to understand the context
head -40 packages/backend/src/tokens/clerkRequest.ts | cat -nRepository: clerk/javascript
Length of output: 2270
🏁 Script executed:
# Verify JavaScript operator precedence using Node.js
node <<'EOF'
// Test the precedence claim
// The expression: init || typeof input === 'string' ? undefined : input
// Simulating different values
const test = (init, input) => {
// Original expression (as reviewer claims it parses)
const original = (init || typeof input === 'string') ? undefined : input;
// Proposed fix
const fixed = init || (typeof input === 'string' ? undefined : input);
console.log(`init="${init}", input="${input}"`);
console.log(` Original (claimed): ${original}`);
console.log(` Fixed: ${fixed}`);
console.log(` Different? ${original !== fixed}`);
console.log('');
};
// Test cases
test(null, 'string'); // init falsy, input is string
test(null, {}); // init falsy, input is object
test({data: 'x'}, 'string'); // init truthy, input is string
test({}, {}); // both truthy
EOFRepository: clerk/javascript
Length of output: 518
Fix operator precedence bug on Line 29 – init parameter is being dropped.
Due to JavaScript operator precedence, init || typeof input === 'string' ? undefined : input parses as (init || typeof input === 'string') ? undefined : input, causing any provided init to be ignored when it's truthy. This breaks request initialization/cloning when init is passed.
Suggested fix
- super(url, init || typeof input === 'string' ? undefined : input);
+ super(url, init || (typeof input === 'string' ? undefined : input));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/backend/src/tokens/clerkRequest.ts` at line 29, The super call in
the constructor of ClerkRequest uses the expression init || typeof input ===
'string' ? undefined : input which mis-parses due to operator precedence and
drops a truthy init; change the second argument to use init as the primary value
and only fall back to the ternary when init is falsy by parenthesizing the
ternary (i.e., pass init || (typeof input === 'string' ? undefined : input)) so
provided init is preserved; update the super(...) call accordingly.
|
Closing — the same 5 |
1 participant
Do not merge. Sandbox to verify whether reverting #8351 (Node.js 24 upgrade) flips the consistently-failing express / hono / generic / nextjs / billing / machine integration suites back to green.
If the integration matrix on this branch goes mostly green compared to recent runs on `main`, that confirms #8351 is the regression source. Will be closed regardless of outcome.
Sibling experiment: #8423 (revert PR 8035).
Related: #8422 (the actual fix PR currently open).