Skip to content

Commit d2551ba

Browse files
os-zhuangclaude
andauthored
fix(scripts): --check reports real divergence instead of calling all 46 components "modified" (#3049)
The heuristic was `lineDiff > 10 || localContent.includes('ObjectUI') || localContent.includes('data-slot')`. Every synced file carries the ObjectUI header this script prepends, so the second clause was true for all 46 of them: `--check` reported the entire tracked set as "modified", every time, which is the same as reporting nothing. #3035 added `localOnlyLines` for the pre-write guard. It is directional — lines in the first argument the second lacks — so calling it both ways answers the two questions that actually decide whether to sync: what would a sync DESTROY, and what would it GAIN. That replaces the heuristic, and splits the old "modified" bucket in two: ↑ outdated upstream moved, no local edits → safe to sync ⚠ modified local edits present → --update refuses Each line now carries counts, and the summary names the components in each bucket so the next step is obvious without running --diff 46 times. What it says about this repo right now: ✓ Identical: 29 ↑ Outdated: 0 ⚠ Modified: 17 ● Custom: 2 Zero outdated is the headline. Every component where upstream has moved also carries local edits, so there is currently no component `--update-all` could touch without deleting something — it has no safe work to do at all. That was invisible before. The per-component counts line up with the guard that enforces them (`command` 35, `select` 32, `sidebar` 24, `table` 17 …), because both sides call the same function. `--check` stays exit 0: it is a status report, and 17 locally-modified components is the steady state here, not a failure. Verified against the live registry. The `outdated` bucket is the one real traffic never exercises, so it was tested by deleting a line from an identical component (`separator`): it flips to `↑ … 1 upstream line(s) to pick up — safe to sync` and the summary offers the exact --update command. Restored after. `--list`, `--diff`, `--update` and both #3033 / #3035 guards regression-checked; no component file changes. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent b1d02ec commit d2551ba

1 file changed

Lines changed: 61 additions & 25 deletions

File tree

scripts/shadcn-sync.js

Lines changed: 61 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -229,28 +229,50 @@ async function checkComponent(name, manifest) {
229229
try {
230230
const localContent = await fs.readFile(localPath, 'utf-8');
231231
const localLines = localContent.split('\n').length;
232-
232+
233233
// Fetch latest from registry
234234
try {
235235
const registryData = await fetchUrl(componentInfo.source);
236236
// Rewrite before comparing, so import-path style alone does not read as
237237
// a difference — the local file has already been through this transform.
238238
const shadcnContent = rewriteRegistryImports(registryData.files?.[0]?.content || '');
239239
const shadcnLines = shadcnContent.split('\n').length;
240-
241-
// Simple heuristic: check if significantly different
242-
const lineDiff = Math.abs(localLines - shadcnLines);
243-
const isDifferent = lineDiff > 10 || localContent.includes('ObjectUI') || localContent.includes('data-slot');
244-
240+
241+
// `localOnlyLines` is directional — lines in the first argument that the
242+
// second lacks — so calling it both ways answers the two questions that
243+
// actually decide whether to sync: what would a sync DESTROY, and what
244+
// would it GAIN?
245+
//
246+
// The previous heuristic (`lineDiff > 10 || includes('ObjectUI')`) could
247+
// not answer either. Every synced file carries the ObjectUI header this
248+
// script prepends, so the second clause was true for all 46 of them and
249+
// `--check` reported the entire set as "modified" — no signal at all.
250+
const localOnly = localOnlyLines(localContent, shadcnContent);
251+
const upstreamOnly = localOnlyLines(shadcnContent, localContent);
252+
253+
let status;
254+
let message;
255+
if (localOnly.length > 0) {
256+
// `--update` refuses these; the local lines would be deleted.
257+
status = 'modified';
258+
message = `${localOnly.length} local line(s) upstream lacks — --update would refuse`;
259+
if (upstreamOnly.length > 0) message += `, ${upstreamOnly.length} upstream line(s) pending`;
260+
} else if (upstreamOnly.length > 0) {
261+
status = 'outdated';
262+
message = `${upstreamOnly.length} upstream line(s) to pick up — safe to sync`;
263+
} else {
264+
status = 'synced';
265+
message = 'Identical to upstream';
266+
}
267+
245268
return {
246269
name,
247-
status: isDifferent ? 'modified' : 'synced',
270+
status,
248271
localLines,
249272
shadcnLines,
250-
lineDiff,
251-
message: isDifferent ?
252-
`Modified (${lineDiff} lines difference)` :
253-
'Synced with Shadcn',
273+
localOnly: localOnly.length,
274+
upstreamOnly: upstreamOnly.length,
275+
message,
254276
};
255277
} catch (fetchError) {
256278
return {
@@ -280,45 +302,59 @@ async function checkAllComponents() {
280302

281303
const results = {
282304
synced: [],
305+
outdated: [],
283306
modified: [],
284307
custom: [],
285308
error: [],
286309
};
287-
310+
288311
for (const component of localComponents) {
289312
const result = await checkComponent(component, manifest);
290313
results[result.status].push(result);
291-
314+
292315
const symbol = {
293316
synced: '✓',
317+
outdated: '↑',
294318
modified: '⚠',
295319
custom: '●',
296320
error: '✗',
297321
}[result.status];
298-
322+
299323
const color = {
300324
synced: 'green',
325+
outdated: 'cyan',
301326
modified: 'yellow',
302327
custom: 'blue',
303328
error: 'red',
304329
}[result.status];
305-
330+
306331
log(`${symbol} ${result.name.padEnd(25)} ${result.message}`, color);
307332
}
308-
333+
309334
// Summary
310335
logSection('Summary');
311-
log(`✓ Synced: ${results.synced.length} components`, 'green');
312-
log(`⚠ Modified: ${results.modified.length} components`, 'yellow');
313-
log(`● Custom: ${results.custom.length} components`, 'blue');
314-
log(`✗ Errors: ${results.error.length} components`, 'red');
315-
336+
log(`✓ Identical: ${results.synced.length} components`, 'green');
337+
log(`↑ Outdated: ${results.outdated.length} components (no local edits — safe to sync)`, 'cyan');
338+
log(`⚠ Modified: ${results.modified.length} components (local edits — --update refuses)`, 'yellow');
339+
log(`● Custom: ${results.custom.length} components (not tracked upstream)`, 'blue');
340+
log(`✗ Errors: ${results.error.length} components`, 'red');
341+
342+
// The actionable bucket: upstream moved and nothing local is at risk.
343+
if (results.outdated.length > 0) {
344+
log('\nSafe to sync now:', 'cyan');
345+
log(` node scripts/shadcn-sync.js --update ${results.outdated[0].name}`, 'dim');
346+
if (results.outdated.length > 1) {
347+
log(` (${results.outdated.length} in total: ${results.outdated.map((r) => r.name).join(', ')})`, 'dim');
348+
}
349+
}
350+
316351
if (results.modified.length > 0) {
317-
log('\nTo update a component:', 'cyan');
318-
log(' node scripts/shadcn-sync.js --update <component-name>', 'dim');
319-
log(' node scripts/shadcn-sync.js --update-all', 'dim');
352+
log('\nThese carry local edits a sync would delete:', 'yellow');
353+
log(` ${results.modified.map((r) => `${r.name}(${r.localOnly})`).join(', ')}`, 'dim');
354+
log(' Port the edits onto the new upstream version by hand, or --force to', 'dim');
355+
log(' take upstream as-is. `--diff <name>` shows both sides.', 'dim');
320356
}
321-
357+
322358
return results;
323359
}
324360

0 commit comments

Comments
 (0)