Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
export const fixtures = [
{
name: 'JSX: isSatellite={true}',
source: `
import { ClerkProvider } from '@clerk/nextjs';

export function App({ children }) {
return (
<ClerkProvider isSatellite={true} domain="example.com">
{children}
</ClerkProvider>
);
}
`,
output: `
import { ClerkProvider } from '@clerk/nextjs';

export function App({ children }) {
return (
<ClerkProvider isSatellite={true} domain="example.com" satelliteAutoSync={true}>
{children}
</ClerkProvider>
);
}
`,
},
{
name: 'JSX: isSatellite (boolean shorthand)',
source: `
import { ClerkProvider } from '@clerk/react';

export const App = () => (
<ClerkProvider isSatellite domain="satellite.example.com">
<Main />
</ClerkProvider>
);
`,
output: `
import { ClerkProvider } from '@clerk/react';

export const App = () => (
<ClerkProvider isSatellite domain="satellite.example.com" satelliteAutoSync={true}>
<Main />
</ClerkProvider>
);
`,
},
{
name: 'JSX: isSatellite with function value',
source: `
import { ClerkProvider } from '@clerk/nextjs';

export function App({ children }) {
return (
<ClerkProvider isSatellite={(url) => url.host === 'satellite.example.com'} domain="example.com">
{children}
</ClerkProvider>
);
}
`,
output: `
import { ClerkProvider } from '@clerk/nextjs';

export function App({ children }) {
return (
<ClerkProvider
isSatellite={(url) => url.host === 'satellite.example.com'}
domain="example.com"
satelliteAutoSync={true}>
{children}
</ClerkProvider>
);
}
`,
},
{
name: 'JSX: already has satelliteAutoSync',
source: `
import { ClerkProvider } from '@clerk/nextjs';

export function App({ children }) {
return (
<ClerkProvider isSatellite={true} satelliteAutoSync={false} domain="example.com">
{children}
</ClerkProvider>
);
}
`,
noChange: true,
},
{
name: 'Object: isSatellite in clerkMiddleware options',
source: `
import { clerkMiddleware } from '@clerk/nextjs/server';

export default clerkMiddleware({
isSatellite: true,
domain: 'example.com',
});
`,
output: `
import { clerkMiddleware } from '@clerk/nextjs/server';

export default clerkMiddleware({
isSatellite: true,
satelliteAutoSync: true,
domain: 'example.com'
});
`,
},
{
name: 'Object: isSatellite in variable declaration',
source: `
const options = {
isSatellite: true,
domain: 'satellite.example.com',
};
`,
output: `
const options = {
isSatellite: true,
satelliteAutoSync: true,
domain: 'satellite.example.com'
};
`,
},
{
name: 'Object: isSatellite with function value',
source: `
import { clerkMiddleware } from '@clerk/nextjs/server';

export default clerkMiddleware({
isSatellite: (url) => url.host === 'satellite.example.com',
domain: 'example.com',
});
`,
output: `
import { clerkMiddleware } from '@clerk/nextjs/server';

export default clerkMiddleware({
isSatellite: (url) => url.host === 'satellite.example.com',
satelliteAutoSync: true,
domain: 'example.com'
});
`,
},
{
name: 'Object: already has satelliteAutoSync',
source: `
const options = {
isSatellite: true,
satelliteAutoSync: false,
domain: 'example.com',
};
`,
noChange: true,
},
{
name: 'No isSatellite present (no changes)',
source: `
import { ClerkProvider } from '@clerk/nextjs';

export function App({ children }) {
return (
<ClerkProvider domain="example.com">
{children}
</ClerkProvider>
);
}
`,
noChange: true,
},
];
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { applyTransform } from 'jscodeshift/dist/testUtils';
import { describe, expect, it } from 'vitest';

import transformer from '../transform-satellite-auto-sync.cjs';
import { fixtures } from './__fixtures__/transform-satellite-auto-sync.fixtures';

describe('transform-satellite-auto-sync', () => {
it.each(fixtures)('$name', ({ source, output, noChange }) => {
const result = applyTransform(transformer, {}, { source });

if (noChange) {
expect(result).toEqual('');
} else {
expect(result).toEqual(output.trim());
}
});
});
74 changes: 74 additions & 0 deletions packages/upgrade/src/codemods/transform-satellite-auto-sync.cjs
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is adding satelliteAutoSync to every component and object that has a isSatellite overly broad, given there might be unrelated components/objects that also has it?

I get this is tricky to narrow down given props and objects can be passed around, is the idea that we'd rather have false positives than miss some?

Seems like an edge case, should still be safe and most people review the output after running a codemod anyway so I'm very fine with this, just wanted to double check this is the intent?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review Fredrik :) I thought about this as well and I decided that I could live with false positives given how impactful the change in behavior could be for apps migrating from Core 2 to Core 3. Happy to change this of course

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking more about this and your comment below, I decided to align the behavior and always add the prop at the end - I do think its important for as to override always, and let the end user decide in the very rare event that they have already added satelliteAutoSync manually (very unlikely as this prop will be introduced with core3)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very true, sounds good to me! 🙏

Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
module.exports = function transformSatelliteAutoSync({ source }, { jscodeshift: j }) {
const root = j(source);
let dirty = false;

// Handle JSX attributes: <Component isSatellite /> → <Component isSatellite satelliteAutoSync={true} />
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;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
});

return dirty ? root.toSource() : undefined;
};

module.exports.parser = 'tsx';

function isJsxAttrNamed(attribute, name) {
return attribute && attribute.type === 'JSXAttribute' && attribute.name && attribute.name.name === name;
}

function isObjectPropertyNamed(prop, name) {
if (!prop || (prop.type !== 'ObjectProperty' && prop.type !== 'Property')) {
return false;
}
const { key } = prop;
if (!key) {
return false;
}
if (key.type === 'Identifier') {
return key.name === name;
}
if (key.type === 'StringLiteral' || key.type === 'Literal') {
return key.value === name;
}
return false;
}
1 change: 1 addition & 0 deletions packages/upgrade/src/versions/core-3/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,6 @@ export default {
{ name: 'transform-clerk-provider-inside-body', packages: ['nextjs'] },
// Migrate @clerk/react-router/api.server → @clerk/react-router/server
{ name: 'transform-react-router-api-server', packages: ['react-router'] },
{ name: 'transform-satellite-auto-sync', packages: ['nextjs', 'react', 'expo', 'astro', 'tanstack-react-start'] },
],
};
Loading