Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/partial-clone-optimization.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@geajs/vite-plugin": patch
---

### @geajs/vite-plugin (patch)

- **Partial clone optimization for child components**: Components containing child component instances now use the clone optimization path. The compiler emits a placeholder element (`data-gea-child-slot`) in the static HTML template and generates `replaceChild` calls in `__cloneTemplate` to swap each placeholder with the child component's `el` at mount time.
87 changes: 80 additions & 7 deletions packages/vite-plugin-gea/src/codegen/gen-clone.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ import { EVENT_TYPES, VOID_ELEMENTS } from '../ir/constants.ts'
import { emitMount } from '../emit/registry.ts'
import { id, js, jsClassProp, jsExpr, jsMethod, tpl } from 'eszter'

/** HTML parent tags where it is safe to insert an arbitrary placeholder child element. */
const SAFE_CLONE_PARENTS = new Set([
'div', 'span', 'section', 'article', 'header', 'footer', 'main', 'nav', 'figure',
])

// ─── Types ─────────────────────────────────────────────────────────

export type CloneContentPatch = {
Expand All @@ -38,6 +43,11 @@ export type CloneIdentityPatch =
| { kind: 'dataGeaEvent'; childPath: number[]; token: string }
| { kind: 'attr'; childPath: number[]; expr: t.Expression; attrName: string }

export type CloneComponentSlotPatch = {
childPath: number[]
instanceVar: string
}

// ─── Event ID expression builder ──────────────────────────────────

function buildEventIdExpr(suffix?: string): t.Expression {
Expand All @@ -56,9 +66,13 @@ export function jsxToStaticHtml(
refCounter: { value: number },
elementPath: string[] = [],
_isRoot = true,
parentTag?: string,
): string | null {
const tagName = getJSXTagName(node.openingElement.name)
if (tagName && isComponentTag(tagName)) return null
if (tagName && isComponentTag(tagName)) {
if (!parentTag || !SAFE_CLONE_PARENTS.has(parentTag)) return null
return `<${parentTag} data-gea-child-slot></${parentTag}>`
}

const effectiveTag = tagName!
let html = `<${effectiveTag}`
Expand Down Expand Up @@ -105,7 +119,7 @@ export function jsxToStaticHtml(
}

html += '>'
const childHtml = processStaticChildren(node.children, refCounter, elementPath)
const childHtml = processStaticChildren(node.children, refCounter, elementPath, undefined, undefined, effectiveTag)
if (childHtml === null) return null
return html + childHtml + `</${effectiveTag}>`
}
Expand All @@ -118,6 +132,7 @@ function processStaticChildren(
parentPath: string[],
dcCursor?: { index: number },
directChildren?: ReturnType<typeof getDirectChildElements>,
parentTag?: string,
): string | null {
const cursor = dcCursor ?? { index: 0 }
const dc = directChildren ?? getDirectChildElements(children as any)
Expand All @@ -130,11 +145,11 @@ function processStaticChildren(
const seg = dc[cursor.index]?.selectorSegment
cursor.index++
const nextPath = seg ? [...parentPath, seg] : parentPath
const inner = jsxToStaticHtml(child, refCounter, nextPath, false)
const inner = jsxToStaticHtml(child, refCounter, nextPath, false, parentTag)
if (inner === null) return null
out += inner
} else if (t.isJSXFragment(child)) {
const inner = processStaticChildren(child.children, refCounter, parentPath, cursor, dc)
const inner = processStaticChildren(child.children, refCounter, parentPath, cursor, dc, parentTag)
if (inner === null) return null
out += inner
} else if (t.isJSXExpressionContainer(child) && !t.isJSXEmptyExpression(child.expression)) {
Expand Down Expand Up @@ -240,8 +255,8 @@ export function collectClonePatchEntries(
const flattened = getDirectChildElements(node.children as any)
flattened.forEach((dc, idx) => {
const tag = getJSXTagName(dc.node.openingElement.name)
const isCompChild = Boolean(tag && isComponentTag(tag))
collectClonePatchEntries(dc.node, [...path, idx], entries, isCompChild)
if (tag && isComponentTag(tag)) return
collectClonePatchEntries(dc.node, [...path, idx], entries, false)
})
}

Expand Down Expand Up @@ -468,6 +483,36 @@ function collectIdentityPatchesForElement(
})
}

// ─── Collect component slot patches ───────────────────────────────

function collectComponentSlotPatches(
node: t.JSXElement,
path: number[],
slots: CloneComponentSlotPatch[],
componentInstances: Map<string, import('../ir/index.ts').ChildComponent[]>,
instanceCursors: Map<string, number>,
consumeOnly = false,
): void {
const flattened = getDirectChildElements(node.children as any)
flattened.forEach((dc, idx) => {
const tag = getJSXTagName(dc.node.openingElement.name)
if (tag && isComponentTag(tag)) {
const instances = componentInstances.get(tag)
const cursor = instanceCursors.get(tag) ?? 0
const instance = instances?.[cursor]
if (instance) {
instanceCursors.set(tag, cursor + 1)
if (!consumeOnly) {
slots.push({ childPath: [...path, idx], instanceVar: instance.instanceVar })
}
Comment on lines +500 to +507

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Skip clone-slot patching for lazy child components.

This records every component instance as replaceable, but packages/vite-plugin-gea/src/codegen/gen-children.ts:138-167 only initializes non-lazy children in the constructor (if (child.lazy) return). The generated call at Lines 645-650 will therefore dereference this.<instanceVar>.el before the instance exists for lazy children and fail during mount. Please bail out of clone optimization for lazy slot children, or make slot collection fail hard instead of silently producing an unreplaced placeholder.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/vite-plugin-gea/src/codegen/gen-clone.ts` around lines 500 - 507,
The current clone-slot collection in gen-clone.ts records every component
instance (using componentInstances, instanceCursors and pushing into slots with
instance.instanceVar) but gen-children.ts only initializes non-lazy children (it
returns early when child.lazy), causing generated code to dereference
this.<instanceVar>.el for lazy children; update the collection logic to detect
lazy slot children (use the same child.lazy condition from gen-children.ts) and
either skip adding those instances to slots (bail out of the clone optimization
for lazy slot children) or throw a clear error during slot collection to fail
fast instead of producing an unreplaced placeholder. Ensure references to
instanceVar are only produced for children guaranteed to be constructed.

}
collectComponentSlotPatches(dc.node, [...path, idx], slots, componentInstances, instanceCursors, true)
} else {
collectComponentSlotPatches(dc.node, [...path, idx], slots, componentInstances, instanceCursors, consumeOnly)
}
})
}

// ─── Generate clone members ────────────────────────────────────────

export function generateCloneMembers(
Expand Down Expand Up @@ -520,7 +565,12 @@ export function generateCloneMembers(
return t;
})()`

const cloneMethodBody = buildCloneTemplateBody(identityPatches, contentPatches, cloneCtx)
const componentSlotPatches: CloneComponentSlotPatch[] = []
if (cloneCtx.componentInstances && cloneCtx.componentInstances.size > 0) {
collectComponentSlotPatches(root, [], componentSlotPatches, cloneCtx.componentInstances, new Map())
}

const cloneMethodBody = buildCloneTemplateBody(identityPatches, contentPatches, componentSlotPatches, cloneCtx)
const cloneMethod = jsMethod`[${id('GEA_CLONE_TEMPLATE')}]() {}`
cloneMethod.body.body.push(...cloneMethodBody)

Expand All @@ -532,6 +582,7 @@ export function generateCloneMembers(
function buildCloneTemplateBody(
identityPatches: CloneIdentityPatch[],
contentPatches: CloneContentPatch[],
componentSlotPatches: CloneComponentSlotPatch[],
cloneCtx: Ctx,
): t.Statement[] {
const rootVar = id('__root')
Expand All @@ -545,6 +596,7 @@ function buildCloneTemplateBody(
const allChildPaths = new Set<string>()
for (const p of identityPatches) allChildPaths.add(p.childPath.join('_'))
for (const p of contentPatches) allChildPaths.add(p.childPath.join('_'))
for (const p of componentSlotPatches) allChildPaths.add(p.childPath.join('_'))
for (const key of allChildPaths) {
if (!key) continue
const path = key.split('_').map((n) => parseInt(n, 10))
Expand Down Expand Up @@ -580,6 +632,27 @@ function buildCloneTemplateBody(
stmts.push(...mountStmts)
}

for (const slot of componentSlotPatches) {
const nav = navFor(slot.childPath)
stmts.push(
t.expressionStatement(
t.callExpression(
t.memberExpression(
t.memberExpression(nav, t.identifier('parentNode')),
t.identifier('replaceChild'),
),
[
t.memberExpression(
t.memberExpression(t.thisExpression(), t.identifier(slot.instanceVar)),
t.identifier('el'),
),
nav,
],
),
),
)
}

stmts.push(js`return ${rootVar};`)
return stmts
}
1 change: 0 additions & 1 deletion packages/vite-plugin-gea/src/codegen/generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,6 @@ export function transformComponentFile(
const preCloneEligible =
t.isJSXElement(retStmt.argument) &&
preReturnStmts.length === 0 &&
componentInstances.size === 0 &&
analysis.conditionalSlots.length === 0 &&
analysis.arrayMaps.length === 0 &&
analysis.unresolvedMaps.length === 0 &&
Expand Down
156 changes: 156 additions & 0 deletions packages/vite-plugin-gea/tests/optimization-tests.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -686,6 +686,162 @@ export default class GridStore extends Store {
}
})

// --- P1-PERF-6: Partial clone optimization for components with child components ---
test('clone optimization: component with child components gets __tpl and __cloneTemplate', () => {
const source = `
import { Component } from '@geajs/core'
import Header from './Header'
export default class MyComp extends Component {
template() {
return (
<div class="wrapper">
<Header />
<p>static text</p>
</div>
)
}
}
`
const code = transformComponentSource(source)
assert.ok(code.includes('__tpl'), 'should have __tpl static field')
assert.ok(code.includes('GEA_CLONE_TEMPLATE'), 'should have [GEA_CLONE_TEMPLATE] method')
})

test('clone optimization: __cloneTemplate replaces placeholder with child component el', () => {
const source = `
import { Component } from '@geajs/core'
import Header from './Header'
export default class MyComp extends Component {
template() {
return (
<div class="wrapper">
<Header />
<p>static text</p>
</div>
)
}
}
`
const code = transformComponentSource(source)
assert.ok(code.includes('_header'), 'should reference header instance')
assert.ok(code.includes('.el'), 'should access child el')
assert.ok(code.includes('replaceChild'), 'should use replaceChild')
})

test('clone optimization: static html skeleton uses placeholder for child component', () => {
const source = `
import { Component } from '@geajs/core'
import Panel from './Panel'
export default class MyComp extends Component {
template() {
return (
<div>
<span>before</span>
<Panel />
<span>after</span>
</div>
)
}
}
`
const code = transformComponentSource(source)
assert.ok(code.includes('data-gea-child-slot'), 'placeholder should be in static html')
assert.ok(code.includes('__tpl'), 'should have clone optimization')
})

test('clone optimization: component with dynamic class and child component', () => {
const source = `
import { Component } from '@geajs/core'
import Footer from './Footer'
export default class MyComp extends Component {
template({ active }) {
return (
<div class={active ? 'active' : 'inactive'}>
<Footer />
</div>
)
}
}
`
const code = transformComponentSource(source)
assert.ok(code.includes('__tpl'), 'should have clone optimization even with dynamic class')
assert.ok(code.includes('GEA_CLONE_TEMPLATE'), 'should have [GEA_CLONE_TEMPLATE]')
assert.ok(code.includes('replaceChild'), 'should replace placeholder')
})

test('clone optimization: runtime DOM - child slot placeholder is replaced after mount', async () => {
const restoreDom = installDom()
let app: { render: (el: HTMLElement) => void; dispose: () => void } | undefined
let root: HTMLElement | undefined

try {
const seed = `opt-runtime-${Date.now()}-${Math.random()}`
const [{ default: Component }] = await loadRuntimeModules(seed)

const headerSource = `
import { Component } from '@geajs/core'
export default function Header() {
return <header class="the-header">Hello</header>
}
`
const Header = await compileJsxComponent(
headerSource,
'/virtual/Header.jsx',
'Header',
{ Component },
)

const parentSource = `
import { Component } from '@geajs/core'
import Header from './Header'
export default class ParentComp extends Component {
template() {
return (
<div class="wrapper">
<Header />
<p>static text</p>
</div>
)
}
}
`
const ParentComp = await compileJsxComponent(
parentSource,
'/virtual/ParentComp.jsx',
'ParentComp',
{ Component, Header },
)

root = document.createElement('div')
document.body.appendChild(root)
app = new (ParentComp as new () => { render: (el: HTMLElement) => void; dispose: () => void })()
app.render(root)
await flushMicrotasks()

assert.equal(
root.querySelector('[data-gea-child-slot]'),
null,
'data-gea-child-slot placeholder should be removed after mount',
)

assert.ok(
root.querySelector('.the-header') !== null,
'child component element (.the-header) should be inside root',
)

assert.ok(
root.querySelector('.wrapper') !== null,
'parent wrapper element should be in root',
)

} finally {
app?.dispose()
await flushMicrotasks()
root?.remove()
restoreDom()
}
})

test('selecting a cell in a grid should only mutate the old and new selected cells, not all cells', async () => {
const restoreDom = installDom()

Expand Down