From bdcfb69ea01c2a1ebdc748416be14ee85fbb0c61 Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Thu, 30 Jul 2026 21:53:50 +0800 Subject: [PATCH] fix(scripts): --check reports real divergence instead of calling all 46 components "modified" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: Claude Opus 5 --- scripts/shadcn-sync.js | 86 ++++++++++++++++++++++++++++++------------ 1 file changed, 61 insertions(+), 25 deletions(-) diff --git a/scripts/shadcn-sync.js b/scripts/shadcn-sync.js index 9f4118065..eb097b95e 100755 --- a/scripts/shadcn-sync.js +++ b/scripts/shadcn-sync.js @@ -229,7 +229,7 @@ async function checkComponent(name, manifest) { try { const localContent = await fs.readFile(localPath, 'utf-8'); const localLines = localContent.split('\n').length; - + // Fetch latest from registry try { const registryData = await fetchUrl(componentInfo.source); @@ -237,20 +237,42 @@ async function checkComponent(name, manifest) { // a difference — the local file has already been through this transform. const shadcnContent = rewriteRegistryImports(registryData.files?.[0]?.content || ''); const shadcnLines = shadcnContent.split('\n').length; - - // Simple heuristic: check if significantly different - const lineDiff = Math.abs(localLines - shadcnLines); - const isDifferent = lineDiff > 10 || localContent.includes('ObjectUI') || localContent.includes('data-slot'); - + + // `localOnlyLines` is directional — lines in the first argument that 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? + // + // The previous heuristic (`lineDiff > 10 || includes('ObjectUI')`) could + // not answer either. Every synced file carries the ObjectUI header this + // script prepends, so the second clause was true for all 46 of them and + // `--check` reported the entire set as "modified" — no signal at all. + const localOnly = localOnlyLines(localContent, shadcnContent); + const upstreamOnly = localOnlyLines(shadcnContent, localContent); + + let status; + let message; + if (localOnly.length > 0) { + // `--update` refuses these; the local lines would be deleted. + status = 'modified'; + message = `${localOnly.length} local line(s) upstream lacks — --update would refuse`; + if (upstreamOnly.length > 0) message += `, ${upstreamOnly.length} upstream line(s) pending`; + } else if (upstreamOnly.length > 0) { + status = 'outdated'; + message = `${upstreamOnly.length} upstream line(s) to pick up — safe to sync`; + } else { + status = 'synced'; + message = 'Identical to upstream'; + } + return { name, - status: isDifferent ? 'modified' : 'synced', + status, localLines, shadcnLines, - lineDiff, - message: isDifferent ? - `Modified (${lineDiff} lines difference)` : - 'Synced with Shadcn', + localOnly: localOnly.length, + upstreamOnly: upstreamOnly.length, + message, }; } catch (fetchError) { return { @@ -280,45 +302,59 @@ async function checkAllComponents() { const results = { synced: [], + outdated: [], modified: [], custom: [], error: [], }; - + for (const component of localComponents) { const result = await checkComponent(component, manifest); results[result.status].push(result); - + const symbol = { synced: '✓', + outdated: '↑', modified: '⚠', custom: '●', error: '✗', }[result.status]; - + const color = { synced: 'green', + outdated: 'cyan', modified: 'yellow', custom: 'blue', error: 'red', }[result.status]; - + log(`${symbol} ${result.name.padEnd(25)} ${result.message}`, color); } - + // Summary logSection('Summary'); - log(`✓ Synced: ${results.synced.length} components`, 'green'); - log(`⚠ Modified: ${results.modified.length} components`, 'yellow'); - log(`● Custom: ${results.custom.length} components`, 'blue'); - log(`✗ Errors: ${results.error.length} components`, 'red'); - + log(`✓ Identical: ${results.synced.length} components`, 'green'); + log(`↑ Outdated: ${results.outdated.length} components (no local edits — safe to sync)`, 'cyan'); + log(`⚠ Modified: ${results.modified.length} components (local edits — --update refuses)`, 'yellow'); + log(`● Custom: ${results.custom.length} components (not tracked upstream)`, 'blue'); + log(`✗ Errors: ${results.error.length} components`, 'red'); + + // The actionable bucket: upstream moved and nothing local is at risk. + if (results.outdated.length > 0) { + log('\nSafe to sync now:', 'cyan'); + log(` node scripts/shadcn-sync.js --update ${results.outdated[0].name}`, 'dim'); + if (results.outdated.length > 1) { + log(` (${results.outdated.length} in total: ${results.outdated.map((r) => r.name).join(', ')})`, 'dim'); + } + } + if (results.modified.length > 0) { - log('\nTo update a component:', 'cyan'); - log(' node scripts/shadcn-sync.js --update ', 'dim'); - log(' node scripts/shadcn-sync.js --update-all', 'dim'); + log('\nThese carry local edits a sync would delete:', 'yellow'); + log(` ${results.modified.map((r) => `${r.name}(${r.localOnly})`).join(', ')}`, 'dim'); + log(' Port the edits onto the new upstream version by hand, or --force to', 'dim'); + log(' take upstream as-is. `--diff ` shows both sides.', 'dim'); } - + return results; }