Skip to content

Commit bfdf8c0

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
fix: restrict design notifications to authored appearance changes
1 parent 99e7228 commit bfdf8c0

29 files changed

Lines changed: 1016 additions & 591 deletions

design-diff.config.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
"exclude": [
44
"(?:^|/)(?:node_modules|__tests__|__fixtures__|fixtures|test-results|\\.source|dist|build)/",
55
"\\.(?:test|spec)\\.[cm]?[jt]sx?$",
6-
"(?:^|/)next-env\\.d\\.ts$"
6+
"(?:^|/)next-env\\.d\\.ts$",
7+
"(?:^|/)public/",
8+
"(?:^|/)sandbox/bundles/"
79
],
810
"renderedMarkdown": ["apps/docs/content/", "apps/sim/content/"],
911
"aliases": [

scripts/design-diff/README.md

Lines changed: 164 additions & 396 deletions
Large diffs are not rendered by default.

scripts/design-diff/analyze.ts

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ 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.4.0',
28-
policyVersion: '4.0.0',
27+
engineVersion: '0.5.0',
28+
policyVersion: '5.0.0',
2929
commits: null,
3030
status: 'failed',
3131
flagged: null,
@@ -155,7 +155,7 @@ export async function analyze(
155155
}
156156
try {
157157
if (scriptPattern.test(file)) {
158-
const defs = extractTsx(resolver, file)
158+
const defs = extractTsx(resolver, file, true)
159159
if (config.nativeRendering.includes(file))
160160
defs.push(
161161
review(
@@ -182,7 +182,7 @@ export async function analyze(
182182
/\.html?$/.test(file) ||
183183
(/\.mdx?$/.test(file) && config.renderedMarkdown.some((root) => file.startsWith(root)))
184184
)
185-
return normalizeAll(extractDocument(source, file, resolver))
185+
return normalizeAll(extractDocument(source, file, resolver, true))
186186
if (/\.(?:scss|sass|less|vue|svelte)$/.test(file))
187187
return [review(file, entry.oid, 'Unsupported rendering syntax')]
188188
} catch {
@@ -232,6 +232,13 @@ export async function analyze(
232232
const oldFile = renames.get(file) ?? file
233233
const a = await extract(before, previousTailwind, oldFile)
234234
const b = await extract(after, nextTailwind, file)
235+
report.limitations = [
236+
...new Set([
237+
...report.limitations,
238+
...a.flatMap((definition) => definition.unresolved),
239+
...b.flatMap((definition) => definition.unresolved),
240+
]),
241+
].sort()
235242
findings.push(...compareDefinitions(a, b, affected))
236243
if (!changed.has(file) && (a.length || b.length)) indirectExamples++
237244
if (

scripts/design-diff/appearance.ts

Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,189 @@
1+
import { canonicalJson } from '#design-diff/ast'
2+
import type { Data, Definition } from '#design-diff/types'
3+
4+
export const appearanceAttributes =
5+
/^(?:className|.*ClassName|class|style|width|height|minWidth|minHeight|maxWidth|maxHeight|size|rows|cols|color|backgroundColor|opacity|variant|orientation|animate|initial|exit|transition|whileHover|whileTap)$/
6+
export const mediaElement =
7+
/^(?:svg|img|image|picture|video|audio|source|canvas|Image|Video|Icon|.*Icon)$/
8+
9+
function object(value: Data | undefined): value is Record<string, Data> {
10+
return !!value && typeof value === 'object' && !Array.isArray(value)
11+
}
12+
13+
/** Ignore asset identity and generated copy while preserving gradients and other CSS effects. */
14+
function cssAppearance(value: string): string {
15+
return value.replace(/url\([^)]*\)/g, 'url(asset)')
16+
}
17+
18+
/** Retain supported style values, never arbitrary captured inputs inside opaque expressions. */
19+
function literal(value: Data, depth = 0, args?: Data[]): Data | undefined {
20+
if (depth > 32) return undefined
21+
if (value === null || typeof value !== 'object') return value
22+
if (Array.isArray(value)) {
23+
const values = value.map((item) => literal(item, depth + 1))
24+
return values.every((item) => item !== undefined) ? (values as Data[]) : undefined
25+
}
26+
if ('$await' in value) return literal(value.$await, depth + 1, args)
27+
if (Array.isArray(value.$parameter) && args) {
28+
let selected: Data = args
29+
for (const key of value.$parameter) {
30+
if (typeof key !== 'number' && typeof key !== 'string') return undefined
31+
if (Array.isArray(selected) && typeof key === 'number') selected = selected[key]
32+
else if (object(selected)) selected = selected[String(key)]
33+
else return undefined
34+
if (selected === undefined) return undefined
35+
}
36+
return literal(selected, depth + 1)
37+
}
38+
if ('$default' in value) {
39+
const input = literal(value.input, depth + 1, args)
40+
const fallback = literal(value.$default, depth + 1, args)
41+
return fallback === undefined ? undefined : { input: input ?? null, fallback }
42+
}
43+
if ('$selectedCall' in value && object(value.$selectedCall))
44+
return literal(
45+
value.$selectedCall,
46+
depth + 1,
47+
Array.isArray(value.arguments) ? value.arguments : args
48+
)
49+
if (object(value.$call) && Array.isArray(value.$call.$function))
50+
return literal(value.$call, depth + 1, Array.isArray(value.arguments) ? value.arguments : args)
51+
if ('$reactState' in value && Array.isArray(value.updates)) {
52+
const initial = literal(value.$reactState, depth + 1)
53+
const updates = value.updates.flatMap((entry) => {
54+
if (!object(entry)) return []
55+
const result = literal(entry.value, depth + 1)
56+
return result === undefined ? [] : [result]
57+
})
58+
return initial !== undefined || updates.length
59+
? { initial: initial ?? null, updates }
60+
: undefined
61+
}
62+
if (Array.isArray(value.$function)) {
63+
const values = value.$function.map((item) =>
64+
object(item) && 'value' in item ? literal(item.value, depth + 1, args) : undefined
65+
)
66+
return values.length && values.every((item) => item !== undefined)
67+
? { returns: values as Data[] }
68+
: undefined
69+
}
70+
if ('$condition' in value) {
71+
const a = literal(value.then, depth + 1)
72+
const b = literal(value.else, depth + 1)
73+
return a !== undefined && b !== undefined ? { branches: [a, b] } : undefined
74+
}
75+
if (Object.keys(value).some((key) => key.startsWith('$'))) return undefined
76+
const result: Record<string, Data> = {}
77+
for (const [key, item] of Object.entries(value)) {
78+
const supported = literal(item, depth + 1)
79+
if (supported === undefined) return undefined
80+
result[key] = supported
81+
}
82+
return result
83+
}
84+
85+
/** Generated CSS is evidence; helper fingerprints, runtime predicates and diagnostics are not. */
86+
function classes(value: Data): Data | undefined {
87+
if (value === null || value === false) return null
88+
if (Array.isArray(value)) {
89+
const items = value.map(classes).filter((item): item is Data => item !== undefined)
90+
return items.length ? items : undefined
91+
}
92+
if (!object(value)) return undefined
93+
if (Array.isArray(value.css) && Array.isArray(value.order)) {
94+
const css = value.css
95+
.filter((item): item is string => typeof item === 'string')
96+
.map((item) => {
97+
const declarations = JSON.parse(item) as [string[], string, string, boolean][]
98+
return JSON.stringify(
99+
declarations
100+
.filter(([, property]) => property !== 'content')
101+
.map(([conditions, property, data, important]) => [
102+
conditions.map((condition) => (condition.startsWith('.') ? '.utility' : condition)),
103+
property,
104+
cssAppearance(data),
105+
important,
106+
])
107+
)
108+
})
109+
if (!css.length && value.order.length) return undefined
110+
const variables = Array.isArray(value.variables)
111+
? value.variables.map((item) => {
112+
if (!object(item) || !object(item.value)) return item
113+
const { order: _order, ...data } = item.value
114+
return { ...item, value: data }
115+
})
116+
: []
117+
return { css, variables }
118+
}
119+
if ('$classes' in value) return classes(value.$classes)
120+
if (Array.isArray(value.$cva)) {
121+
const [base, options] = value.$cva
122+
const variants: Record<string, Data> = {}
123+
if (object(options) && object(options.variants))
124+
for (const [name, choices] of Object.entries(options.variants)) {
125+
const supported: Record<string, Data> = {}
126+
if (object(choices))
127+
for (const [choice, definition] of Object.entries(choices)) {
128+
const result = classes(definition)
129+
if (result !== undefined) supported[choice] = result
130+
}
131+
variants[name] = supported
132+
}
133+
return {
134+
base: classes(base) ?? null,
135+
variants,
136+
defaults: object(options) ? (literal(options.defaultVariants ?? null) ?? null) : null,
137+
compoundVariants: object(options)
138+
? (literal(options.compoundVariants ?? null) ?? null)
139+
: null,
140+
}
141+
}
142+
if ('$variant' in value) return classes(value.$variant)
143+
if ('$condition' in value) return classes([value.then ?? null, value.else ?? null])
144+
if (value.$operator === '&&') return classes(value.right)
145+
return undefined
146+
}
147+
148+
/** The notification policy requires concrete authored appearance, independent of render guards. */
149+
export function appearanceValue(definition: Definition): Data | undefined {
150+
if (definition.appearance?.media) return undefined
151+
if (definition.kind === 'class') {
152+
if (!object(definition.value) || !Array.isArray(definition.value.normalized)) return undefined
153+
const values = definition.value.normalized.flatMap((entry) => {
154+
if (!object(entry)) return []
155+
const value = classes(entry.value)
156+
return value === undefined ? [] : [value]
157+
})
158+
return values.length ? values : undefined
159+
}
160+
if (definition.kind === 'css') {
161+
if (definition.property.startsWith('@')) return undefined
162+
if (definition.property === 'content') return undefined
163+
if (typeof definition.value === 'string') return definition.value
164+
if (!object(definition.value)) return undefined
165+
const { order: _order, ...value } = definition.value
166+
if (typeof value.value === 'string' && /url\(/.test(value.value))
167+
value.value = cssAppearance(value.value)
168+
return { ...value, context: definition.conditions }
169+
}
170+
if (
171+
!['style', 'native'].includes(definition.kind) &&
172+
!(definition.kind === 'attribute' && appearanceAttributes.test(definition.property))
173+
)
174+
return undefined
175+
if (/^(?:icon|setIcon|setImage|trafficLightPosition|setPosition)$/.test(definition.property))
176+
return undefined
177+
if (definition.property === 'content') return undefined
178+
if (object(definition.value) && Array.isArray(definition.value.normalized)) {
179+
const source = literal(definition.value.source)
180+
if (source === undefined) return undefined
181+
return { source, normalized: canonicalJson(definition.value.normalized) }
182+
}
183+
const value = literal(definition.value)
184+
return typeof value === 'string' ? cssAppearance(value) : value
185+
}
186+
187+
export function sameAppearance(a: Definition, b: Definition): boolean {
188+
return JSON.stringify(appearanceValue(a)) === JSON.stringify(appearanceValue(b))
189+
}

scripts/design-diff/benchmark.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -205,8 +205,8 @@ 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.4.0' ||
209-
report.policyVersion !== '4.0.0'
208+
report.engineVersion !== '0.5.0' ||
209+
report.policyVersion !== '5.0.0'
210210
)
211211
throw new Error('Report version mismatch')
212212
if (JSON.stringify(report.commits) !== JSON.stringify(commits))

scripts/design-diff/compare.ts

Lines changed: 97 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { createHash } from 'node:crypto'
2+
import { appearanceValue } from '#design-diff/appearance'
23
import { pureMovement } from '#design-diff/movement'
34
import { changedCategory } from '#design-diff/policy'
45
import { previewChange } from '#design-diff/report'
@@ -24,11 +25,23 @@ export function finding(
2425
unresolved.length ||
2526
definition.kind === 'review' ||
2627
['movement', 'unresolved'].includes(category)
28+
const a = before && appearanceValue(before)
29+
const b = after && appearanceValue(after)
30+
const supported =
31+
(!before || a !== undefined) &&
32+
(!after || b !== undefined) &&
33+
(a !== undefined || b !== undefined)
34+
const changed = JSON.stringify(a) !== JSON.stringify(b)
35+
const flag = supported && changed && !movement && category !== 'movement'
2736
const result: Omit<Change, 'id'> = {
28-
decision: movement ? 'exempt' : 'flag',
37+
decision: flag ? 'flag' : 'exempt',
2938
category: movement ? 'movement' : category,
3039
reason:
31-
reason ??
40+
(flag
41+
? 'Supported authored appearance values changed'
42+
: !movement
43+
? 'No established change to authored appearance under the designer policy'
44+
: reason) ??
3245
(movement
3346
? 'Static geometry establishes movement within unchanged bounds'
3447
: uncertain
@@ -76,14 +89,90 @@ export function compareDefinitions(
7689
after: Definition[],
7790
_changed?: Set<string>
7891
): Change[] {
79-
const previous = new Map(before.map((definition) => [definition.key, definition]))
80-
const next = new Map(after.map((definition) => [definition.key, definition]))
92+
const signatures = new Map<Definition, string | undefined>()
93+
const appearance = (definition: Definition) => {
94+
if (!signatures.has(definition))
95+
signatures.set(definition, JSON.stringify(appearanceValue(definition)))
96+
return signatures.get(definition)
97+
}
98+
const sameAppearance = (a: Definition, b: Definition) => appearance(a) === appearance(b)
99+
if (
100+
[...before, ...after].some(
101+
(definition) =>
102+
definition.kind === 'review' &&
103+
definition.unresolved.some((reason) => /Parser failure|extraction failed/.test(reason))
104+
)
105+
)
106+
return []
107+
const groups = (definitions: Definition[]) => {
108+
const result = new Map<string, Definition[]>()
109+
for (const definition of definitions) {
110+
const key = definition.key.replace(/:\d+$/, '')
111+
const entries = result.get(key) ?? []
112+
entries.push(definition)
113+
result.set(key, entries)
114+
}
115+
return result
116+
}
117+
const previous = groups(before)
118+
const next = groups(after)
81119
const result: Change[] = []
82120
for (const key of [...new Set([...previous.keys(), ...next.keys()])].sort()) {
83-
const a = previous.get(key)
84-
const b = next.get(key)
85-
if (a && b && signature(a) === signature(b)) continue
86-
result.push(finding(a, b))
121+
const left = previous.get(key) ?? []
122+
const right = next.get(key) ?? []
123+
const pairs: [Definition | undefined, Definition | undefined][] = []
124+
if (left.length === right.length) {
125+
left.forEach((definition, index) => pairs.push([definition, right[index]]))
126+
} else {
127+
/** Inserting repeated controls must not turn later unchanged definitions into edits. */
128+
const remaining = new Set(left)
129+
const additions: Definition[] = []
130+
for (const definition of right) {
131+
const match = [...remaining].find((candidate) => sameAppearance(candidate, definition))
132+
if (match) remaining.delete(match)
133+
else additions.push(definition)
134+
}
135+
const removals = [...remaining]
136+
for (let index = 0; index < Math.max(removals.length, additions.length); index++)
137+
pairs.push([removals[index], additions[index]])
138+
}
139+
for (const [a, b] of pairs) {
140+
if (!a && b?.kind === 'attribute' && b.appearance?.shared) {
141+
const element = b.appearance.element?.replace(/:\d+$/, '')
142+
const count = (definitions: Definition[]) =>
143+
definitions.filter(
144+
(definition) =>
145+
definition.kind === 'markup' &&
146+
definition.appearance?.element?.replace(/:\d+$/, '') === element
147+
).length
148+
if (!count(before) || count(after) > count(before)) continue
149+
}
150+
if (a && b && (signature(a) === signature(b) || sameAppearance(a, b))) continue
151+
if ((!a || appearance(a) === undefined) && (!b || appearance(b) === undefined)) continue
152+
if (
153+
!a &&
154+
b &&
155+
before.some(
156+
(definition) =>
157+
definition.kind === b.kind &&
158+
definition.property === b.property &&
159+
sameAppearance(definition, b)
160+
)
161+
)
162+
continue
163+
if (
164+
a &&
165+
!b &&
166+
after.some(
167+
(definition) =>
168+
definition.kind === a.kind &&
169+
definition.property === a.property &&
170+
sameAppearance(definition, a)
171+
)
172+
)
173+
continue
174+
result.push(finding(a, b))
175+
}
87176
}
88177
return result
89178
}

0 commit comments

Comments
 (0)