Skip to content

Commit 7613e12

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
fix: exempt shared media metadata and orphan component removals
1 parent bfdf8c0 commit 7613e12

12 files changed

Lines changed: 173 additions & 19 deletions

File tree

design-diff.config.json

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,10 @@
44
"(?:^|/)(?:node_modules|__tests__|__fixtures__|fixtures|test-results|\\.source|dist|build)/",
55
"\\.(?:test|spec)\\.[cm]?[jt]sx?$",
66
"(?:^|/)next-env\\.d\\.ts$",
7-
"(?:^|/)public/",
7+
"(?:^|/)public/(?!.*\\.(?:woff2?|ttf|otf|eot)$)",
88
"(?:^|/)sandbox/bundles/"
99
],
10+
"mediaModules": ["lucide-react", "react-icons", "next/image", "next/legacy/image"],
1011
"renderedMarkdown": ["apps/docs/content/", "apps/sim/content/"],
1112
"aliases": [
1213
{
@@ -158,7 +159,7 @@
158159
{
159160
"module": "@/components/ui/block-info-card",
160161
"names": ["BlockInfoCard"],
161-
"contentProps": ["type"]
162+
"contentProps": ["type", "color"]
162163
},
163164
{
164165
"module": "@/components/ui/command-table",

scripts/design-diff/README.md

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,16 @@ that every rendered pixel is unchanged.
3636
| Additional dropdown options, rows or controls repeating existing appearance | Exempt |
3737
| Copy, progress/error labels, pricing/privacy wording, documentation prose | Exempt |
3838
| Icons, images, screenshots, inline SVG and media element dimensions | Exempt |
39+
| Font files and font-face changes | Flag |
3940
| Runtime conditions, functional visibility, option data, unknown component props | Exempt |
4041
| Coordinates or translation alone | Exempt |
4142
| Unknown calls, parser/expression limits, plugins and dependency version changes alone | Exempt; retain coverage notes |
4243
| Comments, erased types, supported formatting/constant hoists/local renames | Exempt |
4344

45+
Deleting a shared control also exempts its standard appearance props. Deleted component modules
46+
with no resolved references outside other deleted files are treated as unestablished runtime use,
47+
not designer notifications. Route entry files and active component removals retain normal analysis.
48+
4449
A new `<Button variant="primary" size="sm"/>` uses shared appearance and is exempt.
4550
Adding `<Button className="rounded-none p-6"/>` introduces a custom override and flags.
4651
Changing an existing control's supported `variant`, `size`, classes or style values flags.
@@ -89,13 +94,14 @@ matching is approximate. Changes to unused authored styles and inactive variants
8994
runtime-only effects and unsupported rendering may be missed by this precision-oriented policy.
9095

9196
The scope covers product, landing pages, emails, documentation presentation, desktop and shared
92-
components/themes under `apps/` and `packages/`. Tests, fixtures, public assets and server sandbox
93-
bundles are excluded. Media component modules remain available to import resolution so excluding
97+
components/themes under `apps/` and `packages/`. Tests, fixtures, public image/media assets and server sandbox
98+
bundles are excluded. Font assets remain in scope. Recognized media import modules and resolved icon origins handle aliases.
99+
Media component modules remain available to import resolution so excluding
94100
an Icon does not break resolution of Button through the same barrel.
95101

96102
## Report contract
97103

98-
Schema **3.0.0**, engine **0.5.0**, policy **5.0.0**. The schema remains compatible; the policy
104+
Schema **3.0.0**, engine **0.5.1**, policy **5.0.0**. The schema remains compatible; the policy
99105
meaning changes. Readers must inspect versions when comparing historical qualification rates.
100106
All decisions and identifiers are deterministic for the same engine/configuration and commits.
101107
Execution timing and peak memory are recorded separately by the benchmark, never in engine JSON.

scripts/design-diff/analyze.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import type { Change, Config, Definition, Report } from '#design-diff/types'
2424
export function emptyReport(): Report {
2525
return {
2626
schemaVersion: '3.0.0',
27-
engineVersion: '0.5.0',
27+
engineVersion: '0.5.1',
2828
policyVersion: '5.0.0',
2929
commits: null,
3030
status: 'failed',
@@ -192,6 +192,7 @@ export async function analyze(
192192
}
193193
let extracted = 0
194194
let omittedConsumers = 0
195+
let orphanDeletions = 0
195196
/** Prefer direct consumers before distant opaque application plumbing when retaining one example. */
196197
const downstream = new Map<string, Set<string>>()
197198
for (const graph of [before.graph, after.graph])
@@ -221,6 +222,15 @@ export async function analyze(
221222
if (++extracted % 32 === 0) reclaimMemory()
222223
if (!scoped(file, config) && !infrastructure(file, config)) continue
223224
if ([...renames.values()].includes(file) && !after.entries.has(file)) continue
225+
if (
226+
!after.entries.has(file) &&
227+
file.includes('/components/') &&
228+
/\.[jt]sx$/.test(file) &&
229+
before.graph.usages(file).every((usage) => !after.entries.has(usage.location.file))
230+
) {
231+
orphanDeletions++
232+
continue
233+
}
224234
if (
225235
!changed.has(file) &&
226236
indirectExamples > 0 &&
@@ -283,6 +293,10 @@ export async function analyze(
283293
...report.limitations,
284294
`Indirect analysis omitted for ${omittedConsumers} unchanged files after the PR qualified and a nearby rendering consumer was examined. All in-scope changed files were analyzed; additional indirect effects are not exhaustively catalogued. Categories describe retained evidence; usage counts remain partial resolved references.`,
285295
]
296+
if (orphanDeletions)
297+
report.limitations.push(
298+
`${orphanDeletions} deleted component modules had no resolved static references outside deleted files. Runtime use is unestablished; these removals do not qualify on their own.`
299+
)
286300
for (const file of changed) {
287301
if (file !== 'bun.lock' && !file.endsWith('/package.json') && file !== 'package.json')
288302
continue

scripts/design-diff/appearance.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,8 @@ function classes(value: Data): Data | undefined {
148148
/** The notification policy requires concrete authored appearance, independent of render guards. */
149149
export function appearanceValue(definition: Definition): Data | undefined {
150150
if (definition.appearance?.media) return undefined
151+
if (definition.kind === 'asset' && /\.(?:woff2?|ttf|otf|eot)$/i.test(definition.location.file))
152+
return definition.value
151153
if (definition.kind === 'class') {
152154
if (!object(definition.value) || !Array.isArray(definition.value.normalized)) return undefined
153155
const values = definition.value.normalized.flatMap((entry) => {
@@ -164,7 +166,13 @@ export function appearanceValue(definition: Definition): Data | undefined {
164166
if (!object(definition.value)) return undefined
165167
const { order: _order, ...value } = definition.value
166168
if (typeof value.value === 'string' && /url\(/.test(value.value))
167-
value.value = cssAppearance(value.value)
169+
value.value =
170+
definition.property === 'src' &&
171+
definition.conditions.some(
172+
(condition) => typeof condition === 'string' && condition.startsWith('@font-face')
173+
)
174+
? value.value
175+
: cssAppearance(value.value)
168176
return { ...value, context: definition.conditions }
169177
}
170178
if (

scripts/design-diff/benchmark.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ export async function benchmark(args = process.argv.slice(2)): Promise<void> {
205205
result.reportSha256 = hash(bytes)
206206
if (
207207
report.schemaVersion !== '3.0.0' ||
208-
report.engineVersion !== '0.5.0' ||
208+
report.engineVersion !== '0.5.1' ||
209209
report.policyVersion !== '5.0.0'
210210
)
211211
throw new Error('Report version mismatch')

scripts/design-diff/compare.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -137,15 +137,18 @@ export function compareDefinitions(
137137
pairs.push([removals[index], additions[index]])
138138
}
139139
for (const [a, b] of pairs) {
140-
if (!a && b?.kind === 'attribute' && b.appearance?.shared) {
141-
const element = b.appearance.element?.replace(/:\d+$/, '')
140+
const unpaired = !a ? b : !b ? a : undefined
141+
if (unpaired?.kind === 'attribute' && unpaired.appearance?.shared) {
142+
const element = unpaired.appearance.element?.replace(/:\d+$/, '')
142143
const count = (definitions: Definition[]) =>
143144
definitions.filter(
144145
(definition) =>
145146
definition.kind === 'markup' &&
146147
definition.appearance?.element?.replace(/:\d+$/, '') === element
147148
).length
148-
if (!count(before) || count(after) > count(before)) continue
149+
const existing = !a ? before : after
150+
const added = !a ? after : before
151+
if (!count(existing) || count(added) > count(existing)) continue
149152
}
150153
if (a && b && (signature(a) === signature(b) || sameAppearance(a, b))) continue
151154
if ((!a || appearance(a) === undefined) && (!b || appearance(b) === undefined)) continue

scripts/design-diff/extract/documents.ts

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import remarkMdx from 'remark-mdx'
55
import remarkParse from 'remark-parse'
66
import { unified } from 'unified'
77
import { appearanceAttributes, mediaElement } from '#design-diff/appearance'
8-
import { canonical, semanticSource } from '#design-diff/ast'
8+
import { canonical, parseSource, propertyName, semanticSource } from '#design-diff/ast'
99
import { documentPresentation } from '#design-diff/document-content'
1010
import { cssValue, extractCss } from '#design-diff/extract/css'
1111
import type { Resolver } from '#design-diff/resolve'
@@ -134,8 +134,46 @@ export function extractDocument(
134134
.map((node) => ('value' in node ? String(node.value) : ''))
135135
.join('\n')
136136
if (appearanceOnly) {
137+
const bindings = new Map<string, { module: string; name: string }>()
138+
for (const statement of parseSource(imports, `${file}.tsx`).program.body) {
139+
if (statement.type !== 'ImportDeclaration') continue
140+
for (const specifier of statement.specifiers)
141+
bindings.set(specifier.local.name, {
142+
module: statement.source.value,
143+
name:
144+
specifier.type === 'ImportSpecifier'
145+
? propertyName(specifier.imported)
146+
: specifier.type === 'ImportNamespaceSpecifier'
147+
? '*'
148+
: 'default',
149+
})
150+
}
137151
const walk = (node: Record<string, Data>) => {
138152
if (typeof node.name === 'string' && mediaElement.test(node.name)) return
153+
const imported =
154+
typeof node.name === 'string' ? bindings.get(node.name.split('.')[0]) : undefined
155+
if (
156+
imported &&
157+
resolver?.tree.config.mediaModules?.some(
158+
(module) => imported.module === module || imported.module.startsWith(`${module}/`)
159+
)
160+
)
161+
return
162+
const content =
163+
imported &&
164+
resolver?.tree.config.documentationContent?.components.find(
165+
(component) =>
166+
component.module === imported.module &&
167+
component.names.includes(
168+
imported.name === '*' && typeof node.name === 'string'
169+
? node.name.split('.')[1]
170+
: imported.name
171+
)
172+
)
173+
if (typeof node.name === 'string') {
174+
emit('markup', null, 1, 1, node.name)
175+
result[result.length - 1].appearance = { element: node.name }
176+
}
139177
if (Array.isArray(node.attributes)) {
140178
const position = node.position as
141179
| { start?: { line?: number; column?: number } }
@@ -146,6 +184,7 @@ export function extractDocument(
146184
typeof item !== 'object' ||
147185
Array.isArray(item) ||
148186
typeof item.name !== 'string' ||
187+
(content && content.contentProps.includes(item.name)) ||
149188
!appearanceAttributes.test(item.name)
150189
)
151190
continue

scripts/design-diff/extract/tsx.ts

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,25 @@ export function extractTsx(resolver: Resolver, file: string, appearanceOnly = fa
4141
if (mediaElement.test(name)) return true
4242
const binding = node.scope.getBinding(name.split('.')[0])
4343
const declaration = binding?.path.parentPath
44-
if (
45-
declaration?.isImportDeclaration() &&
46-
/(?:^|\/)icons?(?:\/|$)/.test(declaration.node.source.value)
47-
)
48-
return true
44+
if (declaration?.isImportDeclaration()) {
45+
const module = declaration.node.source.value
46+
if (
47+
/(?:^|\/)icons?(?:\/|$)/.test(module) ||
48+
resolver.tree.config.mediaModules?.some(
49+
(prefix) => module === prefix || module.startsWith(`${prefix}/`)
50+
)
51+
)
52+
return true
53+
const imported = binding?.path.isImportSpecifier()
54+
? propertyName(binding.path.node.imported)
55+
: 'default'
56+
const origin = resolver.tree.graph?.imported(file, module, imported)
57+
if (
58+
origin?.origins.length &&
59+
origin.origins.every((item) => /(?:^|\/)icons?(?:\/|\.[cm]?[jt]sx?$)/.test(item.file))
60+
)
61+
return true
62+
}
4963
}
5064
return false
5165
}

scripts/design-diff/policy.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,15 @@ export function category(definition: Definition): Category {
55
const property = definition.property.replace(/[A-Z]/g, (letter) => `-${letter.toLowerCase()}`)
66
if (definition.property === 'infrastructure') return 'infrastructure'
77
if (definition.kind === 'review') return 'unresolved'
8+
if (
9+
(definition.kind === 'asset' && /\.(?:woff2?|ttf|otf|eot)$/i.test(definition.location.file)) ||
10+
(definition.kind === 'css' &&
11+
definition.property === 'src' &&
12+
definition.conditions.some(
13+
(condition) => typeof condition === 'string' && condition.startsWith('@font-face')
14+
))
15+
)
16+
return 'typography'
817
if (definition.kind === 'asset' || definition.kind === 'content' || definition.kind === 'markup')
918
return 'content'
1019
if (/^(?:src|src-set|alt|title|placeholder|d|points|view-box)$/.test(property)) return 'content'

scripts/design-diff/tests/appearance-policy.test.ts

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,4 +111,63 @@ describe('designer notification policy', () => {
111111
(await compareFiles({ [shared]: source('red') }, { [shared]: source('blue') })).flagged
112112
).toBe(true)
113113
})
114+
115+
it.each([
116+
[
117+
'lucide-react',
118+
'export const Page=()=> <Glyph className="size-4"/>',
119+
'export const Page=()=> <Glyph className="size-8"/>',
120+
],
121+
[
122+
'next/image',
123+
'export const Page=()=> <Glyph width={100}/>',
124+
'export const Page=()=> <Glyph width={200}/>',
125+
],
126+
])('recognizes aliased media imports from %s', async (module, a, b) => {
127+
const prefix = `import Glyph from '${module}';`
128+
expect((await compareFiles({ [file]: prefix + a }, { [file]: prefix + b })).flagged).toBe(false)
129+
})
130+
131+
it('exempts deleting a standard branded documentation card', async () => {
132+
const doc = 'apps/docs/content/evernote.mdx'
133+
const source =
134+
'import {BlockInfoCard} from "@/components/ui/block-info-card"\n\n<BlockInfoCard type="evernote" color="#FFFFFF"/>'
135+
expect((await compareFiles({ [doc]: source }, { [doc]: null })).flagged).toBe(false)
136+
expect(
137+
(await compareFiles({ [doc]: source }, { [doc]: source.replace('#FFFFFF', '#FF0000') }))
138+
.flagged
139+
).toBe(false)
140+
})
141+
142+
it('flags an appearance override added to an existing MDX component', async () => {
143+
const doc = 'apps/docs/content/page.mdx'
144+
expect(
145+
(await compareFiles({ [doc]: '<Callout/>' }, { [doc]: '<Callout variant="compact"/>' }))
146+
.flagged
147+
).toBe(true)
148+
})
149+
150+
it('exempts deleted orphan component clusters but retains active component removals', async () => {
151+
const a = 'apps/sim/components/unused.tsx'
152+
const b = 'apps/sim/components/unused-wrapper.tsx'
153+
const files = {
154+
[a]: 'export const Unused=()=> <div className="p-2"/>',
155+
[b]: 'import {Unused} from "./unused";export const Wrapper=()=> <Unused/>',
156+
}
157+
expect((await compareFiles(files, { [a]: null, [b]: null })).flagged).toBe(false)
158+
expect((await compareFiles(files, { [a]: null })).flagged).toBe(true)
159+
})
160+
161+
it('retains font replacements while ignoring binary image replacements', async () => {
162+
const font = 'apps/sim/public/brand/font.woff2'
163+
const image = 'apps/sim/public/brand/cover.png'
164+
expect(
165+
(await compareFiles({ [font]: Buffer.from([0, 1, 2]) }, { [font]: Buffer.from([0, 1, 3]) }))
166+
.flagged
167+
).toBe(true)
168+
expect(
169+
(await compareFiles({ [image]: Buffer.from([0, 1, 2]) }, { [image]: Buffer.from([0, 1, 3]) }))
170+
.flagged
171+
).toBe(false)
172+
})
114173
})

0 commit comments

Comments
 (0)