Skip to content

Commit 24ba7a2

Browse files
Bill LeoutsakosBill Leoutsakos
Bill Leoutsakos
authored and
Bill Leoutsakos
committed
fix React numeric style unit serialization
1 parent 96301e3 commit 24ba7a2

3 files changed

Lines changed: 132 additions & 22 deletions

File tree

‎scripts/check-design-conformance-command.test.ts‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'nod
33
import os from 'node:os'
44
import path from 'node:path'
55
import { fileURLToPath } from 'node:url'
6+
import { createElement } from 'react'
7+
import { renderToStaticMarkup } from 'react-dom/server'
68
import { afterAll, expect, test } from 'vitest'
79
import { findingFingerprint } from '#control-analysis/review-ledger'
810
import { ciRefs, warningExitCode } from '#design-conformance/ci'
@@ -307,6 +309,75 @@ throw new Error('Product source must not execute')
307309
}
308310
)
309311

312+
test.each([
313+
['grid line integer', 'gridColumnStart', 'grid-column-start', '1', 1, ''],
314+
['grid row integer', 'gridRowEnd', 'grid-row-end', '3', 3, ''],
315+
['animation count', 'animationIterationCount', 'animation-iteration-count', '2', 2, ''],
316+
['border image ratio', 'borderImageSlice', 'border-image-slice', '2', 2, ''],
317+
['vendor line clamp', 'WebkitLineClamp', '-webkit-line-clamp', '2', 2, ''],
318+
['font weight', 'fontWeight', 'font-weight', '600', 600, ''],
319+
['ordinary dimension', 'width', 'width', '1', 1, ''],
320+
['numeric string dimension', 'width', 'width', "'1'", '1', ''],
321+
['numeric alias dimension', 'height', 'height', 'size', 4, 'const size=4;'],
322+
['string member dimension', 'height', 'height', 'sizes.small', '4', "const sizes={small:'4'};"],
323+
['typed grid alias', 'gridColumnEnd', 'grid-column-end', 'line', 2, 'const line=2 as const;'],
324+
['numeric custom variable', '--reserve', '--reserve', '1', 1, ''],
325+
] as const)(
326+
'React numeric style serialization survives the real CLI: %s',
327+
(_name, property, cssProperty, value, runtimeValue, prefix) => {
328+
const { repo } = fixture()
329+
writeFileSync(path.join(repo, ui), 'const A=()=> <button/>')
330+
const base = commit(repo)
331+
writeFileSync(
332+
path.join(repo, ui),
333+
`${prefix}const A=()=> <button style={{'${property}':${value}}}/>`
334+
)
335+
const result = run([
336+
'--repo',
337+
repo,
338+
'--base',
339+
base,
340+
'--working-tree',
341+
'--policy',
342+
'appearance',
343+
'--format',
344+
'json',
345+
])
346+
expect(result.status, result.stderr).toBe(1)
347+
const report = JSON.parse(result.stdout) as Report
348+
expect(report.status).toBe('completed')
349+
const rendered = renderToStaticMarkup(
350+
createElement('button', { style: { [property]: runtimeValue } })
351+
)
352+
const reactValue = rendered.match(/style="[^:"]+:([^"]*)"/)?.[1]
353+
expect(reactValue).toBeTypeOf('string')
354+
expect(report.findings.find((finding) => finding.property === cssProperty)?.value).toBe(
355+
JSON.stringify([reactValue])
356+
)
357+
}
358+
)
359+
360+
test('central numeric-looking style references keep their scalar type ambiguity explicit through the real CLI', () => {
361+
const { repo } = fixture()
362+
mkdirSync(path.join(repo, 'packages/emcn/src'), { recursive: true })
363+
writeFileSync(path.join(repo, 'packages/emcn/src/index.ts'), 'export const dimension=3')
364+
writeFileSync(path.join(repo, ui), 'const A=()=> <button/>')
365+
const base = commit(repo)
366+
writeFileSync(
367+
path.join(repo, ui),
368+
"import {dimension} from '@sim/emcn';const A=()=> <button style={{width:dimension}}/>"
369+
)
370+
const result = run(['--repo', repo, '--base', base, '--working-tree', '--format', 'json'])
371+
expect([0, 1], result.stderr).toContain(result.status)
372+
const report = JSON.parse(result.stdout) as Report
373+
expect(report.status).toBe('completed')
374+
expect(
375+
report.unchecked.some((note) =>
376+
note.reason.includes('Central numeric-looking style value has unknown scalar type')
377+
)
378+
).toBe(true)
379+
})
380+
310381
test('working-tree mode includes a changed central contract registry', () => {
311382
const { repo } = fixture()
312383
const file = 'scripts/design-conformance/contracts.json'

‎scripts/design-conformance/extract.ts‎

Lines changed: 43 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import {
2424
excludedCssProperty,
2525
normalizeValue,
2626
} from '#design-conformance/normalize'
27+
import { reactStyleValue, reactUnitlessProperty } from '#design-conformance/react-style'
2728
import {
2829
htmlArtwork,
2930
htmlSink,
@@ -611,35 +612,43 @@ export function extract(
611612
reference
612613
)
613614
}
614-
const literal = (p: NodePath, active = new Set<t.Node>(), depth = 0): string | undefined => {
615+
const scalarLiteral = (
616+
p: NodePath,
617+
active = new Set<t.Node>(),
618+
depth = 0
619+
): string | number | undefined => {
615620
if (!p.node || depth > 12 || active.has(p.node)) return undefined
616621
const next = new Set(active).add(p.node)
617-
if (p.isStringLiteral() || p.isNumericLiteral()) return String(p.node.value)
622+
if (p.isStringLiteral() || p.isNumericLiteral()) return p.node.value
618623
if (p.isUnaryExpression({ operator: '-' })) {
619-
const x = literal(p.get('argument'), next, depth + 1)
620-
return x !== undefined ? `-${x}` : undefined
624+
const x = scalarLiteral(p.get('argument'), next, depth + 1)
625+
return typeof x === 'number' ? -x : undefined
621626
}
622627
if (p.isTSAsExpression() || p.isTSNonNullExpression() || p.isTSSatisfiesExpression())
623-
return literal(p.get('expression') as NodePath, next, depth + 1)
628+
return scalarLiteral(p.get('expression') as NodePath, next, depth + 1)
624629
if (p.isIdentifier()) {
625630
const b = p.scope.getBinding(p.node.name)
626631
if (b?.constant && b.path.isVariableDeclarator())
627-
return literal(b.path.get('init') as NodePath, next, depth + 1)
632+
return scalarLiteral(b.path.get('init') as NodePath, next, depth + 1)
628633
}
629634
if (p.isTemplateLiteral()) {
630635
let out = p.node.quasis[0].value.cooked ?? p.node.quasis[0].value.raw
631636
for (const [i, e] of (p.get('expressions') as NodePath[]).entries()) {
632-
const v = literal(e, next, depth + 1)
637+
const v = scalarLiteral(e, next, depth + 1)
633638
if (v === undefined) return undefined
634639
out += v + (p.node.quasis[i + 1].value.cooked ?? p.node.quasis[i + 1].value.raw)
635640
}
636641
return out
637642
}
638643
if (p.isBinaryExpression({ operator: '+' })) {
639-
const a = literal(p.get('left'), next, depth + 1)
640-
const b = literal(p.get('right'), next, depth + 1)
641-
if (a !== undefined && b !== undefined && !(/^[-\d.]+$/.test(a) && /^[-\d.]+$/.test(b)))
642-
return a + b
644+
const a = scalarLiteral(p.get('left'), next, depth + 1)
645+
const b = scalarLiteral(p.get('right'), next, depth + 1)
646+
if (
647+
a !== undefined &&
648+
b !== undefined &&
649+
!(/^[-\d.]+$/.test(String(a)) && /^[-\d.]+$/.test(String(b)))
650+
)
651+
return String(a) + String(b)
643652
}
644653
if (p.isMemberExpression() && !p.node.computed && p.get('object').isIdentifier()) {
645654
const b = p.scope.getBinding((p.node.object as t.Identifier).name)
@@ -670,12 +679,16 @@ export function extract(
670679
!x.node.computed &&
671680
name(x.get('key')) === name(p.get('property') as NodePath)
672681
)
673-
if (prop?.isObjectProperty()) return literal(prop.get('value'), next, depth + 1)
682+
if (prop?.isObjectProperty()) return scalarLiteral(prop.get('value'), next, depth + 1)
674683
}
675684
}
676685
}
677686
return undefined
678687
}
688+
const literal = (p: NodePath): string | undefined => {
689+
const value = scalarLiteral(p)
690+
return value === undefined ? undefined : String(value)
691+
}
679692
const classes = (p: NodePath, active = new Set<t.Node>(), depth = 0, context = ''): void => {
680693
if (!p.node) return
681694
if (depth > 12 || active.has(p.node)) {
@@ -879,10 +892,12 @@ export function extract(
879892
note(prop, 'Style spread/computed property is unchecked', 'style')
880893
continue
881894
}
882-
const key = kebab(name(prop.get('key')))
895+
const property = name(prop.get('key'))
896+
const key = kebab(property)
883897
const v = prop.get('value')
884898
const central = options?.conformance ? centralValue(v) : undefined
885-
const value = central?.value ?? literal(v)
899+
const scalar = scalarLiteral(v)
900+
const value = central?.value ?? (scalar === undefined ? undefined : String(scalar))
886901
if (value === undefined) {
887902
const ref =
888903
options?.conformance && v.isIdentifier() ? imported(v, v.node.name) : undefined
@@ -909,18 +924,24 @@ export function extract(
909924
note(v, `Computed ${key} is unchecked`, 'style', key)
910925
continue
911926
}
912-
const unitless =
913-
/^(?:opacity|z-index|font-weight|line-height|flex(?:-grow|-shrink)?|order|scale|aspect-ratio)$/
927+
if (
928+
central &&
929+
/^-?(?:\d*\.)?\d+$/.test(value) &&
930+
value !== '0' &&
931+
!property.startsWith('--') &&
932+
!reactUnitlessProperty(property)
933+
)
934+
note(
935+
v,
936+
'Central numeric-looking style value has unknown scalar type; React units remain unchecked',
937+
'style',
938+
key
939+
)
914940
emit(
915941
v,
916942
'style',
917943
key,
918-
/^-?(?:\d*\.)?\d+$/.test(value) &&
919-
!unitless.test(key) &&
920-
!key.startsWith('--') &&
921-
value !== '0'
922-
? `${value}px`
923-
: value,
944+
scalar === undefined ? value : reactStyleValue(property, scalar),
924945
context,
925946
central?.ref
926947
)

‎scripts/design-conformance/react-style.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
/** React DOM 19.2.4 unitlessNumbers, shared by its client and server style serializers. */
2+
const unitlessNumbers = new Set(
3+
'animationIterationCount aspectRatio borderImageOutset borderImageSlice borderImageWidth boxFlex boxFlexGroup boxOrdinalGroup columnCount columns flex flexGrow flexPositive flexShrink flexNegative flexOrder gridArea gridRow gridRowEnd gridRowSpan gridRowStart gridColumn gridColumnEnd gridColumnSpan gridColumnStart fontWeight lineClamp lineHeight opacity order orphans scale tabSize widows zIndex zoom fillOpacity floodOpacity stopOpacity strokeDasharray strokeDashoffset strokeMiterlimit strokeOpacity strokeWidth MozAnimationIterationCount MozBoxFlex MozBoxFlexGroup MozLineClamp msAnimationIterationCount msFlex msZoom msFlexGrow msFlexNegative msFlexOrder msFlexPositive msFlexShrink msGridColumn msGridColumnSpan msGridRow msGridRowSpan WebkitAnimationIterationCount WebkitBoxFlex WebKitBoxFlexGroup WebkitBoxOrdinalGroup WebkitColumnCount WebkitColumns WebkitFlex WebkitFlexGrow WebkitFlexPositive WebkitFlexShrink WebkitLineClamp'.split(
4+
' '
5+
)
6+
)
7+
8+
export const reactUnitlessProperty = (property: string) => unitlessNumbers.has(property)
9+
10+
/** React preserves strings and custom variables; only nonzero length numbers receive px. */
11+
export function reactStyleValue(property: string, value: string | number): string {
12+
return typeof value === 'number' &&
13+
value !== 0 &&
14+
!property.startsWith('--') &&
15+
!reactUnitlessProperty(property)
16+
? `${value}px`
17+
: String(value)
18+
}

0 commit comments

Comments
 (0)