Skip to content

Commit e8195c7

Browse files
committed
Optimize
1 parent 372592b commit e8195c7

File tree

8 files changed

+164
-120
lines changed

8 files changed

+164
-120
lines changed

src/codegen/Codegen.ts

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -100,16 +100,15 @@ function getBooleanConditionName(
100100
}
101101

102102
/**
103-
* Shallow-clone a NodeTree — only the root-level props object is cloned
104-
* (downstream code mutates tree.props via Object.assign).
105-
* Children and textChildren are shared by reference because they are
106-
* never mutated (verified: no push/pop/splice/sort on children arrays,
107-
* no child.props mutations in ResponsiveCodegen).
103+
* Shallow-clone a NodeTree — creates a new object so that per-instance
104+
* property reassignment (e.g., `tree.props = { ...tree.props, ...selectorProps }`)
105+
* doesn't leak across Codegen instances. Props object itself is shared by
106+
* reference — callers that need to merge create their own objects.
108107
*/
109108
function cloneTree(tree: NodeTree): NodeTree {
110109
return {
111110
component: tree.component,
112-
props: { ...tree.props },
111+
props: tree.props,
113112
children: tree.children,
114113
nodeType: tree.nodeType,
115114
nodeName: tree.nodeName,
@@ -364,14 +363,17 @@ export class Codegen {
364363
}
365364

366365
// Now await props (likely already resolved while children were processing)
367-
const props = await propsPromise
366+
const baseProps = await propsPromise
368367

369-
// Handle TEXT nodes
368+
// Handle TEXT nodes — create NEW merged object instead of mutating getProps() result.
370369
let textChildren: string[] | undefined
370+
let props: Record<string, unknown>
371371
if (node.type === 'TEXT') {
372372
const { children: textContent, props: textProps } = await renderText(node)
373373
textChildren = textContent
374-
Object.assign(props, textProps)
374+
props = { ...baseProps, ...textProps }
375+
} else {
376+
props = baseProps
375377
}
376378

377379
const component = getDevupComponentByNode(node, props)
@@ -486,16 +488,20 @@ export class Codegen {
486488
}
487489

488490
// Await props + selectorProps (likely already resolved while children built)
489-
const [props, selectorProps] = await Promise.all([
491+
const [baseProps, selectorProps] = await Promise.all([
490492
propsPromise,
491493
selectorPropsPromise,
492494
])
493495
perfEnd('getSelectorProps()', t)
494496

495497
const variants: Record<string, string> = {}
496498

499+
// Create a NEW merged object instead of mutating getProps() result.
500+
// This allows getProps cache to return raw references without cloning.
501+
const props = selectorProps
502+
? { ...baseProps, ...selectorProps.props }
503+
: baseProps
497504
if (selectorProps) {
498-
Object.assign(props, selectorProps.props)
499505
Object.assign(variants, selectorProps.variants)
500506
}
501507

src/codegen/props/index.ts

Lines changed: 4 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -38,16 +38,17 @@ export async function getProps(
3838
): Promise<Record<string, unknown>> {
3939
const cacheKey = node.id
4040
if (cacheKey) {
41-
// Sync fast path: return shallow clone from resolved cache (no microtask)
41+
// Sync fast path: return raw reference from resolved cache (no clone needed —
42+
// all callers that need to merge selectorProps create their own objects now).
4243
const resolved = getPropsResolved.get(cacheKey)
4344
if (resolved) {
4445
perfEnd('getProps(cached)', perfStart())
45-
return { ...resolved }
46+
return resolved
4647
}
4748
const cached = getPropsCache.get(cacheKey)
4849
if (cached) {
4950
perfEnd('getProps(cached)', perfStart())
50-
return { ...(await cached) }
51+
return await cached
5152
}
5253
}
5354

@@ -153,54 +154,3 @@ export async function getProps(
153154
perfEnd('getProps()', t)
154155
return result
155156
}
156-
157-
const CENTER_SKIP_KEYS = new Set(['alignItems', 'justifyContent'])
158-
const IMAGE_BOX_SKIP_KEYS = new Set([
159-
'alignItems',
160-
'justifyContent',
161-
'flexDir',
162-
'gap',
163-
'outline',
164-
'outlineOffset',
165-
'overflow',
166-
])
167-
168-
export function filterPropsWithComponent(
169-
component: string,
170-
props: Record<string, unknown>,
171-
): Record<string, unknown> {
172-
const newProps: Record<string, unknown> = {}
173-
for (const [key, value] of Object.entries(props)) {
174-
switch (component) {
175-
case 'Flex':
176-
// Only skip display/flexDir if it's exactly the default value (not responsive array)
177-
if (key === 'display' && value === 'flex') continue
178-
if (key === 'flexDir' && value === 'row') continue
179-
break
180-
case 'Grid':
181-
// Only skip display if it's exactly 'grid' (not responsive array or other value)
182-
if (key === 'display' && value === 'grid') continue
183-
break
184-
case 'Center':
185-
if (CENTER_SKIP_KEYS.has(key)) continue
186-
if (key === 'display' && value === 'flex') continue
187-
if (key === 'flexDir' && value === 'row') continue
188-
break
189-
case 'VStack':
190-
// Only skip flexDir if it's exactly 'column' (not responsive array or other value)
191-
if (key === 'flexDir' && value === 'column') continue
192-
if (key === 'display' && value === 'flex') continue
193-
break
194-
195-
case 'Image':
196-
case 'Box':
197-
if (component === 'Box' && !('maskImage' in props)) break
198-
if (IMAGE_BOX_SKIP_KEYS.has(key)) continue
199-
if (key === 'display' && value === 'flex') continue
200-
if (!('maskImage' in props) && ['bg'].includes(key)) continue
201-
break
202-
}
203-
newProps[key] = value
204-
}
205-
return newProps
206-
}

src/codegen/props/selector.ts

Lines changed: 35 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -146,27 +146,25 @@ async function computeSelectorProps(node: ComponentSetNode): Promise<{
146146

147147
const defaultProps = await getProps(node.defaultVariant)
148148

149-
const result = Object.entries(node.componentPropertyDefinitions).reduce(
150-
(acc, [name, definition]) => {
151-
if (name === 'effect' || name === 'viewport') return acc
152-
153-
const sanitizedName = sanitizePropertyName(name)
154-
if (definition.type === 'VARIANT' && definition.variantOptions) {
155-
acc.variants[sanitizedName] = definition.variantOptions
156-
.map((option) => `'${option}'`)
157-
.join(' | ')
158-
} else if (definition.type === 'INSTANCE_SWAP') {
159-
acc.variants[sanitizedName] = 'React.ReactNode'
160-
} else if (definition.type === 'BOOLEAN') {
161-
acc.variants[sanitizedName] = 'boolean'
162-
}
163-
return acc
164-
},
165-
{
166-
props: {} as Record<string, object | string>,
167-
variants: {} as Record<string, string>,
168-
},
169-
)
149+
const result: {
150+
props: Record<string, object | string>
151+
variants: Record<string, string>
152+
} = { props: {}, variants: {} }
153+
const defs = node.componentPropertyDefinitions
154+
for (const name in defs) {
155+
if (name === 'effect' || name === 'viewport') continue
156+
const definition = defs[name]
157+
const sanitizedName = sanitizePropertyName(name)
158+
if (definition.type === 'VARIANT' && definition.variantOptions) {
159+
result.variants[sanitizedName] = definition.variantOptions
160+
.map((option) => `'${option}'`)
161+
.join(' | ')
162+
} else if (definition.type === 'INSTANCE_SWAP') {
163+
result.variants[sanitizedName] = 'React.ReactNode'
164+
} else if (definition.type === 'BOOLEAN') {
165+
result.variants[sanitizedName] = 'boolean'
166+
}
167+
}
170168

171169
if (components.length > 0) {
172170
const findNodeAction = (action: Action) => action.type === 'NODE'
@@ -213,10 +211,12 @@ export async function getSelectorPropsForGroup(
213211

214212
// Build cache key from componentSet.id + filter + viewport
215213
const setId = componentSet.id
216-
const filterKey = Object.entries(variantFilter)
217-
.sort(([a], [b]) => a.localeCompare(b))
218-
.map(([k, v]) => `${k}=${v}`)
219-
.join('|')
214+
const filterParts: string[] = []
215+
for (const k in variantFilter) {
216+
filterParts.push(`${k}=${variantFilter[k]}`)
217+
}
218+
filterParts.sort()
219+
const filterKey = filterParts.join('|')
220220
const cacheKey = setId ? `${setId}::${filterKey}::${viewportValue ?? ''}` : ''
221221

222222
if (cacheKey) {
@@ -252,8 +252,8 @@ async function computeSelectorPropsForGroup(
252252
const variantProps = child.variantProperties || {}
253253

254254
// Check all filter conditions match
255-
for (const [key, value] of Object.entries(variantFilter)) {
256-
if (variantProps[key] !== value) return false
255+
for (const key in variantFilter) {
256+
if (variantProps[key] !== variantFilter[key]) return false
257257
}
258258

259259
// Check viewport if specified
@@ -335,13 +335,12 @@ function triggerTypeToEffect(triggerType: Trigger['type'] | undefined) {
335335
}
336336

337337
function difference(a: Record<string, unknown>, b: Record<string, unknown>) {
338-
return Object.entries(a).reduce(
339-
(acc, [key, value]) => {
340-
if (value !== undefined && b[key] !== value) {
341-
acc[key] = value
342-
}
343-
return acc
344-
},
345-
{} as Record<string, unknown>,
346-
)
338+
const result: Record<string, unknown> = {}
339+
for (const key in a) {
340+
const value = a[key]
341+
if (value !== undefined && b[key] !== value) {
342+
result[key] = value
343+
}
344+
}
345+
return result
347346
}

src/codegen/render/index.ts

Lines changed: 44 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,29 @@
11
import { space } from '../../utils'
2-
import { filterPropsWithComponent } from '../props'
32
import { isDefaultProp } from '../utils/is-default-prop'
43
import {
54
paddingLeftMultiline,
65
wrapReturnStatement,
76
} from '../utils/padding-left-multiline'
87
import { propsToString } from '../utils/props-to-str'
98

9+
const CENTER_SKIP_KEYS = new Set(['alignItems', 'justifyContent'])
10+
const IMAGE_BOX_SKIP_KEYS = new Set([
11+
'alignItems',
12+
'justifyContent',
13+
'flexDir',
14+
'gap',
15+
'outline',
16+
'outlineOffset',
17+
'overflow',
18+
])
19+
1020
export function renderNode(
1121
component: string,
1222
props: Record<string, unknown>,
1323
deps: number = 0,
1424
childrenCodes: string[],
1525
): string {
16-
const filteredProps = filterProps(props)
17-
18-
const propsString = propsToString(
19-
filterPropsWithComponent(component, filteredProps),
20-
)
26+
const propsString = propsToString(filterAndTransformProps(component, props))
2127
const hasChildren = childrenCodes.length > 0
2228
const tail = hasChildren ? `${space(deps)}</${component}>` : ''
2329
const multiProps = propsString.includes('\n')
@@ -65,16 +71,46 @@ ${Object.entries(filteredVariants)
6571
}`
6672
}
6773

68-
function filterProps(props: Record<string, unknown>) {
74+
function filterAndTransformProps(
75+
component: string,
76+
props: Record<string, unknown>,
77+
) {
78+
const hasMaskImage = 'maskImage' in props
6979
const newProps: Record<string, unknown> = {}
70-
for (const [key, value] of Object.entries(props)) {
80+
for (const key in props) {
81+
const value = props[key]
7182
if (value === null || value === undefined) {
7283
continue
7384
}
7485
const newValue = typeof value === 'number' ? String(value) : value
7586
if (isDefaultProp(key, newValue)) {
7687
continue
7788
}
89+
switch (component) {
90+
case 'Flex':
91+
if (key === 'display' && newValue === 'flex') continue
92+
if (key === 'flexDir' && newValue === 'row') continue
93+
break
94+
case 'Grid':
95+
if (key === 'display' && newValue === 'grid') continue
96+
break
97+
case 'Center':
98+
if (CENTER_SKIP_KEYS.has(key)) continue
99+
if (key === 'display' && newValue === 'flex') continue
100+
if (key === 'flexDir' && newValue === 'row') continue
101+
break
102+
case 'VStack':
103+
if (key === 'flexDir' && newValue === 'column') continue
104+
if (key === 'display' && newValue === 'flex') continue
105+
break
106+
case 'Image':
107+
case 'Box':
108+
if (component === 'Box' && !hasMaskImage) break
109+
if (IMAGE_BOX_SKIP_KEYS.has(key)) continue
110+
if (key === 'display' && newValue === 'flex') continue
111+
if (!hasMaskImage && key === 'bg') continue
112+
break
113+
}
78114
newProps[key] = newValue
79115
}
80116
return newProps

src/codegen/responsive/ResponsiveCodegen.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -421,7 +421,7 @@ export class ResponsiveCodegen {
421421
)
422422
perfEnd('getSelectorPropsForGroup(viewport)', t)
423423
if (Object.keys(selectorProps).length > 0) {
424-
Object.assign(tree.props, selectorProps)
424+
tree.props = { ...tree.props, ...selectorProps }
425425
}
426426
}
427427

@@ -642,7 +642,7 @@ export class ResponsiveCodegen {
642642
)
643643
perfEnd('getSelectorPropsForGroup()', t)
644644
if (Object.keys(selectorProps).length > 0) {
645-
Object.assign(tree.props, selectorProps)
645+
tree.props = { ...tree.props, ...selectorProps }
646646
}
647647
}
648648

@@ -692,7 +692,7 @@ export class ResponsiveCodegen {
692692
// Get pseudo-selector props (hover, active, disabled, etc.)
693693
const selectorProps = await getSelectorPropsForGroup(componentSet, {})
694694
if (Object.keys(selectorProps).length > 0) {
695-
Object.assign(tree.props, selectorProps)
695+
tree.props = { ...tree.props, ...selectorProps }
696696
}
697697

698698
// Render the tree to JSX
@@ -757,9 +757,9 @@ export class ResponsiveCodegen {
757757
const codegen = new Codegen(component)
758758
const tree = await codegen.getTree()
759759
perfEnd('Codegen.getTree(nonViewportVariant)', t)
760-
// Add pseudo-selector props to tree
760+
// Add pseudo-selector props to tree — create NEW props to avoid mutating cached tree
761761
if (selectorProps && Object.keys(selectorProps).length > 0) {
762-
Object.assign(tree.props, selectorProps)
762+
tree.props = { ...tree.props, ...selectorProps }
763763
}
764764
variantResults.push([variantValue, tree] as const)
765765
}

src/codegen/responsive/__tests__/index.test.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,15 @@ describe('responsive index helpers', () => {
6868
expect(optimized).toEqual(obj)
6969
})
7070

71+
it('collapses equal array values in responsive optimization', () => {
72+
const optimized = optimizeResponsiveValue([
73+
['10px', '20px'],
74+
['10px', '20px'],
75+
null,
76+
])
77+
expect(optimized).toEqual(['10px', '20px'])
78+
})
79+
7180
it('converts viewport variant values to breakpoints (case-insensitive)', () => {
7281
// lowercase
7382
expect(viewportToBreakpoint('mobile')).toBe('mobile')

0 commit comments

Comments
 (0)