feat(upgrade): add satellite auto-sync codemod for core 3 migration #7653
+270
−0
Conversation
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
Why: Core 3 changed satellite behavior — apps no longer auto-sync on first visit unless `satelliteAutoSync: true` is set. This codemod preserves Core 2 behavior by adding the prop wherever `isSatellite` is configured.
🦋 Changeset detectedLatest commit: b4d0d96 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@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: |
Contributor
📝 WalkthroughWalkthroughAdds a new codemod, 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/upgrade/src/codemods/transform-satellite-auto-sync.cjs`:
- Around line 6-47: The object transform in root.find(j.ObjectExpression)
currently finds isSatelliteIndex and unconditionally inserts satelliteAutoSync
at isSatelliteIndex + 1, which can be shadowed by a following spread; update the
logic to detect any SpreadElement/SpreadProperty (AST node representing ...obj)
after the isSatellite property: if a spread exists after isSatellite, either
skip the transform or insert the new j.objectProperty('satelliteAutoSync',
j.booleanLiteral(true)) before the first spread instead of at isSatelliteIndex +
1; adjust the check that uses isObjectPropertyNamed(...) and the insertion that
calls properties.splice(...) to use the computed safe insert index (or bail out)
so explicit property values are not overridden by spreads.
Comment on lines
+6
to
+47
| root.find(j.JSXOpeningElement).forEach(path => { | ||
| const { attributes } = path.node; | ||
| if (!attributes) { | ||
| return; | ||
| } | ||
|
|
||
| const hasIsSatellite = attributes.some(attr => isJsxAttrNamed(attr, 'isSatellite')); | ||
| if (!hasIsSatellite) { | ||
| return; | ||
| } | ||
|
|
||
| const hasAutoSync = attributes.some(attr => isJsxAttrNamed(attr, 'satelliteAutoSync')); | ||
| if (hasAutoSync) { | ||
| return; | ||
| } | ||
|
|
||
| const autoSyncAttr = j.jsxAttribute( | ||
| j.jsxIdentifier('satelliteAutoSync'), | ||
| j.jsxExpressionContainer(j.booleanLiteral(true)), | ||
| ); | ||
| attributes.push(autoSyncAttr); | ||
| dirty = true; | ||
| }); | ||
|
|
||
| // Handle object properties: { isSatellite: true } → { isSatellite: true, satelliteAutoSync: true } | ||
| root.find(j.ObjectExpression).forEach(path => { | ||
| const { properties } = path.node; | ||
|
|
||
| const hasIsSatellite = properties.some(prop => isObjectPropertyNamed(prop, 'isSatellite')); | ||
| if (!hasIsSatellite) { | ||
| return; | ||
| } | ||
|
|
||
| const hasAutoSync = properties.some(prop => isObjectPropertyNamed(prop, 'satelliteAutoSync')); | ||
| if (hasAutoSync) { | ||
| return; | ||
| } | ||
|
|
||
| const isSatelliteIndex = properties.findIndex(prop => isObjectPropertyNamed(prop, 'isSatellite')); | ||
| const autoSyncProp = j.objectProperty(j.identifier('satelliteAutoSync'), j.booleanLiteral(true)); | ||
| properties.splice(isSatelliteIndex + 1, 0, autoSyncProp); | ||
| dirty = true; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/upgrade/src/codemods/transform-satellite-auto-sync.cjsRepository: clerk/javascript
Length of output: 2830
🏁 Script executed:
cat > /tmp/test_spreads.js << 'EOF'
// Test JSX spread override semantics
// In JSX: <Component isSatellite {...obj} satelliteAutoSync={true} />
// Does the explicit attribute AFTER the spread override the spread's value?
// For objects: { isSatellite: true, ...obj, satelliteAutoSync: true }
// Does the property AFTER the spread override?
// Let's verify with a simple example
const obj = { satelliteAutoSync: false, other: 123 };
// Object property override
const result1 = { isSatellite: true, ...obj, satelliteAutoSync: true };
console.log("Object with spread BEFORE override:");
console.log(" Input: { isSatellite: true, ...{satelliteAutoSync: false}, satelliteAutoSync: true }");
console.log(" Result satelliteAutoSync:", result1.satelliteAutoSync);
console.log(" Spread is overridden by later property? ", result1.satelliteAutoSync === true ? "YES" : "NO");
const result2 = { isSatellite: true, satelliteAutoSync: true, ...obj };
console.log("\nObject with spread AFTER property:");
console.log(" Input: { isSatellite: true, satelliteAutoSync: true, ...{satelliteAutoSync: false} }");
console.log(" Result satelliteAutoSync:", result2.satelliteAutoSync);
console.log(" Spread overrides property? ", result2.satelliteAutoSync === false ? "YES (PROBLEM)" : "NO");
EOF
node /tmp/test_spreads.jsRepository: clerk/javascript
Length of output: 463
🏁 Script executed:
cat > /tmp/analyze_code.js << 'EOF'
// Simulate the current codemod logic with different spread positions
// For objects: current code does:
// const isSatelliteIndex = properties.findIndex(prop => isObjectPropertyNamed(prop, 'isSatellite'));
// properties.splice(isSatelliteIndex + 1, 0, autoSyncProp);
console.log("=== Object property insertion analysis ===\n");
// Case 1: Spread BEFORE isSatellite
console.log("Case 1: { ...obj, isSatellite: true }");
console.log(" isSatelliteIndex = 1");
console.log(" splice(1 + 1, 0, autoSyncProp)");
console.log(" Result: [ ...obj, isSatellite, satelliteAutoSync, ... ]");
console.log(" Override risk: NO (property after spread wins)\n");
// Case 2: Spread AFTER isSatellite
console.log("Case 2: { isSatellite: true, ...obj }");
console.log(" isSatelliteIndex = 0");
console.log(" splice(0 + 1, 0, autoSyncProp)");
console.log(" Result: [ isSatellite, satelliteAutoSync, ...obj, ... ]");
console.log(" Override risk: YES (spread after property wins)\n");
// For JSX: current code does:
// attributes.push(autoSyncAttr);
console.log("=== JSX attribute append analysis ===\n");
console.log("JSX appends attribute at END (last position)");
console.log(" After push: [ ...any, isSatellite, ...any, satelliteAutoSync=true ]");
console.log(" Key point: Is there ANYTHING after the push?");
console.log(" If spread is already last: spreads at END would follow the attribute");
console.log(" But push adds to end, so new attr is always last");
console.log(" Override risk: Need to check JSX semantics for attributes after spreads\n");
EOF
node /tmp/analyze_code.jsRepository: clerk/javascript
Length of output: 906
🌐 Web query:
JSX spread attributes override semantics later explicit attributes
💡 Result:
Short answer: later attributes win — an explicit prop after a spread overrides the same key from the spread. Multiple spreads follow the same last-wins rule.
Why: JSX is transpiled into a props merge (like Object.assign/_extends) where sources are applied in source order; later sources overwrite earlier ones. Example:
- <Comp {...a} x="2" /> → x === "2"
- <Comp x="1" {...a} /> → x === a.x (spread wins)
(Refs: React docs and explanation). [1][2]
Sources:
[1] React — JSX In Depth (spread attributes, order/usage).
[2] Explanation / examples showing "later attributes override previous ones".
🏁 Script executed:
cat > /tmp/verify_risk.js << 'EOF'
console.log("=== RISK ANALYSIS ===\n");
console.log("OBJECT CASE: Properties order matters\n");
// Case A: Spread BEFORE isSatellite
console.log("A. { ...obj (with satelliteAutoSync: false), isSatellite: true }");
console.log(" isSatelliteIndex = 1");
console.log(" splice(1+1=2, 0, satelliteAutoSync=true)");
console.log(" Result: [ ...obj, isSatellite, satelliteAutoSync, ] ");
console.log(" Outcome: satelliteAutoSync is AFTER spread → WINS (true is correct)");
console.log(" Risk: NONE\n");
// Case B: Spread AFTER isSatellite
console.log("B. { isSatellite: true, ...obj (with satelliteAutoSync: false) }");
console.log(" isSatelliteIndex = 0");
console.log(" splice(0+1=1, 0, satelliteAutoSync=true)");
console.log(" Result: [ isSatellite, satelliteAutoSync, ...obj, ]");
console.log(" Outcome: satelliteAutoSync is BEFORE spread → LOSES (false overrides true)");
console.log(" Risk: CRITICAL! User's satelliteAutoSync:false is lost\n");
console.log("JSX CASE: Attributes order matters\n");
console.log("C. <Comp isSatellite {...obj (with satelliteAutoSync: false)} />");
console.log(" attributes.push() appends to end");
console.log(" Result: [ isSatellite, ...spread, satelliteAutoSync=true ]");
console.log(" Transpiles to: { isSatellite, ...obj, satelliteAutoSync: true }");
console.log(" Outcome: satelliteAutoSync is AFTER spread → WINS (true is correct)");
console.log(" Risk: NONE\n");
console.log("D. <Comp {...obj (with satelliteAutoSync: false)} isSatellite />");
console.log(" attributes.push() appends to end");
console.log(" Result: [ ...spread, isSatellite, satelliteAutoSync=true ]");
console.log(" Transpiles to: { ...obj, isSatellite, satelliteAutoSync: true }");
console.log(" Outcome: satelliteAutoSync is AFTER spread → WINS (true is correct)");
console.log(" Risk: NONE\n");
console.log("CONCLUSION:");
console.log("- JSX: SAFE (push always appends to end)");
console.log("- Objects: RISKY when spread comes AFTER isSatellite");
EOF
node /tmp/verify_risk.jsRepository: clerk/javascript
Length of output: 1495
🏁 Script executed:
cat > /tmp/check_hasAutoSync.js << 'EOF'
// Check if hasAutoSync guards against the spread override risk
console.log("=== hasAutoSync check analysis ===\n");
console.log("Code: const hasAutoSync = properties.some(prop => isObjectPropertyNamed(prop, 'satelliteAutoSync'));");
console.log("This checks if satelliteAutoSync exists ANYWHERE in properties.\n");
console.log("Risk scenario:");
console.log("{ isSatellite: true, ...obj } // obj = { satelliteAutoSync: false }");
console.log("hasAutoSync checks the actual properties array");
console.log("properties = [ {isSatellite}, {spread} ]");
console.log("The spread node itself is type: SpreadElement/SpreadProperty");
console.log("hasAutoSync looks for properties with name 'satelliteAutoSync'");
console.log("The spread doesn't have a 'name', so it's not detected by isObjectPropertyNamed");
console.log("Result: hasAutoSync = false (spread content is not inspected)");
console.log("Code proceeds to add the property at wrong position");
console.log("Conclusion: hasAutoSync does NOT protect against spread override risk");
EOF
node /tmp/check_hasAutoSync.jsRepository: clerk/javascript
Length of output: 821
Guard against spreads positioned after isSatellite in objects.
When an object is { isSatellite: true, ...obj } where obj contains satelliteAutoSync, the code inserts the new property at isSatelliteIndex + 1, placing it before the spread. This allows the spread's value to override the inserted property, silently reversing user intent. The hasAutoSync check doesn't detect spreads (only named properties), so this case isn't skipped.
For JSX, the current attributes.push() is safe—appending ensures the new attribute is always last, so it wins over any preceding spread.
For objects, skip the transform if a spread appears after isSatellite, or insert before the first spread so explicit values are not overridden.
🤖 Prompt for AI Agents
In `@packages/upgrade/src/codemods/transform-satellite-auto-sync.cjs` around lines
6 - 47, The object transform in root.find(j.ObjectExpression) currently finds
isSatelliteIndex and unconditionally inserts satelliteAutoSync at
isSatelliteIndex + 1, which can be shadowed by a following spread; update the
logic to detect any SpreadElement/SpreadProperty (AST node representing ...obj)
after the isSatellite property: if a spread exists after isSatellite, either
skip the transform or insert the new j.objectProperty('satelliteAutoSync',
j.booleanLiteral(true)) before the first spread instead of at isSatelliteIndex +
1; adjust the check that uses isObjectPropertyNamed(...) and the insertion that
calls properties.splice(...) to use the computed safe insert index (or bail out)
so explicit property values are not overridden by spreads.
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.
Why
Core 3 introduced
satelliteAutoSync(defaults tofalse), changing satellite behavior. In Core 2, satellite apps always auto-synced on first visit. In Core 3, they don't unlesssatelliteAutoSync: trueis explicitly set. Users upgrading need this prop added automatically to preserve existing behavior.What changed
transform-satellite-auto-sync.cjscodemod that findsisSatelliteusage (both JSX props and object properties) and addssatelliteAutoSync: trueas a siblingsatelliteAutoSyncis already presentPackages affected
@clerk/upgrade: New codemod + testsSummary by CodeRabbit
New Features
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.