From 76a59900a131bce72a1c75b6eeee1409d6a85e16 Mon Sep 17 00:00:00 2001 From: Ruud Date: Thu, 15 Oct 2026 11:03:56 +0200 Subject: [PATCH 01/17] perf(render): diff and map commitupdate props in one pass --- .changeset/fused-commit-update-diff.md | 5 + .../src/render/createHostConfig.ts | 21 +--- .../render/mapReactPropsToLightning.test.ts | 108 +++++++++++++++++- .../src/render/mapReactPropsToLightning.ts | 91 ++++++++++++--- .../react-lightning/src/utils/simpleDiff.ts | 2 +- 5 files changed, 186 insertions(+), 41 deletions(-) create mode 100644 .changeset/fused-commit-update-diff.md diff --git a/.changeset/fused-commit-update-diff.md b/.changeset/fused-commit-update-diff.md new file mode 100644 index 00000000..93e39232 --- /dev/null +++ b/.changeset/fused-commit-update-diff.md @@ -0,0 +1,5 @@ +--- +'@plextv/react-lightning': patch +--- + +diff and map commitUpdate props in one pass, and skip diffing children on views and images diff --git a/packages/react-lightning/src/render/createHostConfig.ts b/packages/react-lightning/src/render/createHostConfig.ts index 101cf109..e31cca47 100644 --- a/packages/react-lightning/src/render/createHostConfig.ts +++ b/packages/react-lightning/src/render/createHostConfig.ts @@ -12,9 +12,8 @@ import { type RendererNode, } from '../types'; import { scheduleMicrotask } from '../utils/scheduleMicrotask'; -import { simpleDiff } from '../utils/simpleDiff'; import { isPrimitiveTextContent } from './isValidTextChild'; -import { mapReactPropsToLightning } from './mapReactPropsToLightning'; +import { diffReactPropsToLightning, mapReactPropsToLightning } from './mapReactPropsToLightning'; import type { Plugin } from './Plugin'; export type ReconcilerContainer = { @@ -43,14 +42,6 @@ export type LightningHostConfig = HostConfig< extraDevToolsConfig: unknown; }; -function hasKeys(object: object): boolean { - for (const _ in object) { - return true; - } - - return false; -} - type LightningHostConfigOptions = Pick; export function createHostConfig(options?: LightningHostConfigOptions): LightningHostConfig { @@ -229,15 +220,9 @@ export function createHostConfig(options?: LightningHostConfigOptions): Lightnin }, commitUpdate(instance, type, oldProps, newProps) { - const diffedProps: Partial | null = simpleDiff(oldProps, newProps); - - if (!diffedProps) { - return null; - } - - const updatePayload = mapReactPropsToLightning(type, diffedProps); + const updatePayload = diffReactPropsToLightning(type, oldProps, newProps); - if (hasKeys(updatePayload)) { + if (updatePayload) { instance.setProps(updatePayload); } diff --git a/packages/react-lightning/src/render/mapReactPropsToLightning.test.ts b/packages/react-lightning/src/render/mapReactPropsToLightning.test.ts index 5b89139c..fe5f5411 100644 --- a/packages/react-lightning/src/render/mapReactPropsToLightning.test.ts +++ b/packages/react-lightning/src/render/mapReactPropsToLightning.test.ts @@ -1,8 +1,15 @@ import { createElement } from 'react'; -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; -import { LightningElementType, type LightningTextElementProps } from '../types'; -import { mapReactPropsToLightning } from './mapReactPropsToLightning'; +import { + type LightningElementProps, + LightningElementType, + type LightningTextElementProps, +} from '../types'; +import type { LightningElement } from '../types'; +import { simpleDiff } from '../utils/simpleDiff'; +import { createHostConfig } from './createHostConfig'; +import { diffReactPropsToLightning, mapReactPropsToLightning } from './mapReactPropsToLightning'; describe('mapReactPropsToLightning — text children', () => { const mapText = (children: unknown) => @@ -32,3 +39,98 @@ describe('mapReactPropsToLightning — text children', () => { expect(mapText(formattedMessage).text).toBeUndefined(); }); }); + +describe('diffReactPropsToLightning', () => { + const reference = ( + type: LightningElementType, + oldProps: Record, + newProps: Record, + ) => { + const diff = simpleDiff(oldProps, newProps); + + if (!diff) { + return null; + } + + const mapped = mapReactPropsToLightning(type, diff as LightningElementProps); + + return Object.keys(mapped).length ? mapped : null; + }; + + const onFocus = () => {}; + const child = (w: number) => createElement('lng-view', { key: w, style: { w } }); + + const cases: Array<[string, Record, Record]> = [ + ['same object', { a: 1 }, { a: 1 }], + ['equal fresh style', { style: { w: 1, h: 2 } }, { style: { w: 1, h: 2 } }], + ['changed style', { style: { w: 1 } }, { style: { w: 2 } }], + ['added prop', { a: 1 }, { a: 1, b: 2 }], + ['added undefined prop', { a: 1 }, { a: 1, b: undefined }], + ['removed prop only', { a: 1, b: 2 }, { a: 1 }], + ['removed and changed', { a: 1, b: 2 }, { a: 3 }], + ['handler swapped', { onFocus }, { onFocus: () => {} }], + ['ref and key changes', { ref: {}, key: 'a', a: 1 }, { ref: {}, key: 'b', a: 1 }], + ['element children changed', { children: [child(1)] }, { children: [child(2)] }], + ['string children changed', { children: 'a' }, { children: 'b' }], + ['array children changed', { children: ['n: ', 1] }, { children: ['n: ', 2] }], + ['children added', {}, { children: 'x' }], + ['element child for text', { children: 'a' }, { children: child(1) }], + ['children and style', { children: 'a', style: { w: 1 } }, { children: 'b', style: { w: 2 } }], + ]; + + for (const type of [LightningElementType.View, LightningElementType.Text]) { + for (const [name, oldProps, newProps] of cases) { + it(`matches simpleDiff + map for ${type}: ${name}`, () => { + const same = name === 'same object'; + const next = same ? oldProps : newProps; + + expect( + diffReactPropsToLightning( + type, + oldProps as LightningElementProps, + next as LightningElementProps, + ), + ).toEqual(reference(type, oldProps, next)); + }); + } + } + + it('keeps the key order of the new props', () => { + const result = diffReactPropsToLightning( + LightningElementType.Text, + { b: 1, children: 'a', a: 1 } as unknown as LightningElementProps, + { b: 2, children: 'b', a: 2 } as unknown as LightningElementProps, + ); + + expect(Object.keys(result ?? {})).toEqual(['b', 'text', 'a']); + }); +}); + +describe('commitUpdate', () => { + const commit = (type: LightningElementType, oldProps: object, newProps: object) => { + const setProps = vi.fn(); + + createHostConfig().commitUpdate?.( + { setProps } as unknown as LightningElement, + type, + oldProps as LightningElementProps, + newProps as LightningElementProps, + {} as never, + ); + + return setProps; + }; + + it('skips a view whose element children changed but nothing else', () => { + const before = { children: [createElement('lng-view', { key: 1 })], style: { w: 1 } }; + const after = { children: [createElement('lng-view', { key: 2 })], style: { w: 1 } }; + + expect(commit(LightningElementType.View, before, after)).not.toHaveBeenCalled(); + }); + + it('stages changed text children as text', () => { + const setProps = commit(LightningElementType.Text, { children: 'a' }, { children: 'b' }); + + expect(setProps).toHaveBeenCalledExactlyOnceWith({ text: 'b' }); + }); +}); diff --git a/packages/react-lightning/src/render/mapReactPropsToLightning.ts b/packages/react-lightning/src/render/mapReactPropsToLightning.ts index 4aeec87e..e2e03c91 100644 --- a/packages/react-lightning/src/render/mapReactPropsToLightning.ts +++ b/packages/react-lightning/src/render/mapReactPropsToLightning.ts @@ -3,8 +3,34 @@ import { LightningElementType, type LightningTextElementProps, } from '../types'; +import { areValuesEqual } from '../utils/simpleDiff'; import { isValidTextChild } from './isValidTextChild'; +// Text takes its children as raw text content rather than as rendered +// child nodes — but only the primitive cases reach us here. Anything +// React must render (a ``, a ternary, a fragment) is +// routed through the reconciler by `shouldSetTextContent` and folded +// back in by `LightningTextElement`, so we never see it as a prop. +function mapTextChildren(children: unknown): string | undefined { + if (isValidTextChild(children)) { + return String(children); + } + + if (Array.isArray(children)) { + let text = ''; + + for (let i = 0; i < children.length; i++) { + if (isValidTextChild(children[i])) { + text += String(children[i]); + } + } + + return text; + } + + return undefined; +} + /** * Converts React props to work with LightningElements */ @@ -23,27 +49,11 @@ export function mapReactPropsToLightning( for (prop in props) { switch (prop) { case 'children': - // Text takes its children as raw text content rather than as rendered - // child nodes — but only the primitive cases reach us here. Anything - // React must render (a ``, a ternary, a fragment) is - // routed through the reconciler by `shouldSetTextContent` and folded - // back in by `LightningTextElement`, so we never see it as a prop. if (type === LightningElementType.Text) { - const textProps = mappedProps as LightningTextElementProps; - const children = props[prop]; - - if (isValidTextChild(children)) { - textProps.text = String(children); - } else if (Array.isArray(children)) { - let text = ''; - - for (let i = 0; i < children.length; i++) { - if (isValidTextChild(children[i])) { - text += String(children[i]); - } - } + const text = mapTextChildren(props[prop]); - textProps.text = text; + if (text !== undefined) { + (mappedProps as LightningTextElementProps).text = text; } } @@ -63,3 +73,46 @@ export function mapReactPropsToLightning( return mappedProps; } + +// Same result as mapReactPropsToLightning(type, simpleDiff(old, new)) in one pass. Only text +// maps children to a prop, so other types skip diffing them; removed props are ignored, as before. +export function diffReactPropsToLightning( + type: LightningElementType, + oldProps: LightningElementProps, + newProps: LightningElementProps, +): Partial | null { + if (oldProps === newProps) { + return null; + } + + const isText = type === LightningElementType.Text; + let mappedProps: Partial | null = null; + let prop: keyof typeof newProps; + + for (prop in newProps) { + if (prop === 'ref' || prop === 'key' || (prop === 'children' && !isText)) { + continue; + } + + const value = newProps[prop]; + + if (prop in oldProps && areValuesEqual(oldProps[prop], value)) { + continue; + } + + if (prop === 'children') { + const text = mapTextChildren(value); + + if (text !== undefined) { + mappedProps ??= {}; + (mappedProps as LightningTextElementProps).text = text; + } + } else { + mappedProps ??= {}; + // oxlint-disable-next-line typescript/no-explicit-any -- TODO + mappedProps[prop] = value as any; + } + } + + return mappedProps; +} diff --git a/packages/react-lightning/src/utils/simpleDiff.ts b/packages/react-lightning/src/utils/simpleDiff.ts index 6afd3260..716bba32 100644 --- a/packages/react-lightning/src/utils/simpleDiff.ts +++ b/packages/react-lightning/src/utils/simpleDiff.ts @@ -18,7 +18,7 @@ function isReactElement(value: unknown): value is ReactElement { ); } -function areValuesEqual(first: unknown, second: unknown): boolean { +export function areValuesEqual(first: unknown, second: unknown): boolean { // Strict equality check for primitives and same references if (first === second) { return true; From f9d6d82faf30077a70fe895c52efed1d73d148a5 Mon Sep 17 00:00:00 2001 From: Ruud Date: Thu, 15 Oct 2026 11:07:01 +0200 Subject: [PATCH 02/17] perf(element): destroy a deleted subtree without detaching each descendant --- .changeset/destroy-deleted-subtree-once.md | 5 + .../src/element/LightningViewElement.ts | 48 +++++- .../src/render/createHostConfig.ts | 1 + .../src/render/deleteSubtree.test.ts | 153 ++++++++++++++++++ 4 files changed, 200 insertions(+), 7 deletions(-) create mode 100644 .changeset/destroy-deleted-subtree-once.md create mode 100644 packages/react-lightning/src/render/deleteSubtree.test.ts diff --git a/.changeset/destroy-deleted-subtree-once.md b/.changeset/destroy-deleted-subtree-once.md new file mode 100644 index 00000000..694909bc --- /dev/null +++ b/.changeset/destroy-deleted-subtree-once.md @@ -0,0 +1,5 @@ +--- +'@plextv/react-lightning': patch +--- + +destroy each element once, and skip detaching descendants one by one when react deletes a whole subtree diff --git a/packages/react-lightning/src/element/LightningViewElement.ts b/packages/react-lightning/src/element/LightningViewElement.ts index 5f3dd0d9..0b84d3d0 100644 --- a/packages/react-lightning/src/element/LightningViewElement.ts +++ b/packages/react-lightning/src/element/LightningViewElement.ts @@ -244,6 +244,8 @@ export class LightningViewElement< public _flatOffsetX = 0; public _flatOffsetY = 0; private _isObservingResize = false; + private _destroyed = false; + private _deleted = false; public get visible(): boolean { return this._visible; @@ -832,7 +834,40 @@ export class LightningViewElement< this._eventEmitter.emit('initialized'); } + /** + * Marks the root of a subtree React deleted. It's already out of its parent, + * so its descendants can skip detaching one by one as they're destroyed. + */ + public markDeleted(): void { + this._deleted = true; + } + + // Memoized up the chain, so a deleted subtree costs one walk in total. + private _isInDeletedTree(): boolean { + if (this._deleted || this._destroyed) { + return true; + } + + if (this._parent?._isInDeletedTree()) { + this._deleted = true; + + return true; + } + + return false; + } + public destroy(): void { + // React calls detachDeletedInstance for a fiber and its alternate, which + // share this instance. + if (this._destroyed) { + return; + } + + const detached = this._isInDeletedTree(); + + this._destroyed = true; + if (this._isObservingResize && this._resizeObserver) { this._resizeObserver.unobserve(this); this._isObservingResize = false; @@ -846,7 +881,10 @@ export class LightningViewElement< this.children[i]?.destroy(); } - this.parent?.removeChild(this); + if (!detached) { + this.parent?.removeChild(this); + } + this.children.length = 0; // More efficient than reassigning if (this._deferNodeRemovalHandler) { @@ -985,12 +1023,8 @@ export class LightningViewElement< public removeChild(child: LightningElement): void { const index = this.children.indexOf(child); - // Idempotent — React's reconciler hits this path twice per unmount: - // first via the host-config `removeChild`, then again from inside - // `destroy()` (`this.parent?.removeChild(this)`). The second call is - // a no-op for `this.children` but used to re-emit `childRemoved`, - // doubling downstream worker traffic in plugin-flexbox (each emit - // fires `removeNode` + `queueRender` etc.). + // Idempotent: a removed child that's destroyed later (outside a React + // deletion) lands here again, and must not re-emit `childRemoved`. if (index < 0) { return; } diff --git a/packages/react-lightning/src/render/createHostConfig.ts b/packages/react-lightning/src/render/createHostConfig.ts index e31cca47..53d857ff 100644 --- a/packages/react-lightning/src/render/createHostConfig.ts +++ b/packages/react-lightning/src/render/createHostConfig.ts @@ -208,6 +208,7 @@ export function createHostConfig(options?: LightningHostConfigOptions): Lightnin removeChild(parentInstance, child) { if (child) { parentInstance.removeChild(child); + child.markDeleted(); } }, diff --git a/packages/react-lightning/src/render/deleteSubtree.test.ts b/packages/react-lightning/src/render/deleteSubtree.test.ts new file mode 100644 index 00000000..f9e51c39 --- /dev/null +++ b/packages/react-lightning/src/render/deleteSubtree.test.ts @@ -0,0 +1,153 @@ +import type { RendererMain } from '@lightningjs/renderer'; +import { createElement, type ReactNode } from 'react'; +import createReconciler from 'react-reconciler'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import { LightningViewElement } from '../element/LightningViewElement'; +import { type LightningElement, LightningElementType } from '../types'; +import { createHostConfig } from './createHostConfig'; + +const destroyedNodes: number[] = []; +let nodeId = 100; + +const makeNode = (props: Record) => { + const node = { + ...props, + id: nodeId++, + alpha: 1, + w: 0, + h: 0, + parent: null, + on: () => {}, + off: () => {}, + emit: () => {}, + destroy: () => destroyedNodes.push(node.id), + }; + + return node; +}; + +const renderer = { + createNode: makeNode, + createTextNode: makeNode, + destroyNode: (node: { destroy: () => void }) => node.destroy(), +} as unknown as RendererMain; + +function setup() { + const reconciler = createReconciler(createHostConfig()); + const container = reconciler.createContainer( + { renderer, plugins: [] }, + 0, + null, + false, + null, + '', + () => {}, + () => {}, + () => {}, + () => {}, + ); + + return (element: ReactNode) => { + reconciler.updateContainerSync(element, container, null, null); + reconciler.flushSyncWork(); + reconciler.flushPassiveEffects(); + }; +} + +const view = (props: Record, ...children: ReactNode[]) => + createElement(LightningElementType.View, props, ...children); + +const elementsById = () => ({ ...LightningViewElement.allElements }); + +afterEach(() => { + destroyedNodes.length = 0; + vi.restoreAllMocks(); +}); + +describe('deleting a subtree', () => { + const tree = (showList: boolean, color: number) => + view( + { style: { color } }, + showList + ? view( + { key: 'list', style: { color } }, + view({ key: 'a', style: { color } }, view({ style: { color } })), + view({ key: 'b', style: { color } }), + createElement(LightningElementType.Text, { key: 't', style: { color } }, 'hi'), + ) + : null, + ); + + it('destroys every node once and only tells the surviving parent', () => { + const render = setup(); + + render(tree(true, 1)); + // An update gives every fiber an alternate, which is what makes React call + // detachDeletedInstance twice per instance. + render(tree(true, 2)); + + const before = elementsById(); + const root = Object.values(before).find((el) => el.children.length === 1 && !el.parent?.parent); + const list = root?.children[0] as LightningElement; + const dying = [list]; + + for (let i = 0; i < dying.length; i++) { + dying.push(...(dying[i] as LightningElement).children); + } + + const destroy = vi.spyOn(LightningViewElement.prototype, 'destroy'); + const removed: Array<[number, number]> = []; + + for (const el of [root, ...dying] as LightningElement[]) { + el.on('childRemoved', (child) => removed.push([el.id, child.id])); + } + + render(tree(false, 2)); + + expect(removed).toEqual([[root?.id, list.id]]); + expect(root?.children).toEqual([]); + expect(new Set(destroyedNodes).size).toBe(destroyedNodes.length); + expect(destroyedNodes.sort()).toEqual(dying.map((el) => el.node.id).sort()); + expect(destroy.mock.calls.length).toBeGreaterThanOrEqual(dying.length); + + for (const el of dying) { + expect(LightningViewElement.allElements[el.id]).toBeUndefined(); + expect(el.children).toEqual([]); + } + }); +}); + +describe('destroying outside a React deletion', () => { + const make = () => + new LightningViewElement({ style: {} }, renderer, [], { _debugInfo: {} } as never); + + it('still detaches a live element from its parent', () => { + const parent = make(); + const child = make(); + const removed = vi.fn(); + + parent.insertChild(child); + parent.on('childRemoved', removed); + child.destroy(); + + expect(parent.children).toEqual([]); + expect(removed).toHaveBeenCalledOnce(); + }); + + it('runs once when called again', () => { + const parent = make(); + const child = make(); + const destroyed = vi.fn(); + + parent.insertChild(child); + child.on('destroy', destroyed); + parent.destroy(); + child.destroy(); + parent.destroy(); + + expect(destroyed).toHaveBeenCalledOnce(); + expect(new Set(destroyedNodes).size).toBe(2); + expect(destroyedNodes).toHaveLength(2); + }); +}); From 334dd62a32a067f9f9e4c2194007e1cb9ffc6e58 Mon Sep 17 00:00:00 2001 From: Ruud Date: Thu, 15 Oct 2026 11:10:31 +0200 Subject: [PATCH 03/17] perf(element): skip node props on copy instead of deleting them --- .changeset/skip-node-props-without-delete.md | 5 +++ .../LightningViewElement.flatten.spec.ts | 11 +++++ .../src/element/LightningViewElement.spec.ts | 20 +++++++++ .../src/element/LightningViewElement.ts | 43 +++++++++++++------ 4 files changed, 66 insertions(+), 13 deletions(-) create mode 100644 .changeset/skip-node-props-without-delete.md diff --git a/.changeset/skip-node-props-without-delete.md b/.changeset/skip-node-props-without-delete.md new file mode 100644 index 00000000..5b1e09c9 --- /dev/null +++ b/.changeset/skip-node-props-without-delete.md @@ -0,0 +1,5 @@ +--- +'@plextv/react-lightning': patch +--- + +skip non-numeric sizes and a withheld alpha while copying props onto the node instead of deleting them first diff --git a/packages/react-lightning/src/element/LightningViewElement.flatten.spec.ts b/packages/react-lightning/src/element/LightningViewElement.flatten.spec.ts index 9295f221..7ad9bed3 100644 --- a/packages/react-lightning/src/element/LightningViewElement.flatten.spec.ts +++ b/packages/react-lightning/src/element/LightningViewElement.flatten.spec.ts @@ -91,6 +91,17 @@ describe('flattenLayoutViews', () => { expect(el.node.h).toBe(50); }); + it('stays flattened when a styled alpha lands while paint is withheld', async () => { + const el = createElement({ w: 100, h: 50 }); + + el.withholdPaintUntilLayout(); + el.setProps({ testID: 'a', style: { w: 100, h: 50, alpha: 0.5 } } as never); + await flush(); + + expect(el.isFlattened).toBe(true); + expect(el.node.alpha).toBe(0); + }); + it('does not flatten a view with a visual prop', () => { const el = createElement({ w: 100, h: 50, color: 0xff0000ff }); diff --git a/packages/react-lightning/src/element/LightningViewElement.spec.ts b/packages/react-lightning/src/element/LightningViewElement.spec.ts index ae1c31b3..4ce4d906 100644 --- a/packages/react-lightning/src/element/LightningViewElement.spec.ts +++ b/packages/react-lightning/src/element/LightningViewElement.spec.ts @@ -139,6 +139,26 @@ describe('LightningViewElement paint withholding', () => { expect(el.node.alpha).toBe(0.25); }); + it('keeps a withheld alpha and non-numeric sizes off the node on a full update', async () => { + const el = createElement({ w: 100, h: 50, alpha: 1 }); + + el.withholdPaintUntilLayout(); + // A non-style prop sends the update through the full (plugin) path. + el.setProps({ + focusable: true, + style: { w: '100%', h: '50%', alpha: 0.25, x: 7 }, + } as unknown as Parameters[0]); + await flush(); + + expect(el.node.x).toBe(7); + expect(el.node.w).toBe(100); + expect(el.node.h).toBe(50); + expect(el.node.alpha).toBe(0); + + el.emitLayoutEvent(); + expect(el.node.alpha).toBe(0.25); + }); + it('reveals immediately when released before a layout (e.g. detached by a boundary)', () => { const el = createElement({ w: 100, h: 50, alpha: 1 }); diff --git a/packages/react-lightning/src/element/LightningViewElement.ts b/packages/react-lightning/src/element/LightningViewElement.ts index 0b84d3d0..2554df37 100644 --- a/packages/react-lightning/src/element/LightningViewElement.ts +++ b/packages/react-lightning/src/element/LightningViewElement.ts @@ -226,8 +226,15 @@ export class LightningViewElement< } } - private static _isLayoutOnlyNodeProps(props: Record): boolean { + private static _isLayoutOnlyNodeProps( + props: Record, + ignoreAlpha = false, + ): boolean { for (const key in props) { + if (ignoreAlpha && key === 'alpha') { + continue; + } + if (LightningViewElement._needsRealNode(key, props[key])) { return false; } @@ -1467,20 +1474,18 @@ export class LightningViewElement< // node itself. Once the layout system calculates the actual pixel values, // it will update the lightning node and remove the string values from the // style proxy. - if (typeof lngProps.w !== 'number') { - delete lngProps.w; - } - - if (typeof lngProps.h !== 'number') { - delete lngProps.h; - } + // These keys are skipped when the props are copied onto the node below, + // rather than deleted here: a delete drops lngProps into dictionary mode. + const skipW = typeof lngProps.w !== 'number'; + const skipH = typeof lngProps.h !== 'number'; // While paint is withheld, a styled alpha change must update the alpha we // restore on first layout, not the node's (which stays 0 so the node keeps // hiding). See {@link withholdPaintUntilLayout}. - if (this._paintWithheld && lngProps.alpha !== undefined) { - this._withheldAlpha = lngProps.alpha; - delete lngProps.alpha; + const skipAlpha = this._paintWithheld && lngProps.alpha !== undefined; + + if (skipAlpha) { + this._withheldAlpha = lngProps.alpha as number; } let flattenedMoved = false; @@ -1489,7 +1494,10 @@ export class LightningViewElement< if ( this._flattened && (this.props.transition !== undefined || - !LightningViewElement._isLayoutOnlyNodeProps(lngProps as Record)) + !LightningViewElement._isLayoutOnlyNodeProps( + lngProps as Record, + skipAlpha, + )) ) { this._materialize(); } @@ -1524,7 +1532,16 @@ export class LightningViewElement< (this.node as { updateShaderUniforms?: boolean }).updateShaderUniforms = false; } - Object.assign(this.node, lngProps); + const node = this.node as unknown as Record; + let key: keyof typeof lngProps; + + for (key in lngProps) { + if ((key === 'w' && skipW) || (key === 'h' && skipH) || (key === 'alpha' && skipAlpha)) { + continue; + } + + node[key] = lngProps[key]; + } if (flattenedMoved) { this._refreshFlattenedChildren(false); From 32050a8ec71a5d8f288a19553c6f097fd1ddf0a4 Mon Sep 17 00:00:00 2001 From: Ruud Date: Thu, 15 Oct 2026 11:37:19 +0200 Subject: [PATCH 04/17] perf(element): build node props in one object without a rest copy --- .changeset/node-props-one-object.md | 5 ++ .../LightningViewElement.nodeProps.spec.ts | 88 +++++++++++++++++++ .../src/element/LightningViewElement.ts | 63 +++++++------ 3 files changed, 130 insertions(+), 26 deletions(-) create mode 100644 .changeset/node-props-one-object.md create mode 100644 packages/react-lightning/src/element/LightningViewElement.nodeProps.spec.ts diff --git a/.changeset/node-props-one-object.md b/.changeset/node-props-one-object.md new file mode 100644 index 00000000..392cb4ab --- /dev/null +++ b/.changeset/node-props-one-object.md @@ -0,0 +1,5 @@ +--- +'@plextv/react-lightning': patch +--- + +map element props to node props in one object, without a rest copy diff --git a/packages/react-lightning/src/element/LightningViewElement.nodeProps.spec.ts b/packages/react-lightning/src/element/LightningViewElement.nodeProps.spec.ts new file mode 100644 index 00000000..e49bd6b5 --- /dev/null +++ b/packages/react-lightning/src/element/LightningViewElement.nodeProps.spec.ts @@ -0,0 +1,88 @@ +import type { RendererMain } from '@lightningjs/renderer'; +import type { Fiber } from 'react-reconciler'; +import { describe, expect, it } from 'vitest'; + +import type { LightningViewElementProps, LightningViewElementStyle } from '../types'; +import { LightningViewElement } from './LightningViewElement'; + +function createMockNode(props: Record = {}) { + return { + x: 0, + y: 0, + w: 0, + h: 0, + alpha: 1, + color: 0, + shader: { props: {} }, + parent: null, + on() {}, + off() {}, + animate() { + return { once() {}, start() {} }; + }, + destroy() {}, + ...props, + }; +} + +function createRenderer(created: Record[]) { + return { + createNode: (props: Record) => { + created.push(props); + + return createMockNode(props); + }, + createTextNode: (props: Record) => createMockNode(props), + createShader: () => ({ props: {} }), + createTexture: () => ({}), + destroyNode() {}, + } as unknown as RendererMain; +} + +function createElement(props: Record, created: Record[] = []) { + return new LightningViewElement( + props as unknown as LightningViewElementProps, + createRenderer(created), + [], + {} as Fiber, + ); +} + +describe('LightningViewElement node props', () => { + it('merges style keys after the top-level props, style winning in place', () => { + const el = createElement({ + alpha: 0.5, + testID: 'tile', + style: { color: 3, alpha: 0.25, w: 10 }, + }); + const props = el._toLightningNodeProps(el.props, true); + + expect(Object.keys(props)).toEqual(['alpha', 'testID', 'color', 'w']); + expect(props).toEqual({ alpha: 0.25, testID: 'tile', color: 3, w: 10 }); + }); + + it('drops id, data, style, shader and texture from the top level', () => { + const el = createElement({ id: 'x', data: { a: 1 }, focusable: true, style: { w: 1 } }); + const props = el._toLightningNodeProps(el.props, true) as Record; + + expect(props).toEqual({ focusable: true, w: 1, color: 0 }); + }); + + it('keeps a top-level value when the style value is null', () => { + const el = createElement({ x: 4, style: { x: null } }); + + expect(el._toLightningNodeProps(el.props, true)).toMatchObject({ x: 4 }); + }); + + it('never hands the renderer a data prop', () => { + const created: Record[] = []; + + createElement({ data: { a: 1 }, style: { data: { b: 2 }, w: 5 } }, created); + createElement({ style: { w: 6 } }, created); + + expect(created).toHaveLength(2); + expect('data' in (created[0] as object)).toBe(false); + expect(created[0]).toMatchObject({ w: 5 }); + expect(created[1]).toMatchObject({ w: 6 }); + }); +}); diff --git a/packages/react-lightning/src/element/LightningViewElement.ts b/packages/react-lightning/src/element/LightningViewElement.ts index 2554df37..76c17a78 100644 --- a/packages/react-lightning/src/element/LightningViewElement.ts +++ b/packages/react-lightning/src/element/LightningViewElement.ts @@ -1396,8 +1396,17 @@ export class LightningViewElement< } } - // Don't pass down the `data` prop to the lightning node. - private _createNode({ data: _data, ...props }: Partial): RendererNode { + // Don't pass down the `data` prop to the lightning node. The renderer copies + // the props into its own object, so only a props object carrying `data` needs a copy. + private _createNode(nodeProps: Partial): RendererNode { + let props = nodeProps; + + if ('data' in nodeProps) { + const { data: _data, ...rest } = nodeProps; + + props = rest; + } + const node = this.isTextElement ? this._renderer.createTextNode(props) : this._renderer.createNode(props); @@ -1938,19 +1947,23 @@ export class LightningViewElement< initial = false, ): Partial { // There is no style object in the lightning node, all the style props are on the - // node itself, so we need to destructure the style object into props. - const { - // These props are ignored as they conflict with lightning nodes - id: _id, - data: _data, - // These props require processing and mapping to lightning node props - style, - shader, - texture, - ...otherProps - } = props; - - const finalStyle: Partial = {}; + // node itself, so the style keys are written next to the other props. + const { style, shader, texture } = props; + const finalProps: Partial = {}; + + for (const key in props) { + // id and data conflict with lightning nodes; style, shader and texture + // are mapped below. + if ( + key !== 'id' && + key !== 'data' && + key !== 'style' && + key !== 'shader' && + key !== 'texture' + ) { + (finalProps as Record)[key] = props[key]; + } + } if (style !== undefined && style !== null) { const transition = !initial ? this.props.transition : undefined; @@ -1974,23 +1987,23 @@ export class LightningViewElement< const rect = value as NonNullable; if (!style.w) { - finalStyle.w = rect.w; + finalProps.w = rect.w; } if (!style.h) { - finalStyle.h = rect.h; + finalProps.h = rect.h; } if (!style.x) { - finalStyle.x = rect.x; + finalProps.x = rect.x; } if (!style.y) { - finalStyle.y = rect.y; + finalProps.y = rect.y; } } else { // oxlint-disable-next-line typescript/no-explicit-any -- TODO - (finalStyle as any)[key] = value; + (finalProps as any)[key] = value; } } } @@ -2053,7 +2066,7 @@ export class LightningViewElement< } } } else { - finalStyle.shader = this._renderer.createShader( + finalProps.shader = this._renderer.createShader( this._shaderDef.type, this._shaderDef.props, ); @@ -2063,12 +2076,12 @@ export class LightningViewElement< // has none — clear it so the previous shader stops painting. Setting the // node's shader to null resets it to the stage's default shader. Without // this a removed border would linger on an already-mounted node. - (finalStyle as { shader: INode['shader'] | null }).shader = null; + (finalProps as { shader: INode['shader'] | null }).shader = null; } if (texture && texture !== this._textureDef) { this._textureDef = texture; - finalStyle.texture = createTexture(this._renderer, texture); + finalProps.texture = createTexture(this._renderer, texture); } // borderRadius + clipping (overflow: hidden) = rounded clipping, like the @@ -2088,12 +2101,10 @@ export class LightningViewElement< const clipRadius = (Array.isArray(radius) ? Math.max(...radius) : (radius as number)) || 0; if (initial ? clipRadius > 0 : (this.node.clipRadius ?? 0) !== clipRadius) { - finalStyle.clipRadius = clipRadius; + finalProps.clipRadius = clipRadius; } } - const finalProps = Object.assign(otherProps, finalStyle); - if ( initial === true && this.isImageElement === false && From 037a0bf73392dc3bfb8ae5c0b9907c293c4b02dd Mon Sep 17 00:00:00 2001 From: Ruud Date: Thu, 15 Oct 2026 11:41:02 +0200 Subject: [PATCH 05/17] perf(element): use prototype methods instead of per-instance closures --- .changeset/element-prototype-methods.md | 5 + .../LightningViewElement.events.spec.ts | 156 ++++++++++++++++++ .../src/element/LightningViewElement.ts | 149 ++++++++++------- 3 files changed, 253 insertions(+), 57 deletions(-) create mode 100644 .changeset/element-prototype-methods.md create mode 100644 packages/react-lightning/src/element/LightningViewElement.events.spec.ts diff --git a/.changeset/element-prototype-methods.md b/.changeset/element-prototype-methods.md new file mode 100644 index 00000000..4bf69d9f --- /dev/null +++ b/.changeset/element-prototype-methods.md @@ -0,0 +1,5 @@ +--- +'@plextv/react-lightning': patch +--- + +move per-element event, visibility and style proxy closures to the prototype diff --git a/packages/react-lightning/src/element/LightningViewElement.events.spec.ts b/packages/react-lightning/src/element/LightningViewElement.events.spec.ts new file mode 100644 index 00000000..7948cf2a --- /dev/null +++ b/packages/react-lightning/src/element/LightningViewElement.events.spec.ts @@ -0,0 +1,156 @@ +import type { RendererMain } from '@lightningjs/renderer'; +import type { Fiber } from 'react-reconciler'; +import { describe, expect, it, vi } from 'vitest'; + +import type { LightningViewElementProps, LightningViewElementStyle } from '../types'; +import { AllStyleProps } from './AllStyleProps'; +import { LightningViewElement } from './LightningViewElement'; +import { PARTIAL_STYLE } from './partialStyle'; + +function createMockNode(props: Record = {}) { + return { + x: 0, + y: 0, + w: 0, + h: 0, + alpha: 1, + color: 0, + shader: { props: {} }, + parent: null as unknown, + on() {}, + off() {}, + animate() { + return { once() {}, start() {} }; + }, + destroy() {}, + ...props, + }; +} + +const destroyNode = vi.fn(); +const renderer = { + createNode: (props: Record) => createMockNode(props), + createTextNode: (props: Record) => createMockNode(props), + createShader: () => ({ props: {} }), + createTexture: () => ({}), + destroyNode, +} as unknown as RendererMain; + +function createElement(style: Partial = {}) { + return new LightningViewElement( + { style } as LightningViewElementProps, + renderer, + [], + {} as Fiber, + ); +} + +const flush = () => Promise.resolve(); + +describe('LightningViewElement events', () => { + it('returns a disposer from on that removes the listener', () => { + const el = createElement(); + const calls: unknown[] = []; + const dispose = el.on('layout', (dimensions) => calls.push(dimensions)); + + el.emit('layout', { x: 1, y: 2, w: 3, h: 4 }); + dispose(); + el.emit('layout', { x: 0, y: 0, w: 0, h: 0 }); + + expect(calls).toEqual([{ x: 1, y: 2, w: 3, h: 4 }]); + }); + + it('fires a once listener a single time and its disposer is safe after', () => { + const el = createElement(); + const listener = vi.fn(); + const dispose = el.once('textChanged', listener); + + el.emit('textChanged', el); + el.emit('textChanged', el); + dispose(); + + expect(listener).toHaveBeenCalledTimes(1); + }); + + it('works when the methods are called through the instance from other code', () => { + const el = createElement(); + const listener = vi.fn(); + + el.on('childAdded', listener); + el.insertChild(createElement()); + + expect(listener).toHaveBeenCalledTimes(1); + expect(el.emit('childAdded', el, 0)).toBe(true); + }); +}); + +describe('LightningViewElement deferred removal', () => { + it('destroys the node once the deferred handler signals done', () => { + destroyNode.mockClear(); + + const parent = createElement({ w: 10, h: 10, color: 1 }); + const child = createElement({ w: 5, h: 5, color: 2 }); + let finish: (() => void) | undefined; + + parent.insertChild(child); + parent.deferNodeRemoval = (done) => { + finish = done; + }; + + parent.destroy(); + expect(destroyNode).not.toHaveBeenCalled(); + + finish?.(); + expect(destroyNode).toHaveBeenCalledWith(parent.node); + expect(destroyNode).toHaveBeenCalledWith(child.node); + }); + + it('stops listening to the old target when the target changes', () => { + destroyNode.mockClear(); + + const a = createElement({ w: 1, h: 1, color: 1 }); + const b = createElement({ w: 1, h: 1, color: 1 }); + const el = createElement({ w: 1, h: 1, color: 1 }); + + el.deferTarget = a; + el.deferTarget = b; + a.emit('deferredDestroyComplete'); + + expect(destroyNode).not.toHaveBeenCalled(); + + b.emit('deferredDestroyComplete'); + expect(destroyNode).toHaveBeenCalledWith(el.node); + }); +}); + +describe('LightningViewElement style proxy', () => { + it('stages an imperative set as a partial style push', async () => { + const el = createElement({ w: 10 }); + const spy = vi.spyOn(el, 'setProps'); + + el.style.color = 0xff; + + expect(spy).toHaveBeenCalledTimes(1); + + const pushed = spy.mock.calls[0]?.[0].style as Record; + + expect(pushed.color).toBe(0xff); + expect(pushed[PARTIAL_STYLE]).toBe(true); + expect(el.style.color).toBe(0xff); + + await flush(); + expect(el.node.color).toBe(0xff); + }); + + it('skips a set to the current value and reads staged values first', () => { + const el = createElement({ w: 10 }); + const spy = vi.spyOn(el, 'setProps'); + + el.style.w = 10; + expect(spy).not.toHaveBeenCalled(); + + el.setProps({ style: { w: 20 } }); + expect(el.style.w).toBe(20); + expect((el.style as Record)[AllStyleProps]).toMatchObject({ w: 10 }); + }); +}); diff --git a/packages/react-lightning/src/element/LightningViewElement.ts b/packages/react-lightning/src/element/LightningViewElement.ts index 76c17a78..e466a626 100644 --- a/packages/react-lightning/src/element/LightningViewElement.ts +++ b/packages/react-lightning/src/element/LightningViewElement.ts @@ -72,6 +72,46 @@ function createTexture(renderer: RendererMain, textureDef: TextureDef): Texture let idCounter = 0; +type OnArgs = Parameters['on']>; +type OnceArgs = Parameters['once']>; + +type StyleRecord = Record; + +// Traps live on the prototype; the handler only carries the element. +class StyleProxyHandler implements ProxyHandler { + public constructor(private readonly _element: LightningViewElement) {} + + public get(target: StyleRecord, prop: PropertyKey): unknown { + if (prop === AllStyleProps) { + return target; + } + + const staged = (this._element as unknown as { _stagedUpdates?: { style?: StyleRecord } }) + ._stagedUpdates; + + return staged?.style?.[prop] ?? target[prop]; + } + + public set(target: StyleRecord, prop: PropertyKey, value: unknown): boolean { + const element = this._element; + + if ((element.style as StyleRecord)[prop] === value) { + return true; + } + + target[prop] = value; + + // Single-key imperative set: mark it partial so shader/flex processors + // keep the props this push omits. + const style: StyleRecord = { [prop]: value }; + style[PARTIAL_STYLE] = true; + + element.setProps({ style } as Partial); + + return true; + } +} + // Returned when a requested animation is a proven no-op; satisfies the // controller contract without registering anything with the renderer. const noopAnimationController = { @@ -665,9 +705,18 @@ export class LightningViewElement< } public set deferTarget(value: LightningElement | null) { - this._deferTarget?.off('deferredDestroyComplete', this._destroyFinalize); + if (this._destroyFinalizeHandler) { + this._deferTarget?.off('deferredDestroyComplete', this._destroyFinalizeHandler); + } + this._deferTarget = value; - this._deferTarget?.once('deferredDestroyComplete', this._destroyFinalize); + + if (value) { + value.once( + 'deferredDestroyComplete', + (this._destroyFinalizeHandler ??= () => this._destroyFinalize()), + ); + } // Optimize child iteration for (let i = 0; i < this.children.length; i++) { @@ -907,38 +956,45 @@ export class LightningViewElement< this._eventEmitter.emit('destroy'); } - public on = (...args: Parameters['on']>): (() => void) => { - this._eventEmitter.on(...args); + public on(event: OnArgs[0], listener: OnArgs[1]): () => void { + this._eventEmitter.on(event, listener); - if (args[0] === 'resized') { + if (event === 'resized') { this._reconcileResizeObserving(); } - return () => this.off(...args); - }; - public once = ( - ...args: Parameters['once']> - ): (() => void) => { - this._eventEmitter.once(...args); + return () => this.off(event, listener); + } + + public once(event: OnceArgs[0], listener: OnceArgs[1]): () => void { + this._eventEmitter.once(event, listener); - if (args[0] === 'resized') { + if (event === 'resized') { this._reconcileResizeObserving(); } - return () => this.off(...args); - }; + return () => this.off(event, listener); + } - public off: IEventEmitter['off'] = (...args) => { - const result = this._eventEmitter.off(...args); + public off( + event: EventKey, + listener: LightningElementEvents[EventKey], + ): IEventEmitter { + const result = this._eventEmitter.off(event, listener); - if (args[0] === 'resized') { + if (event === 'resized') { this._reconcileResizeObserving(); } return result; - }; - public emit: IEventEmitter['emit'] = (...args) => - this._eventEmitter.emit(...args); + } + + public emit( + event: EventKey, + ...args: Parameters + ): boolean { + return this._eventEmitter.emit(event, ...args); + } public setLightningNode(node: RendererNode): void { const oldNode = this.node; @@ -1286,7 +1342,7 @@ export class LightningViewElement< this.props.onResize?.(dimensions); } - public recalculateVisibility = (): void => { + public recalculateVisibility(): void { const prevFocusable = this.focusable; const prevVisible = this._visible; @@ -1306,7 +1362,7 @@ export class LightningViewElement< if (currentFocusable !== prevFocusable) { this._eventEmitter.emit('focusableChanged', this, currentFocusable); } - }; + } public animateStyle( key: K, @@ -1367,14 +1423,20 @@ export class LightningViewElement< return `${this._recycled ? '\u267B ' : ''}${this.constructor.name} id=${this.id}${this.focusable ? ' focusable' : ''}${this.visible ? ' visible' : ''}${expanded ? ` props=${JSON.stringify(this.props)}` : ''}`; } - private _destroyFinalize = () => { - this._deferTarget?.off('deferredDestroyComplete', this._destroyFinalize); + // Bound on first use: a deferred removal is rare, so most elements never need it. + private _destroyFinalizeHandler: (() => void) | null = null; + + private _destroyFinalize(): void { + if (this._destroyFinalizeHandler) { + this._deferTarget?.off('deferredDestroyComplete', this._destroyFinalizeHandler); + } + this.node.parent = null; if (!this._flattened) { this._renderer.destroyNode(this.node); } - }; + } private _reconcileResizeObserving(): void { const shouldObserve = this.props.onResize != null || this._eventEmitter.hasListeners('resized'); @@ -1767,7 +1829,7 @@ export class LightningViewElement< this._eventEmitter.emit('textureFailed', ...args); }; - private _onLayout = (dimensions: Rect) => { + private _onLayout(dimensions: Rect): void { this._hasLayout = true; // Reveal a withheld node at its now-correct geometry. A pixel translate @@ -1794,7 +1856,7 @@ export class LightningViewElement< } this.props.onLayout?.(dimensions); - }; + } private _createAnimation( props: Partial, @@ -2120,36 +2182,9 @@ export class LightningViewElement< } // Setting any styling applied to the style attribute on an element - private _styleProxyHandler: ProxyHandler> = { - get: (target, prop) => { - if (prop === AllStyleProps) { - return target; - } - - return ( - this._stagedUpdates?.style?.[prop as keyof LightningViewElementStyle] ?? - target[prop as keyof TStyleProps] - ); - }, - set: (target, prop, value) => { - const key = prop as keyof TStyleProps; - - if (this.style[key] === value) { - return true; - } - - target[key] = value; - - // Single-key imperative set: mark it partial so shader/flex processors - // keep the props this push omits. - const style = { [key]: value } as Partial; - (style as Record)[PARTIAL_STYLE] = true; - - this.setProps({ style } as Partial); - - return true; - }, - }; + private _styleProxyHandler = new StyleProxyHandler( + this as unknown as LightningViewElement, + ) as unknown as ProxyHandler>; private _transformProps(props: Partial) { let transformedProps = props; From 89a6e34a3d8d1110fdaa849a5577457a5ee17be1 Mon Sep 17 00:00:00 2001 From: Ruud Date: Thu, 15 Oct 2026 11:48:20 +0200 Subject: [PATCH 06/17] perf(flexbox): track elements with one shared observer instead of per-element listeners --- .changeset/element-lifecycle-observer.md | 6 + .../src/LightningManager.observer.test.ts | 149 +++++++++ .../plugin-flexbox/src/LightningManager.ts | 306 ++++++++++-------- .../src/element/LightningTextElement.ts | 1 + .../LightningViewElement.events.spec.ts | 70 ++++ .../src/element/LightningViewElement.ts | 58 ++++ .../src/types/LightningElementObserver.ts | 38 +++ packages/react-lightning/src/types/index.ts | 1 + 8 files changed, 501 insertions(+), 128 deletions(-) create mode 100644 .changeset/element-lifecycle-observer.md create mode 100644 packages/plugin-flexbox/src/LightningManager.observer.test.ts create mode 100644 packages/react-lightning/src/types/LightningElementObserver.ts diff --git a/.changeset/element-lifecycle-observer.md b/.changeset/element-lifecycle-observer.md new file mode 100644 index 00000000..c7cb9ad9 --- /dev/null +++ b/.changeset/element-lifecycle-observer.md @@ -0,0 +1,6 @@ +--- +'@plextv/react-lightning': patch +'@plextv/react-lightning-plugin-flexbox': patch +--- + +let plugins observe element lifecycle through one shared observer, and track flexbox elements with it instead of nine listeners per element diff --git a/packages/plugin-flexbox/src/LightningManager.observer.test.ts b/packages/plugin-flexbox/src/LightningManager.observer.test.ts new file mode 100644 index 00000000..69136a93 --- /dev/null +++ b/packages/plugin-flexbox/src/LightningManager.observer.test.ts @@ -0,0 +1,149 @@ +import { describe, expect, it } from 'vitest'; + +import { + type LightningElement, + LightningTextElement, + LightningViewElement, + type LightningViewElementProps, + type Plugin, +} from '@plextv/react-lightning'; + +import { plugin } from './index'; +import { getFlexboxManager } from './manager'; + +function createMockNode(props: Record = {}) { + return { + x: 0, + y: 0, + w: 0, + h: 0, + alpha: 1, + color: 0, + text: '', + shader: { props: {} }, + parent: null as unknown, + on() {}, + off() {}, + animate() { + return { once() {}, start() {} }; + }, + destroy() {}, + ...props, + }; +} + +const renderer = { + createNode: (props: Record) => createMockNode(props), + createTextNode: (props: Record) => createMockNode(props), + createShader: () => ({ props: {} }), + createTexture: () => ({}), + destroyNode() {}, +} as unknown as ConstructorParameters[1]; +const fiber = {} as ConstructorParameters[3]; + +async function setup() { + const flexPlugin = plugin() as Plugin; + + await flexPlugin.init?.(renderer, null as never); + + const plugins = [flexPlugin]; + const view = (style: Record) => + new LightningViewElement({ style } as LightningViewElementProps, renderer, plugins, fiber); + const manager = getFlexboxManager(); + + if (!manager) { + throw new Error('no flexbox manager'); + } + + const yoga = (manager as unknown as { _yogaManager: { flushLayout(): void } })._yogaManager; + const layout = async () => { + await Promise.resolve(); + yoga.flushLayout(); + }; + + return { manager, view, plugins, layout }; +} + +describe('LightningManager with real elements', () => { + it('lays out, reorders and removes children through the element observer', async () => { + const { manager, view, layout } = await setup(); + const root = view({ w: 100, h: 10, flexDirection: 'row' }); + const [a, b, c] = [view({ w: 10, h: 10 }), view({ w: 20, h: 10 }), view({ w: 30, h: 10 })]; + + root.insertChild(a); + root.insertChild(b); + root.insertChild(c); + manager.markFlexRoot(root); + await layout(); + + expect([a.node.x, b.node.x, c.node.x]).toEqual([0, 10, 30]); + + root.insertChild(c, a); + await layout(); + + expect([c.node.x, a.node.x, b.node.x]).toEqual([0, 30, 40]); + + root.removeChild(a); + await layout(); + + expect([c.node.x, b.node.x]).toEqual([0, 30]); + }); + + it('applies style changes and forgets an element once it is destroyed', async () => { + const { manager, view, layout } = await setup(); + const root = view({ w: 100, h: 10, flexDirection: 'row' }); + const a = view({ w: 10, h: 10 }); + const b = view({ w: 10, h: 10 }); + + root.insertChild(a); + root.insertChild(b); + manager.markFlexRoot(root); + await layout(); + + a.setProps({ style: { w: 25, h: 10 } }); + await layout(); + + expect(b.node.x).toBe(25); + + const elements = (manager as unknown as { _elements: Map })._elements; + + expect(elements.has(a.id)).toBe(true); + + root.removeChild(a); + a.destroy(); + + expect(elements.has(a.id)).toBe(false); + + // A queued update landing after destroy must not reach the manager. + a.setProps({ style: { w: 50 } }); + await layout(); + + expect(elements.has(a.id)).toBe(false); + }); + + it('keeps text measurement in sync with text changes', async () => { + const { manager, plugins } = await setup(); + const text = new LightningTextElement( + { text: 'a', style: { fontFamily: 'Unknown' } } as never, + renderer, + plugins, + fiber, + ); + const calls: string[] = []; + const original = (manager as unknown as { _syncTextMeasure(e: LightningElement): void }) + ._syncTextMeasure; + + (manager as unknown as { _syncTextMeasure(e: LightningElement): void })._syncTextMeasure = + function (this: unknown, element: LightningElement) { + calls.push((element as LightningTextElement).text); + original.call(this, element); + }; + + text.text = 'b'; + text.setProps({ style: { fontSize: 30 } }); + await Promise.resolve(); + + // textChanged, then stylesChanged and propsChanged from the update. + expect(calls).toEqual(['b', 'b', 'b']); + }); +}); diff --git a/packages/plugin-flexbox/src/LightningManager.ts b/packages/plugin-flexbox/src/LightningManager.ts index 62ccfb66..27ed7412 100644 --- a/packages/plugin-flexbox/src/LightningManager.ts +++ b/packages/plugin-flexbox/src/LightningManager.ts @@ -1,5 +1,6 @@ import type { LightningElement, + LightningElementObserver, LightningElementStyle, LightningTextElement, LightningTextElementStyle, @@ -329,146 +330,35 @@ export class LightningManager { this._syncTextMeasure(element); } + // Real elements take the shared observer: no per-element listeners or + // disposers. Anything else that looks like an element gets events. + if (typeof (element as { observe?: unknown }).observe === 'function') { + element.observe(this._observer); + + return; + } + const disposers = [ element.on('destroy', () => { for (const dispose of disposers) { dispose(); } - const yogaParent = this._yogaParents.get(element.id); - - if (yogaParent !== undefined) { - this._clearYogaParent(element.id, yogaParent); - } - - this._elements.delete(element.id); - this._boundaries.delete(element.id); - this._flexRoots.delete(element.id); - this._yogaChildCounts.delete(element.id); - this._measuredText.delete(element.id); - this._textContextWidth.delete(element.id); - // oxlint-disable-next-line typescript/no-non-null-assertion -- Guaranteed to exist. But avoiding the nullish operator for perf reasons - this._yogaManager!.applyStyle(element.id, null, true); - // oxlint-disable-next-line typescript/no-non-null-assertion -- Guaranteed to exist. See above - this._yogaManager!.removeIndependentRoot(element.id); - // oxlint-disable-next-line typescript/no-non-null-assertion -- Guaranteed to exist. See above - this._yogaManager!.removeNode(element.id); - }), - - element.on('childAdded', (child, index) => { - if (this._isInBoundary(element)) { - return; - } - - // Translate React index → yoga index. Skipped siblings (boundaries, - // flex roots) cause "memory access out of bounds" otherwise. - const yogaIndex = this._yogaIndexFor(element, index); - - // oxlint-disable-next-line typescript/no-non-null-assertion -- Guaranteed to exist. See above - this._yogaManager!.addChildNode(element.id, child.id, yogaIndex); - this._setYogaParent(child.id, element.id); - // See _reattachChildren — withhold paint until first layout. - child.withholdPaintUntilLayout(); - this.applyStyle(element.id, element.style); - - // React mounts bottom-up: `child`'s descendants were inserted - // before `child` joined the flex tree, so they were skipped at - // their own childAdded time. Promote them now. - if (!this._boundaries.has(child.id)) { - this._reattachChildren(child); - } + this._onDestroy(element); }), - - element.on('childRemoved', (child) => { - const childYogaParent = this._yogaParents.get(child.id); - - if (childYogaParent !== undefined) { - this._clearYogaParent(child.id, childYogaParent); - } - - // oxlint-disable-next-line typescript/no-non-null-assertion -- Guaranteed to exist. See above - this._yogaManager!.applyStyle(child.id, null, true); - // oxlint-disable-next-line typescript/no-non-null-assertion -- Guaranteed to exist. See above - this._yogaManager!.removeNode(child.id); - - // Re-layout — without this a shrink-fit parent keeps the old size - // (node.w/h stay at last computed) and NodeResizeObserver never - // fires the shrink event to consumers like VL.reportItemSize. - // oxlint-disable-next-line typescript/no-non-null-assertion -- Guaranteed to exist. See above - this._yogaManager!.queueRender(element.id); - }), - - // Same-parent reorder (React's keyed move). Reindex in place, keep the yoga node alive. - element.on('childMoved', (child, _fromIndex, toIndex) => { - if (this._yogaParents.get(child.id) !== element.id) { - // Not one of this parent's yoga children (boundary or flex root), nothing to reindex. - return; - } - - // Detach before computing the index: the append-at-end fast path in - // _yogaIndexFor reads the cached count, which must not include the child. - // oxlint-disable-next-line typescript/no-non-null-assertion -- Guaranteed to exist. See above - this._yogaManager!.detachChildNode(element.id, child.id); - this._clearYogaParent(child.id, element.id); - - const yogaIndex = this._yogaIndexFor(element, toIndex); - - // oxlint-disable-next-line typescript/no-non-null-assertion -- Guaranteed to exist. See above - this._yogaManager!.addChildNode(element.id, child.id, yogaIndex); - this._setYogaParent(child.id, element.id); - // oxlint-disable-next-line typescript/no-non-null-assertion -- Guaranteed to exist. See above - this._yogaManager!.queueRender(element.id); - }), - - element.on('inViewport', () => { - if (!element.isTextElement && !element.isImageElement) { - this.applyStyle(element.id, element.props.style); - } - }), - - element.on('stylesChanged', () => { - this.applyStyle(element.id, element.props.style); - - // Font/size/maxLines changes affect measurement. - if (element.isTextElement) { - this._syncTextMeasure(element); - } - }), - + element.on('childAdded', (child, index) => this._onChildAdded(element, child, index)), + element.on('childRemoved', (child) => this._onChildRemoved(element, child)), + element.on('childMoved', (child, _fromIndex, toIndex) => + this._onChildMoved(element, child, toIndex), + ), + element.on('inViewport', () => this._onInViewport(element)), + element.on('stylesChanged', () => this._onStylesChanged(element)), element.on( 'textureLoaded', ( node: RendererNode | TextRendererNode, event: { type: string; dimensions: { w: number; h: number } }, - ) => { - if (element.isTextElement) { - // Static text (set once at mount via the initial-props path, not the - // `text` setter) never fires `textChanged`, so `_syncTextMeasure` - // only ran at trackElement before the content existed and the node - // was never measured. The renderer's texture-loaded event is the - // first point where `node.text` is reliably populated — sync now so - // such text gets a measure func and wraps like dynamic text does. - if (!this._measuredText.has(element.id)) { - this._syncTextMeasure(element); - } - - // When Yoga measures the text itself, it owns the node's size — - // pushing the async texture size back would fight the measure func. - if (this._measuredText.has(element.id)) { - return; - } - - this.applyStyle(element.id, { - w: event.dimensions.w, - h: event.dimensions.h, - }); - } else { - this.applyStyle(element.id, { - w: node.w, - h: node.h, - }); - } - }, + ) => this._onTextureLoaded(element, node, event), ), ]; @@ -488,6 +378,166 @@ export class LightningManager { } } + private _observer: LightningElementObserver = { + destroy: (element) => { + element.unobserve(this._observer); + this._onDestroy(element); + }, + childAdded: (element, child, index) => this._onChildAdded(element, child, index), + childRemoved: (element, child) => this._onChildRemoved(element, child), + childMoved: (element, child, _fromIndex, toIndex) => + this._onChildMoved(element, child, toIndex), + inViewport: (element) => this._onInViewport(element), + stylesChanged: (element) => this._onStylesChanged(element), + textureLoaded: (element, node, event) => this._onTextureLoaded(element, node, event), + // Text only, like the propsChanged/textChanged listeners above. + propsChanged: (element) => { + if (element.isTextElement) { + this._syncTextMeasure(element); + } + }, + textChanged: (element) => { + if (element.isTextElement) { + this._syncTextMeasure(element); + } + }, + }; + + private _onDestroy(element: LightningElement): void { + const yogaParent = this._yogaParents.get(element.id); + + if (yogaParent !== undefined) { + this._clearYogaParent(element.id, yogaParent); + } + + this._elements.delete(element.id); + this._boundaries.delete(element.id); + this._flexRoots.delete(element.id); + this._yogaChildCounts.delete(element.id); + this._measuredText.delete(element.id); + this._textContextWidth.delete(element.id); + // oxlint-disable-next-line typescript/no-non-null-assertion -- Guaranteed to exist. But avoiding the nullish operator for perf reasons + this._yogaManager!.applyStyle(element.id, null, true); + // oxlint-disable-next-line typescript/no-non-null-assertion -- Guaranteed to exist. See above + this._yogaManager!.removeIndependentRoot(element.id); + // oxlint-disable-next-line typescript/no-non-null-assertion -- Guaranteed to exist. See above + this._yogaManager!.removeNode(element.id); + } + + private _onChildAdded(element: LightningElement, child: LightningElement, index: number): void { + if (this._isInBoundary(element)) { + return; + } + + // Translate React index → yoga index. Skipped siblings (boundaries, + // flex roots) cause "memory access out of bounds" otherwise. + const yogaIndex = this._yogaIndexFor(element, index); + + // oxlint-disable-next-line typescript/no-non-null-assertion -- Guaranteed to exist. See above + this._yogaManager!.addChildNode(element.id, child.id, yogaIndex); + this._setYogaParent(child.id, element.id); + // See _reattachChildren — withhold paint until first layout. + child.withholdPaintUntilLayout(); + this.applyStyle(element.id, element.style); + + // React mounts bottom-up: `child`'s descendants were inserted + // before `child` joined the flex tree, so they were skipped at + // their own childAdded time. Promote them now. + if (!this._boundaries.has(child.id)) { + this._reattachChildren(child); + } + } + + private _onChildRemoved(element: LightningElement, child: LightningElement): void { + const childYogaParent = this._yogaParents.get(child.id); + + if (childYogaParent !== undefined) { + this._clearYogaParent(child.id, childYogaParent); + } + + // oxlint-disable-next-line typescript/no-non-null-assertion -- Guaranteed to exist. See above + this._yogaManager!.applyStyle(child.id, null, true); + // oxlint-disable-next-line typescript/no-non-null-assertion -- Guaranteed to exist. See above + this._yogaManager!.removeNode(child.id); + + // Re-layout — without this a shrink-fit parent keeps the old size + // (node.w/h stay at last computed) and NodeResizeObserver never + // fires the shrink event to consumers like VL.reportItemSize. + // oxlint-disable-next-line typescript/no-non-null-assertion -- Guaranteed to exist. See above + this._yogaManager!.queueRender(element.id); + } + + // Same-parent reorder (React's keyed move). Reindex in place, keep the yoga node alive. + private _onChildMoved(element: LightningElement, child: LightningElement, toIndex: number): void { + if (this._yogaParents.get(child.id) !== element.id) { + // Not one of this parent's yoga children (boundary or flex root), nothing to reindex. + return; + } + + // Detach before computing the index: the append-at-end fast path in + // _yogaIndexFor reads the cached count, which must not include the child. + // oxlint-disable-next-line typescript/no-non-null-assertion -- Guaranteed to exist. See above + this._yogaManager!.detachChildNode(element.id, child.id); + this._clearYogaParent(child.id, element.id); + + const yogaIndex = this._yogaIndexFor(element, toIndex); + + // oxlint-disable-next-line typescript/no-non-null-assertion -- Guaranteed to exist. See above + this._yogaManager!.addChildNode(element.id, child.id, yogaIndex); + this._setYogaParent(child.id, element.id); + // oxlint-disable-next-line typescript/no-non-null-assertion -- Guaranteed to exist. See above + this._yogaManager!.queueRender(element.id); + } + + private _onInViewport(element: LightningElement): void { + if (!element.isTextElement && !element.isImageElement) { + this.applyStyle(element.id, element.props.style); + } + } + + private _onStylesChanged(element: LightningElement): void { + this.applyStyle(element.id, element.props.style); + + // Font/size/maxLines changes affect measurement. + if (element.isTextElement) { + this._syncTextMeasure(element); + } + } + + private _onTextureLoaded( + element: LightningElement, + node: RendererNode | TextRendererNode, + event: { type: string; dimensions: { w: number; h: number } }, + ): void { + if (element.isTextElement) { + // Static text (set once at mount via the initial-props path, not the + // `text` setter) never fires `textChanged`, so `_syncTextMeasure` + // only ran at trackElement before the content existed and the node + // was never measured. The renderer's texture-loaded event is the + // first point where `node.text` is reliably populated — sync now so + // such text gets a measure func and wraps like dynamic text does. + if (!this._measuredText.has(element.id)) { + this._syncTextMeasure(element); + } + + // When Yoga measures the text itself, it owns the node's size — + // pushing the async texture size back would fight the measure func. + if (this._measuredText.has(element.id)) { + return; + } + + this.applyStyle(element.id, { + w: event.dimensions.w, + h: event.dimensions.h, + }); + } else { + this.applyStyle(element.id, { + w: node.w, + h: node.h, + }); + } + } + public applyStyle( elementId: number, style?: Partial | null, diff --git a/packages/react-lightning/src/element/LightningTextElement.ts b/packages/react-lightning/src/element/LightningTextElement.ts index 727c473f..6c5c5d56 100644 --- a/packages/react-lightning/src/element/LightningTextElement.ts +++ b/packages/react-lightning/src/element/LightningTextElement.ts @@ -47,6 +47,7 @@ export class LightningTextElement extends LightningViewElement< // on every value change, not just prop changes. if (v !== this._lastEmittedText) { this._lastEmittedText = v; + this._notifyObservers('textChanged'); this.emit('textChanged', this); // A fragment nested more than one level deep (Text > Text > string) diff --git a/packages/react-lightning/src/element/LightningViewElement.events.spec.ts b/packages/react-lightning/src/element/LightningViewElement.events.spec.ts index 7948cf2a..e80f8570 100644 --- a/packages/react-lightning/src/element/LightningViewElement.events.spec.ts +++ b/packages/react-lightning/src/element/LightningViewElement.events.spec.ts @@ -154,3 +154,73 @@ describe('LightningViewElement style proxy', () => { expect((el.style as Record)[AllStyleProps]).toMatchObject({ w: 10 }); }); }); + +describe('LightningViewElement observers', () => { + it('calls observers with the element ahead of the event listeners', () => { + const parent = createElement(); + const child = createElement(); + const calls: string[] = []; + + parent.on('childAdded', () => calls.push('listener')); + parent.observe({ + childAdded: (element, added, index) => { + calls.push(`observer ${element === parent} ${added === child} ${index}`); + }, + }); + + parent.insertChild(child); + + expect(calls).toEqual(['observer true true 0', 'listener']); + }); + + it('reports child moves, removals and style changes', async () => { + const parent = createElement(); + const [a, b] = [createElement(), createElement()]; + const calls: unknown[][] = []; + + parent.insertChild(a); + parent.insertChild(b); + parent.observe({ + childMoved: (_e, child, from, to) => calls.push(['moved', child === b, from, to]), + childRemoved: (_e, child, index) => calls.push(['removed', child === a, index]), + stylesChanged: (_e, styles) => calls.push(['styles', styles]), + propsChanged: () => calls.push(['props']), + }); + + parent.insertChild(b, a); + parent.removeChild(a); + parent.setProps({ style: { w: 3 } }); + await flush(); + + expect(calls).toEqual([ + ['moved', true, 1, 0], + ['removed', true, 1], + ['styles', { w: 3 }], + ['props'], + ]); + }); + + it('lets an observer unobserve while it is being notified', () => { + const el = createElement(); + const calls: string[] = []; + const first = { + destroy: () => { + calls.push('first'); + el.unobserve(first); + }, + }; + const second = { destroy: () => calls.push('second') }; + + el.observe(first); + el.observe(second); + el.destroy(); + + expect(calls).toEqual(['first', 'second']); + + el.unobserve(second); + el.observe(first); + el.unobserve(first); + el.insertChild(createElement()); + expect(calls).toEqual(['first', 'second']); + }); +}); diff --git a/packages/react-lightning/src/element/LightningViewElement.ts b/packages/react-lightning/src/element/LightningViewElement.ts index e466a626..cefef7b2 100644 --- a/packages/react-lightning/src/element/LightningViewElement.ts +++ b/packages/react-lightning/src/element/LightningViewElement.ts @@ -21,6 +21,7 @@ import { type Focusable, type LightningElement, type LightningElementEvents, + type LightningElementObserver, type LightningElementStyle, LightningElementType, type LightningViewElementProps, @@ -72,6 +73,8 @@ function createTexture(renderer: RendererMain, textureDef: TextureDef): Texture let idCounter = 0; +type ObserverCall = (element: LightningElement, a?: unknown, b?: unknown, c?: unknown) => void; + type OnArgs = Parameters['on']>; type OnceArgs = Parameters['once']>; @@ -182,6 +185,7 @@ export class LightningViewElement< private _reactHidden = false; private _withheldAlpha = 1; private _eventEmitter = new EventEmitter(); + private _observers: LightningElementObserver[] | null = null; private _deferTarget: LightningElement | null = null; private _deferNodeRemovalHandler: ((destroy: () => void) => void) | null = null; private _resizeObserver: NodeResizeObserver | null = null; @@ -953,6 +957,7 @@ export class LightningViewElement< delete LightningViewElement.allElements[this.id]; + this._notifyObservers('destroy'); this._eventEmitter.emit('destroy'); } @@ -996,6 +1001,51 @@ export class LightningViewElement< return this._eventEmitter.emit(event, ...args); } + /** Register a lifecycle observer, see {@link LightningElementObserver}. */ + public observe(observer: LightningElementObserver): void { + if (this._observers === null) { + this._observers = [observer]; + } else { + this._observers.push(observer); + } + } + + public unobserve(observer: LightningElementObserver): void { + const observers = this._observers; + + if (observers === null) { + return; + } + + // Replaced rather than spliced: an observer may unobserve while being notified. + if (observers.length === 1) { + if (observers[0] === observer) { + this._observers = null; + } + } else { + const remaining = observers.filter((o) => o !== observer); + + this._observers = remaining.length > 0 ? remaining : null; + } + } + + protected _notifyObservers( + method: keyof LightningElementObserver, + a?: unknown, + b?: unknown, + c?: unknown, + ): void { + const observers = this._observers; + + if (observers === null) { + return; + } + + for (let i = 0; i < observers.length; i++) { + (observers[i]?.[method] as ObserverCall | undefined)?.(this as LightningElement, a, b, c); + } + } + public setLightningNode(node: RendererNode): void { const oldNode = this.node; @@ -1055,6 +1105,7 @@ export class LightningViewElement< child.deferTarget = this._deferTarget; } + this._notifyObservers('childAdded', child, index); this._eventEmitter.emit('childAdded', child, index); } @@ -1079,6 +1130,7 @@ export class LightningViewElement< child._placeInHost(); if (toIndex !== fromIndex) { + this._notifyObservers('childMoved', child, fromIndex, toIndex); this._eventEmitter.emit('childMoved', child, fromIndex, toIndex); } } @@ -1098,6 +1150,7 @@ export class LightningViewElement< child.node.parent = null; } + this._notifyObservers('childRemoved', child, index); this._eventEmitter.emit('childRemoved', child, index); } @@ -1487,6 +1540,7 @@ export class LightningViewElement< const changed = this._doUpdate(); if (changed) { + this._notifyObservers('propsChanged', this.props); this._eventEmitter.emit('propsChanged', this.props); } }); @@ -1636,6 +1690,7 @@ export class LightningViewElement< } if (hasStyleChanges) { + this._notifyObservers('stylesChanged', this.props.style); this._eventEmitter.emit('stylesChanged', this.props.style as Partial); } @@ -1800,6 +1855,7 @@ export class LightningViewElement< this.recalculateVisibility(); } + this._notifyObservers('stylesChanged', this.props.style); this._eventEmitter.emit('stylesChanged', this.props.style as Partial); this._isUpdateQueued = false; @@ -1816,11 +1872,13 @@ export class LightningViewElement< } private _onInViewport: NodeRenderStateEventHandler = (node, renderState) => { + this._notifyObservers('inViewport', node, renderState); this._eventEmitter.emit('inViewport', node, renderState); }; private _onTextureLoaded: NodeLoadedEventHandler = (node, event) => { this._handleTextureLoaded(event); + this._notifyObservers('textureLoaded', node, event); this._eventEmitter.emit('textureLoaded', node, event); this.props.onTextureReady?.(event.dimensions); }; diff --git a/packages/react-lightning/src/types/LightningElementObserver.ts b/packages/react-lightning/src/types/LightningElementObserver.ts new file mode 100644 index 00000000..0a761eae --- /dev/null +++ b/packages/react-lightning/src/types/LightningElementObserver.ts @@ -0,0 +1,38 @@ +import type { NodeLoadedEventHandler, NodeRenderStateEventHandler } from '@lightningjs/renderer'; + +import type { LightningElement } from './Element'; +import type { RendererNode } from './LightningNode'; +import type { LightningElementProps } from './Props'; +import type { LightningElementStyle } from './Styles'; + +/** + * Lifecycle calls an element makes ahead of the matching events, to observers + * registered with `observe`. One observer object can serve every element (the + * element comes first in each call), so a plugin that tracks all elements + * needs no per-element listeners. An observer is called before any listener + * of the same event. + */ +export interface LightningElementObserver { + destroy?(element: LightningElement): void; + childAdded?(element: LightningElement, child: LightningElement, index: number): void; + childRemoved?(element: LightningElement, child: LightningElement, index: number): void; + childMoved?( + element: LightningElement, + child: LightningElement, + fromIndex: number, + toIndex: number, + ): void; + inViewport?( + element: LightningElement, + node: RendererNode, + renderState: Parameters[1], + ): void; + textureLoaded?( + element: LightningElement, + node: RendererNode, + event: Parameters[1], + ): void; + stylesChanged?(element: LightningElement, styles: Partial): void; + propsChanged?(element: LightningElement, props: Partial): void; + textChanged?(element: LightningElement): void; +} diff --git a/packages/react-lightning/src/types/index.ts b/packages/react-lightning/src/types/index.ts index 7a997d42..1218868f 100644 --- a/packages/react-lightning/src/types/index.ts +++ b/packages/react-lightning/src/types/index.ts @@ -5,6 +5,7 @@ export type * from './Geometry'; export type * from './jsx'; export type { KeyEvent } from './KeyEvent'; export type { LightningElementEvents } from './LightningElementEvents'; +export type { LightningElementObserver } from './LightningElementObserver'; export { LightningElementType } from './LightningElementType'; export type { RendererNode, TextRendererNode } from './LightningNode'; export type * from './Props'; From 645ad8f2899ceb62b306b29e65a1cfaaee2159ea Mon Sep 17 00:00:00 2001 From: Ruud Date: Thu, 15 Oct 2026 11:49:56 +0200 Subject: [PATCH 07/17] perf(flexbox): skip style proxy reads in layout readback without a raw position --- .changeset/flexbox-readback-raw-first.md | 5 ++ .../src/LightningManager.applyUpdates.test.ts | 89 +++++++++++++++++++ .../plugin-flexbox/src/LightningManager.ts | 18 ++-- 3 files changed, 103 insertions(+), 9 deletions(-) create mode 100644 .changeset/flexbox-readback-raw-first.md create mode 100644 packages/plugin-flexbox/src/LightningManager.applyUpdates.test.ts diff --git a/.changeset/flexbox-readback-raw-first.md b/.changeset/flexbox-readback-raw-first.md new file mode 100644 index 00000000..b94a46f0 --- /dev/null +++ b/.changeset/flexbox-readback-raw-first.md @@ -0,0 +1,5 @@ +--- +'@plextv/react-lightning-plugin-flexbox': patch +--- + +check the raw style before reading the style proxy when applying layout positions diff --git a/packages/plugin-flexbox/src/LightningManager.applyUpdates.test.ts b/packages/plugin-flexbox/src/LightningManager.applyUpdates.test.ts new file mode 100644 index 00000000..3c838559 --- /dev/null +++ b/packages/plugin-flexbox/src/LightningManager.applyUpdates.test.ts @@ -0,0 +1,89 @@ +import { describe, expect, it } from 'vitest'; + +import type { LightningElement } from '@plextv/react-lightning'; + +import { LightningManager } from './LightningManager'; + +type FakeStyle = Record; + +function fakeElement(id: number, style: FakeStyle, rawStyle: FakeStyle, parentStyle: FakeStyle) { + return { + id, + isTextElement: false, + hasLayout: true, + style, + rawProps: { style: rawStyle }, + parent: { style: parentStyle }, + node: { x: -1, y: -1, w: 0, h: 0 } as Record, + setNodeProp(key: string, value: number) { + if (this.node[key] === value) { + return false; + } + + this.node[key] = value; + + return true; + }, + emit() {}, + emitLayoutEvent() {}, + }; +} + +function applyLayout(elements: ReturnType[], x: number, y: number) { + const manager = new LightningManager(); + const map = (manager as unknown as { _elements: Map })._elements; + const buffer = new ArrayBuffer(20 * elements.length); + const view = new DataView(buffer); + + elements.forEach((el, i) => { + map.set(el.id, el as unknown as LightningElement); + view.setUint32(i * 20, el.id, true); + view.setInt32(i * 20 + 4, x, true); + view.setInt32(i * 20 + 8, y, true); + view.setInt32(i * 20 + 12, 10, true); + view.setInt32(i * 20 + 16, 10, true); + }); + + (manager as unknown as { _applyUpdates(buffer: ArrayBuffer): void })._applyUpdates(buffer); +} + +describe('LightningManager layout readback', () => { + it('keeps an explicitly positioned axis outside a flex parent', () => { + const el = fakeElement(1, { x: 5 }, { x: 5 }, {}); + + applyLayout([el], 40, 50); + + expect(el.node.x).toBe(-1); + expect(el.node.y).toBe(50); + }); + + it('writes the layout position inside a flex parent, under a translate, or without a raw position', () => { + const inFlex = fakeElement(1, { x: 5 }, { x: 5 }, { display: 'flex' }); + const translated = fakeElement(2, { y: 5, transform: { translateY: 3 } }, { y: 5 }, {}); + const styleOnly = fakeElement(3, { x: 5, y: 5 }, {}, {}); + + applyLayout([inFlex, translated, styleOnly], 40, 50); + + expect([inFlex.node.x, inFlex.node.y]).toEqual([40, 50]); + expect([translated.node.x, translated.node.y]).toEqual([40, 50]); + expect([styleOnly.node.x, styleOnly.node.y]).toEqual([40, 50]); + }); + + it('skips the style reads entirely when the raw style has no position', () => { + const reads: PropertyKey[] = []; + const watched = (target: FakeStyle) => + new Proxy(target, { + get(t, key) { + reads.push(key); + + return t[key as string]; + }, + }); + const el = fakeElement(1, watched({ x: 5 }), {}, watched({ display: 'none' })); + + applyLayout([el], 1, 2); + + expect(reads).toEqual([]); + expect([el.node.x, el.node.y]).toEqual([1, 2]); + }); +}); diff --git a/packages/plugin-flexbox/src/LightningManager.ts b/packages/plugin-flexbox/src/LightningManager.ts index 27ed7412..f80de786 100644 --- a/packages/plugin-flexbox/src/LightningManager.ts +++ b/packages/plugin-flexbox/src/LightningManager.ts @@ -584,15 +584,15 @@ export class LightningManager { const isText = el.isTextElement; - if (el.parent?.style.display !== 'flex') { - skipX = - el.style.x !== undefined && - el.rawProps.style?.x !== undefined && - el.style.transform?.translateX === undefined; - skipY = - el.style.y !== undefined && - el.rawProps.style?.y !== undefined && - el.style.transform?.translateY === undefined; + // The raw style check goes first: it's a plain read, and when it fails + // (almost always) none of the style proxy reads below are needed. + const rawStyle = el.rawProps.style; + const rawX = rawStyle?.x !== undefined; + const rawY = rawStyle?.y !== undefined; + + if ((rawX || rawY) && el.parent?.style.display !== 'flex') { + skipX = rawX && el.style.x !== undefined && el.style.transform?.translateX === undefined; + skipY = rawY && el.style.y !== undefined && el.style.transform?.translateY === undefined; } if (!skipX) { From 170bba581381d0d32627ef9e9535a281e67c9f88 Mon Sep 17 00:00:00 2001 From: Ruud Date: Thu, 15 Oct 2026 11:55:30 +0200 Subject: [PATCH 08/17] perf(flexbox): read yoga display only when the style can change it --- .changeset/flexbox-display-read-on-change.md | 5 ++ .../src/YogaManager.hidden.test.ts | 52 +++++++++++++++++++ packages/plugin-flexbox/src/YogaManager.ts | 14 +++-- 3 files changed, 67 insertions(+), 4 deletions(-) create mode 100644 .changeset/flexbox-display-read-on-change.md create mode 100644 packages/plugin-flexbox/src/YogaManager.hidden.test.ts diff --git a/.changeset/flexbox-display-read-on-change.md b/.changeset/flexbox-display-read-on-change.md new file mode 100644 index 00000000..7b332ea0 --- /dev/null +++ b/.changeset/flexbox-display-read-on-change.md @@ -0,0 +1,5 @@ +--- +'@plextv/react-lightning-plugin-flexbox': patch +--- + +only read a node's display back from yoga when the style could have changed it diff --git a/packages/plugin-flexbox/src/YogaManager.hidden.test.ts b/packages/plugin-flexbox/src/YogaManager.hidden.test.ts new file mode 100644 index 00000000..994eacfe --- /dev/null +++ b/packages/plugin-flexbox/src/YogaManager.hidden.test.ts @@ -0,0 +1,52 @@ +import { describe, expect, it } from 'vitest'; + +import { YogaManager } from './YogaManager'; + +async function setup() { + const m = new YogaManager(); + await m.init(); + m.addNode(1); + + const hidden = () => (m as unknown as { _hiddenElements: Set })._hiddenElements.has(1); + + return { m, hidden }; +} + +describe('YogaManager hidden tracking', () => { + it('follows a display key and survives pushes without one', async () => { + const { m, hidden } = await setup(); + + m.applyStyle(1, { display: 'none', w: 10 }, true); + expect(hidden()).toBe(true); + + m.applyStyle(1, { w: 20 }, true); + expect(hidden()).toBe(true); + + m.applyStyle(1, { display: 'flex' }, true); + expect(hidden()).toBe(false); + + m.applyStyle(1, { w: 30 }, true, true); + expect(hidden()).toBe(false); + }); + + it('clears when a full style drops display and resets it', async () => { + const { m, hidden } = await setup(); + + m.applyStyle(1, { display: 'none', w: 10 }, true, true); + expect(hidden()).toBe(true); + + m.applyStyle(1, { w: 10 }, true, true); + expect(hidden()).toBe(false); + }); + + it('keeps a null display from unhiding without a reset', async () => { + const { m, hidden } = await setup(); + + m.applyStyle(1, { display: 'none' }, true); + m.applyStyle(1, { display: null } as never, true); + expect(hidden()).toBe(true); + + m.applyStyle(1, { w: 5 }, true, true); + expect(hidden()).toBe(false); + }); +}); diff --git a/packages/plugin-flexbox/src/YogaManager.ts b/packages/plugin-flexbox/src/YogaManager.ts index 07b871e8..7522acea 100644 --- a/packages/plugin-flexbox/src/YogaManager.ts +++ b/packages/plugin-flexbox/src/YogaManager.ts @@ -488,15 +488,21 @@ export class YogaManager { return; } + // Display only changes through a display key or the reset of one, so the + // hidden set is already right otherwise (and getDisplay is a wasm call). + const mayChangeDisplay = 'display' in style || (resetMissing && 'display' in yogaNode.props); + applyReactPropsToYoga(this._yoga, this._yogaOptions, yogaNode, style, resetMissing); // Yoga zeroes the computed box of a display:none node, so emitting its // layout would drag the node back to the origin — a node parked off screen // by a translate lands in view of anything reading the scene graph. - if (yogaNode.node.getDisplay() === this._yoga.DISPLAY_NONE) { - this._hiddenElements.add(elementId); - } else { - this._hiddenElements.delete(elementId); + if (mayChangeDisplay) { + if (yogaNode.node.getDisplay() === this._yoga.DISPLAY_NONE) { + this._hiddenElements.add(elementId); + } else { + this._hiddenElements.delete(elementId); + } } if (style.transform) { From ec12c3efc4d55b350a69df316c12f4decfb65602 Mon Sep 17 00:00:00 2001 From: Ruud Date: Thu, 15 Oct 2026 11:57:02 +0200 Subject: [PATCH 09/17] perf(element): skip the style proxy for internal style reads and merges --- .changeset/element-style-target-direct.md | 5 ++++ .../LightningViewElement.events.spec.ts | 27 +++++++++++++++++++ .../src/element/LightningViewElement.ts | 21 ++++++++------- 3 files changed, 43 insertions(+), 10 deletions(-) create mode 100644 .changeset/element-style-target-direct.md diff --git a/.changeset/element-style-target-direct.md b/.changeset/element-style-target-direct.md new file mode 100644 index 00000000..d59044f8 --- /dev/null +++ b/.changeset/element-style-target-direct.md @@ -0,0 +1,5 @@ +--- +'@plextv/react-lightning': patch +--- + +read and merge the element's own style target directly instead of through the style proxy diff --git a/packages/react-lightning/src/element/LightningViewElement.events.spec.ts b/packages/react-lightning/src/element/LightningViewElement.events.spec.ts index e80f8570..b6e1f6c2 100644 --- a/packages/react-lightning/src/element/LightningViewElement.events.spec.ts +++ b/packages/react-lightning/src/element/LightningViewElement.events.spec.ts @@ -155,6 +155,33 @@ describe('LightningViewElement style proxy', () => { }); }); +describe('LightningViewElement style target', () => { + it('merges updates into the target the style proxy reads', async () => { + const el = createElement({ w: 10, color: 1 }); + + el.setProps({ style: { w: 20 } }); + await flush(); + + expect((el.style as Record)[AllStyleProps]).toMatchObject({ + w: 20, + color: 1, + }); + }); + + it('starts a fresh target when the style prop is replaced', async () => { + const el = createElement({ w: 10, color: 1 }); + + el.style = {} as LightningViewElementStyle; + el.setProps({ style: { h: 5 } }); + await flush(); + + const target = (el.style as Record)[AllStyleProps]; + + expect(target).toEqual({ h: 5 }); + expect(el.style.color).toBeUndefined(); + }); +}); + describe('LightningViewElement observers', () => { it('calls observers with the element ahead of the event listeners', () => { const parent = createElement(); diff --git a/packages/react-lightning/src/element/LightningViewElement.ts b/packages/react-lightning/src/element/LightningViewElement.ts index cefef7b2..5a19b81a 100644 --- a/packages/react-lightning/src/element/LightningViewElement.ts +++ b/packages/react-lightning/src/element/LightningViewElement.ts @@ -98,7 +98,7 @@ class StyleProxyHandler implements ProxyHandler { public set(target: StyleRecord, prop: PropertyKey, value: unknown): boolean { const element = this._element; - if ((element.style as StyleRecord)[prop] === value) { + if (this.get(target, prop) === value) { return true; } @@ -169,6 +169,8 @@ export class LightningViewElement< protected _stagedUpdates: Partial = {}; private _styleProxy: Partial; + // The proxy's target, so internal reads and merges skip the proxy traps. + private _styleTarget: Partial; private _isUpdateQueued = false; private _shaderDef?: ShaderDef; private _textureDef?: TextureDef; @@ -683,7 +685,8 @@ export class LightningViewElement< public set style(_style: TStyleProps) { // If you set the style prop, it creates a new CSSStyleDeclaration object. // We'll create a new proxy and reset the styles. - this._styleProxy = new Proxy({}, this._styleProxyHandler); + this._styleTarget = {}; + this._styleProxy = new Proxy(this._styleTarget, this._styleProxyHandler); this.setProps({ style: {}, @@ -851,7 +854,8 @@ export class LightningViewElement< const lngProps = this._toLightningNodeProps(this.props, true); - this._styleProxy = new Proxy(this.props.style ?? {}, this._styleProxyHandler); + this._styleTarget = this.props.style ?? {}; + this._styleProxy = new Proxy(this._styleTarget, this._styleProxyHandler); if (import.meta.env.DEV) { __checkProps(Object.keys(lngProps)); @@ -1672,8 +1676,7 @@ export class LightningViewElement< this._refreshFlattenedChildren(false); } - // oxlint-disable-next-line typescript/no-explicit-any -- Required for accessing AllStyleProps symbol - Object.assign((this.style as any)[AllStyleProps], this.props.style); + Object.assign(this._styleTarget, this.props.style); if (previousOpacity !== this.node.alpha) { this.recalculateVisibility(); @@ -1833,8 +1836,7 @@ export class LightningViewElement< } } - // oxlint-disable-next-line typescript/no-explicit-any -- Required for accessing AllStyleProps symbol - Object.assign((this.style as any)[AllStyleProps], style); + Object.assign(this._styleTarget, style); // A direct `clipping` change must keep rounded clipping in sync. Shader // props force the slow path, so the shader is stable here. @@ -2137,8 +2139,7 @@ export class LightningViewElement< // lone borderColor / outlineColor would rebuild the shader without a width. const shaderStyle = isPartialStyle ? ({ - // oxlint-disable-next-line typescript/no-explicit-any -- Required for accessing AllStyleProps symbol - ...((this.style as any)[AllStyleProps] as TStyleProps), + ...(this._styleTarget as TStyleProps), ...style, } as TStyleProps) : style; @@ -2211,7 +2212,7 @@ export class LightningViewElement< // At mount the style proxy doesn't exist yet; the passed style is full. const effectiveClipping = initial ? style?.clipping - : (style?.clipping ?? this.style.clipping); + : (style?.clipping ?? this._stagedUpdates.style?.clipping ?? this._styleTarget.clipping); const radius = effectiveClipping === true && (shaderType === 'Rounded' || shaderType === 'RoundedWithBorder') From 4b2d6d96cc39e162c09fcb63ec3d53c35b05564d Mon Sep 17 00:00:00 2001 From: Ruud Date: Thu, 15 Oct 2026 12:03:07 +0200 Subject: [PATCH 10/17] perf(element): skip unchanged alpha and clipping writes to the node --- .changeset/skip-unchanged-alpha-clipping.md | 5 + .../src/element/LightningViewElement.ts | 20 +++- ...ightningViewElement.unguardedProps.spec.ts | 97 +++++++++++++++++++ 3 files changed, 120 insertions(+), 2 deletions(-) create mode 100644 .changeset/skip-unchanged-alpha-clipping.md create mode 100644 packages/react-lightning/src/element/LightningViewElement.unguardedProps.spec.ts diff --git a/.changeset/skip-unchanged-alpha-clipping.md b/.changeset/skip-unchanged-alpha-clipping.md new file mode 100644 index 00000000..7f554f2f --- /dev/null +++ b/.changeset/skip-unchanged-alpha-clipping.md @@ -0,0 +1,5 @@ +--- +'@plextv/react-lightning': patch +--- + +skip rewriting an unchanged alpha or clipping on the node, which made the renderer recompute the whole subtree diff --git a/packages/react-lightning/src/element/LightningViewElement.ts b/packages/react-lightning/src/element/LightningViewElement.ts index 5a19b81a..a664a638 100644 --- a/packages/react-lightning/src/element/LightningViewElement.ts +++ b/packages/react-lightning/src/element/LightningViewElement.ts @@ -73,6 +73,13 @@ function createTexture(renderer: RendererMain, textureDef: TextureDef): Texture let idCounter = 0; +// The renderer's alpha and clipping setters don't skip an unchanged value, and +// each write re-propagates world alpha or clipping through the whole subtree on +// the next frame. Every full update rewrites them, so compare first. +function isUnguardedNodeProp(key: string): boolean { + return key === 'alpha' || key === 'clipping'; +} + type ObserverCall = (element: LightningElement, a?: unknown, b?: unknown, c?: unknown) => void; type OnArgs = Parameters['on']>; @@ -1669,7 +1676,13 @@ export class LightningViewElement< continue; } - node[key] = lngProps[key]; + const value = lngProps[key]; + + if (isUnguardedNodeProp(key) && node[key] === value) { + continue; + } + + node[key] = value; } if (flattenedMoved) { @@ -1830,7 +1843,10 @@ export class LightningViewElement< typeof value === 'number' ) { this._writeAxis(key as 'x' | 'y', value); - } else { + } else if ( + !isUnguardedNodeProp(key) || + (this.node as unknown as Record)[key] !== value + ) { // oxlint-disable-next-line typescript/no-explicit-any -- direct node property assignment (this.node as any)[key] = value; } diff --git a/packages/react-lightning/src/element/LightningViewElement.unguardedProps.spec.ts b/packages/react-lightning/src/element/LightningViewElement.unguardedProps.spec.ts new file mode 100644 index 00000000..5d1612fd --- /dev/null +++ b/packages/react-lightning/src/element/LightningViewElement.unguardedProps.spec.ts @@ -0,0 +1,97 @@ +import type { RendererMain } from '@lightningjs/renderer'; +import type { Fiber } from 'react-reconciler'; +import { describe, expect, it } from 'vitest'; + +import type { LightningViewElementProps, LightningViewElementStyle } from '../types'; +import { LightningViewElement } from './LightningViewElement'; + +// Like the renderer's CoreNode: alpha and clipping setters always dirty the +// node, so every write counts. +class CountingNode { + public x = 0; + public y = 0; + public w = 0; + public h = 0; + public color = 0; + public shader = { props: {} }; + public parent: unknown = null; + public writes: string[] = []; + private _alpha = 1; + private _clipping = false; + + public constructor(props: Record) { + Object.assign(this, props); + this.writes.length = 0; + } + + public get alpha() { + return this._alpha; + } + + public set alpha(value: number) { + this.writes.push(`alpha=${value}`); + this._alpha = value; + } + + public get clipping() { + return this._clipping; + } + + public set clipping(value: boolean) { + this.writes.push(`clipping=${value}`); + this._clipping = value; + } + + public on() {} + public off() {} + public animate() { + return { once() {}, start() {} }; + } + public destroy() {} +} + +const renderer = { + createNode: (props: Record) => new CountingNode(props), + createTextNode: (props: Record) => new CountingNode(props), + createShader: () => ({ props: {} }), + createTexture: () => ({}), + destroyNode() {}, +} as unknown as RendererMain; + +function createElement(props: Record) { + return new LightningViewElement( + props as unknown as LightningViewElementProps, + renderer, + [], + {} as Fiber, + ); +} + +const flush = () => Promise.resolve(); +const writes = (el: LightningViewElement) => (el.node as unknown as CountingNode).writes; + +describe('LightningViewElement unguarded node props', () => { + it('does not rewrite an unchanged alpha or clipping on a full update', async () => { + const el = createElement({ testID: 'a', style: { alpha: 0.5, clipping: true, w: 10 } }); + + el.setProps({ testID: 'b' }); + await flush(); + + expect(writes(el)).toEqual([]); + expect(el.node.alpha).toBe(0.5); + expect(el.node.clipping).toBe(true); + }); + + it('still writes them when they change, on the full and the style-only path', async () => { + const el = createElement({ testID: 'a', style: { alpha: 0.5, clipping: true, w: 10 } }); + + el.setProps({ testID: 'b', style: { alpha: 1, clipping: false, w: 10 } }); + await flush(); + expect(writes(el)).toEqual(['alpha=1', 'clipping=false']); + + el.setProps({ style: { alpha: 1, clipping: true } }); + await flush(); + expect(writes(el)).toEqual(['alpha=1', 'clipping=false', 'clipping=true']); + expect(el.node.alpha).toBe(1); + }); +}); From b44eab2c87105254ddb4882f5274357b5b470e1a Mon Sep 17 00:00:00 2001 From: Ruud Date: Thu, 15 Oct 2026 12:06:34 +0200 Subject: [PATCH 11/17] perf(element): create the event emitter on first subscription --- .changeset/lazy-element-emitter.md | 5 ++ .../LightningViewElement.events.spec.ts | 24 ++++++++ .../src/element/LightningViewElement.ts | 58 +++++++++++-------- 3 files changed, 63 insertions(+), 24 deletions(-) create mode 100644 .changeset/lazy-element-emitter.md diff --git a/.changeset/lazy-element-emitter.md b/.changeset/lazy-element-emitter.md new file mode 100644 index 00000000..33a212b3 --- /dev/null +++ b/.changeset/lazy-element-emitter.md @@ -0,0 +1,5 @@ +--- +'@plextv/react-lightning': patch +--- + +create an element's event emitter on its first subscription diff --git a/packages/react-lightning/src/element/LightningViewElement.events.spec.ts b/packages/react-lightning/src/element/LightningViewElement.events.spec.ts index b6e1f6c2..549e77dd 100644 --- a/packages/react-lightning/src/element/LightningViewElement.events.spec.ts +++ b/packages/react-lightning/src/element/LightningViewElement.events.spec.ts @@ -84,6 +84,30 @@ describe('LightningViewElement events', () => { }); }); +describe('LightningViewElement lazy event emitter', () => { + it('emits, removes and reports nothing before anything subscribes', () => { + const el = createElement(); + + expect(el.emit('layout', { x: 0, y: 0, w: 0, h: 0 })).toBe(false); + expect(() => el.off('layout', () => {})).not.toThrow(); + }); + + it('delivers internal events once a listener subscribes later', () => { + const el = createElement(); + const added: unknown[] = []; + + el.insertChild(createElement()); + el.on('childAdded', (child) => added.push(child)); + + const second = createElement(); + + el.insertChild(second); + + expect(added).toEqual([second]); + expect(el.emit('childAdded', second, 1)).toBe(true); + }); +}); + describe('LightningViewElement deferred removal', () => { it('destroys the node once the deferred handler signals done', () => { destroyNode.mockClear(); diff --git a/packages/react-lightning/src/element/LightningViewElement.ts b/packages/react-lightning/src/element/LightningViewElement.ts index a664a638..d337dc29 100644 --- a/packages/react-lightning/src/element/LightningViewElement.ts +++ b/packages/react-lightning/src/element/LightningViewElement.ts @@ -193,7 +193,8 @@ export class LightningViewElement< private _unsettledLayouts = 0; private _reactHidden = false; private _withheldAlpha = 1; - private _eventEmitter = new EventEmitter(); + // Created on the first subscription: most elements are never listened to. + private _eventEmitter: EventEmitter | null = null; private _observers: LightningElementObserver[] | null = null; private _deferTarget: LightningElement | null = null; private _deferNodeRemovalHandler: ((destroy: () => void) => void) | null = null; @@ -627,7 +628,7 @@ export class LightningViewElement< this.blur(); } - this._eventEmitter.emit('focusableChanged', this, this._focusable); + this._eventEmitter?.emit('focusableChanged', this, this._focusable); } public get focused(): boolean { @@ -902,7 +903,7 @@ export class LightningViewElement< this._reconcileResizeObserving(); } - this._eventEmitter.emit('initialized'); + this._eventEmitter?.emit('initialized'); } /** @@ -969,11 +970,11 @@ export class LightningViewElement< delete LightningViewElement.allElements[this.id]; this._notifyObservers('destroy'); - this._eventEmitter.emit('destroy'); + this._eventEmitter?.emit('destroy'); } public on(event: OnArgs[0], listener: OnArgs[1]): () => void { - this._eventEmitter.on(event, listener); + this._getEventEmitter().on(event, listener); if (event === 'resized') { this._reconcileResizeObserving(); @@ -983,7 +984,7 @@ export class LightningViewElement< } public once(event: OnceArgs[0], listener: OnceArgs[1]): () => void { - this._eventEmitter.once(event, listener); + this._getEventEmitter().once(event, listener); if (event === 'resized') { this._reconcileResizeObserving(); @@ -996,7 +997,7 @@ export class LightningViewElement< event: EventKey, listener: LightningElementEvents[EventKey], ): IEventEmitter { - const result = this._eventEmitter.off(event, listener); + const result = this._getEventEmitter().off(event, listener); if (event === 'resized') { this._reconcileResizeObserving(); @@ -1009,7 +1010,15 @@ export class LightningViewElement< event: EventKey, ...args: Parameters ): boolean { - return this._eventEmitter.emit(event, ...args); + return this._eventEmitter?.emit(event, ...args) ?? false; + } + + private _getEventEmitter(): EventEmitter { + if (this._eventEmitter === null) { + this._eventEmitter = new EventEmitter(); + } + + return this._eventEmitter; } /** Register a lifecycle observer, see {@link LightningElementObserver}. */ @@ -1117,7 +1126,7 @@ export class LightningViewElement< } this._notifyObservers('childAdded', child, index); - this._eventEmitter.emit('childAdded', child, index); + this._eventEmitter?.emit('childAdded', child, index); } private _moveChild(child: LightningElement, beforeChild?: LightningElement | null): void { @@ -1142,7 +1151,7 @@ export class LightningViewElement< if (toIndex !== fromIndex) { this._notifyObservers('childMoved', child, fromIndex, toIndex); - this._eventEmitter.emit('childMoved', child, fromIndex, toIndex); + this._eventEmitter?.emit('childMoved', child, fromIndex, toIndex); } } @@ -1162,14 +1171,14 @@ export class LightningViewElement< } this._notifyObservers('childRemoved', child, index); - this._eventEmitter.emit('childRemoved', child, index); + this._eventEmitter?.emit('childRemoved', child, index); } public focus(): void { if (!this._focused) { this.props.onFocusCapture?.(this); this._focused = true; - this._eventEmitter.emit('focusChanged', this, true); + this._eventEmitter?.emit('focusChanged', this, true); this.props.onFocus?.(this); } } @@ -1177,7 +1186,7 @@ export class LightningViewElement< public blur(): void { if (this._focused) { this._focused = false; - this._eventEmitter.emit('focusChanged', this, false); + this._eventEmitter?.emit('focusChanged', this, false); this.props.onBlur?.(this); } } @@ -1402,7 +1411,7 @@ export class LightningViewElement< * Not intended to be called by application code. */ public _emitResize(dimensions: { w: number; h: number }): void { - this._eventEmitter.emit('resized', this, dimensions); + this._eventEmitter?.emit('resized', this, dimensions); this.props.onResize?.(dimensions); } @@ -1413,7 +1422,7 @@ export class LightningViewElement< this._visible = this.node.alpha > 0 && (!this.parent || this.parent.visible); if (this._visible !== prevVisible) { - this._eventEmitter.emit('visibilityChanged', this._visible); + this._eventEmitter?.emit('visibilityChanged', this._visible); // Optimize child iteration - use traditional for loop for better performance for (let i = 0; i < this.children.length; i++) { @@ -1424,7 +1433,7 @@ export class LightningViewElement< const currentFocusable = this.focusable; if (currentFocusable !== prevFocusable) { - this._eventEmitter.emit('focusableChanged', this, currentFocusable); + this._eventEmitter?.emit('focusableChanged', this, currentFocusable); } } @@ -1503,7 +1512,8 @@ export class LightningViewElement< } private _reconcileResizeObserving(): void { - const shouldObserve = this.props.onResize != null || this._eventEmitter.hasListeners('resized'); + const shouldObserve = + this.props.onResize != null || this._eventEmitter?.hasListeners('resized'); if (shouldObserve === this._isObservingResize) { return; @@ -1552,7 +1562,7 @@ export class LightningViewElement< if (changed) { this._notifyObservers('propsChanged', this.props); - this._eventEmitter.emit('propsChanged', this.props); + this._eventEmitter?.emit('propsChanged', this.props); } }); } @@ -1707,7 +1717,7 @@ export class LightningViewElement< if (hasStyleChanges) { this._notifyObservers('stylesChanged', this.props.style); - this._eventEmitter.emit('stylesChanged', this.props.style as Partial); + this._eventEmitter?.emit('stylesChanged', this.props.style as Partial); } this._isUpdateQueued = false; @@ -1874,7 +1884,7 @@ export class LightningViewElement< } this._notifyObservers('stylesChanged', this.props.style); - this._eventEmitter.emit('stylesChanged', this.props.style as Partial); + this._eventEmitter?.emit('stylesChanged', this.props.style as Partial); this._isUpdateQueued = false; @@ -1891,18 +1901,18 @@ export class LightningViewElement< private _onInViewport: NodeRenderStateEventHandler = (node, renderState) => { this._notifyObservers('inViewport', node, renderState); - this._eventEmitter.emit('inViewport', node, renderState); + this._eventEmitter?.emit('inViewport', node, renderState); }; private _onTextureLoaded: NodeLoadedEventHandler = (node, event) => { this._handleTextureLoaded(event); this._notifyObservers('textureLoaded', node, event); - this._eventEmitter.emit('textureLoaded', node, event); + this._eventEmitter?.emit('textureLoaded', node, event); this.props.onTextureReady?.(event.dimensions); }; private _onTextureFailed: NodeFailedEventHandler = (...args) => { - this._eventEmitter.emit('textureFailed', ...args); + this._eventEmitter?.emit('textureFailed', ...args); }; private _onLayout(dimensions: Rect): void { @@ -1943,7 +1953,7 @@ export class LightningViewElement< const animation = this.node.animate(props, animationSettings || {}); animation.once('stopped', (controller) => { - this._eventEmitter.emit('animationFinished', controller); + this._eventEmitter?.emit('animationFinished', controller); if (styleKey !== undefined) { this.props.onTransitionEnd?.({ From b8bad41e5c60456fcb9ffe6bb35488b01db94ba3 Mon Sep 17 00:00:00 2001 From: Ruud Date: Thu, 15 Oct 2026 12:09:36 +0200 Subject: [PATCH 12/17] perf(react-native): track first layout with a shared observer instead of a listener per element --- .changeset/rn-loaded-layout-observer.md | 6 ++ .../LightningViewElement.events.spec.ts | 11 +++ .../src/element/LightningViewElement.ts | 1 + .../src/types/LightningElementObserver.ts | 5 +- .../reactNativePolyfillsPlugin.test.ts | 99 +++++++++++++++++++ .../src/plugins/reactNativePolyfillsPlugin.ts | 20 ++++ 6 files changed, 141 insertions(+), 1 deletion(-) create mode 100644 .changeset/rn-loaded-layout-observer.md create mode 100644 packages/react-native-lightning/src/plugins/reactNativePolyfillsPlugin.test.ts diff --git a/.changeset/rn-loaded-layout-observer.md b/.changeset/rn-loaded-layout-observer.md new file mode 100644 index 00000000..bd015b4a --- /dev/null +++ b/.changeset/rn-loaded-layout-observer.md @@ -0,0 +1,6 @@ +--- +'@plextv/react-lightning': patch +'@plextv/react-native-lightning': patch +--- + +mark react-native elements loaded through a shared layout observer instead of a once listener per element diff --git a/packages/react-lightning/src/element/LightningViewElement.events.spec.ts b/packages/react-lightning/src/element/LightningViewElement.events.spec.ts index 549e77dd..b7dd8a2c 100644 --- a/packages/react-lightning/src/element/LightningViewElement.events.spec.ts +++ b/packages/react-lightning/src/element/LightningViewElement.events.spec.ts @@ -251,6 +251,17 @@ describe('LightningViewElement observers', () => { ]); }); + it('calls the layout observer after the layout listeners', () => { + const el = createElement({ w: 4, h: 5 }); + const calls: string[] = []; + + el.on('layout', () => calls.push('listener')); + el.observe({ layout: (_e, dimensions) => calls.push(`observer ${dimensions.w}`) }); + el.emitLayoutEvent(); + + expect(calls).toEqual(['listener', 'observer 4']); + }); + it('lets an observer unobserve while it is being notified', () => { const el = createElement(); const calls: string[] = []; diff --git a/packages/react-lightning/src/element/LightningViewElement.ts b/packages/react-lightning/src/element/LightningViewElement.ts index d337dc29..706b731d 100644 --- a/packages/react-lightning/src/element/LightningViewElement.ts +++ b/packages/react-lightning/src/element/LightningViewElement.ts @@ -1401,6 +1401,7 @@ export class LightningViewElement< }; this.emit('layout', dimensions); + this._notifyObservers('layout', dimensions); this._onLayout(dimensions); } diff --git a/packages/react-lightning/src/types/LightningElementObserver.ts b/packages/react-lightning/src/types/LightningElementObserver.ts index 0a761eae..ecfcb944 100644 --- a/packages/react-lightning/src/types/LightningElementObserver.ts +++ b/packages/react-lightning/src/types/LightningElementObserver.ts @@ -1,6 +1,7 @@ import type { NodeLoadedEventHandler, NodeRenderStateEventHandler } from '@lightningjs/renderer'; import type { LightningElement } from './Element'; +import type { Rect } from './Geometry'; import type { RendererNode } from './LightningNode'; import type { LightningElementProps } from './Props'; import type { LightningElementStyle } from './Styles'; @@ -10,7 +11,7 @@ import type { LightningElementStyle } from './Styles'; * registered with `observe`. One observer object can serve every element (the * element comes first in each call), so a plugin that tracks all elements * needs no per-element listeners. An observer is called before any listener - * of the same event. + * of the same event, except `layout`, which runs after the layout listeners. */ export interface LightningElementObserver { destroy?(element: LightningElement): void; @@ -35,4 +36,6 @@ export interface LightningElementObserver { stylesChanged?(element: LightningElement, styles: Partial): void; propsChanged?(element: LightningElement, props: Partial): void; textChanged?(element: LightningElement): void; + /** After the `layout` listeners, before the onLayout prop. */ + layout?(element: LightningElement, dimensions: Rect): void; } diff --git a/packages/react-native-lightning/src/plugins/reactNativePolyfillsPlugin.test.ts b/packages/react-native-lightning/src/plugins/reactNativePolyfillsPlugin.test.ts new file mode 100644 index 00000000..0007c227 --- /dev/null +++ b/packages/react-native-lightning/src/plugins/reactNativePolyfillsPlugin.test.ts @@ -0,0 +1,99 @@ +import { describe, expect, it } from 'vitest'; + +import { LightningViewElement, type LightningViewElementProps } from '@plextv/react-lightning'; + +import { reactNativePolyfillsPlugin } from './reactNativePolyfillsPlugin'; + +type LoadedElement = LightningViewElement & { + __loaded?: boolean; + measure(callback: (...args: number[]) => void): void; +}; + +function createMockNode(props: Record = {}) { + return { + x: 0, + y: 0, + w: 0, + h: 0, + alpha: 1, + color: 0, + shader: { props: {} }, + parent: null as unknown, + on() {}, + off() {}, + animate() { + return { once() {}, start() {} }; + }, + destroy() {}, + ...props, + }; +} + +const renderer = { + createNode: (props: Record) => createMockNode(props), + createTextNode: (props: Record) => createMockNode(props), + createShader: () => ({ props: {} }), + createTexture: () => ({}), + destroyNode() {}, +} as unknown as ConstructorParameters[1]; +const fiber = {} as ConstructorParameters[3]; + +async function setup() { + const plugin = reactNativePolyfillsPlugin(); + + await plugin.init?.(renderer, null as never); + + return (style: Record) => + new LightningViewElement( + { style } as LightningViewElementProps, + renderer, + [plugin], + fiber, + ) as LoadedElement; +} + +describe('reactNativePolyfillsPlugin load tracking', () => { + it('marks an element loaded on its first layout, after the layout listeners', async () => { + const create = await setup(); + const el = create({ w: 10, h: 10 }); + const seen: (boolean | undefined)[] = []; + + el.on('layout', () => seen.push(el.__loaded)); + expect(el.__loaded).toBeUndefined(); + + el.emitLayoutEvent(); + el.emitLayoutEvent(); + + expect(seen).toEqual([undefined, true]); + expect(el.__loaded).toBe(true); + }); + + it('answers a measure made before the first layout once it lands', async () => { + const create = await setup(); + const el = create({ w: 10, h: 20 }); + const calls: number[][] = []; + + el.measure((...args) => calls.push(args)); + await Promise.resolve(); + expect(calls).toEqual([]); + + el.emitLayoutEvent(); + await Promise.resolve(); + await Promise.resolve(); + + expect(calls).toHaveLength(1); + }); + + it('falls back to a layout listener for elements without observers', async () => { + const plugin = reactNativePolyfillsPlugin(); + const listeners: (() => void)[] = []; + const fake = { + once: (_event: string, listener: () => void) => listeners.push(listener), + } as unknown as { __loaded?: boolean }; + + plugin.onCreateInstance?.(fake as never, {} as never, fiber); + listeners.forEach((listener) => listener()); + + expect(fake.__loaded).toBe(true); + }); +}); diff --git a/packages/react-native-lightning/src/plugins/reactNativePolyfillsPlugin.ts b/packages/react-native-lightning/src/plugins/reactNativePolyfillsPlugin.ts index 70f4e4d4..84534ed7 100644 --- a/packages/react-native-lightning/src/plugins/reactNativePolyfillsPlugin.ts +++ b/packages/react-native-lightning/src/plugins/reactNativePolyfillsPlugin.ts @@ -5,6 +5,7 @@ import type { Fiber } from 'react-reconciler'; import { type FocusManager, getFocusManager, + type LightningElementObserver, LightningViewElement, type Plugin, } from '@plextv/react-lightning'; @@ -53,6 +54,18 @@ function whenLoaded(element: LightningNativeViewElement): Promise { let polyfilled = false; +// Marks an element loaded on its first layout, like a `once('layout')` would. +const loadedObserver: LightningElementObserver = { + layout(element) { + const nativeInstance = element as LightningNativeViewElement; + + if (!nativeInstance.__loaded) { + nativeInstance.__loaded = true; + nativeInstance.__resolveLoad?.(); + } + }, +}; + export const reactNativePolyfillsPlugin = (): Plugin => { let fiberFocusManager: FocusManager | null = null; @@ -173,6 +186,13 @@ export const reactNativePolyfillsPlugin = (): Plugin => { onCreateInstance(instance) { const nativeInstance = instance as LightningNativeViewElement; + // One shared observer for every element instead of a once listener each. + if (typeof (nativeInstance as { observe?: unknown }).observe === 'function') { + nativeInstance.observe(loadedObserver); + + return; + } + nativeInstance.once('layout', () => { nativeInstance.__loaded = true; nativeInstance.__resolveLoad?.(); From b7312a4c3732598a849cbf17ed7a76a5dae20395 Mon Sep 17 00:00:00 2001 From: Ruud Date: Thu, 15 Oct 2026 13:05:06 +0200 Subject: [PATCH 13/17] perf(element): let the classname plugin skip style-only updates it has nothing to merge into --- .changeset/classname-style-fast-path.md | 6 + .../LightningViewElement.fastPath.spec.ts | 140 ++++++++++++++++ .../src/element/LightningViewElement.ts | 24 ++- packages/react-lightning/src/render/Plugin.ts | 9 ++ .../cssClassNameTransformPlugin.test.ts | 150 +++++++++++++++++- .../plugins/cssClassNameTransformPlugin.ts | 20 ++- 6 files changed, 342 insertions(+), 7 deletions(-) create mode 100644 .changeset/classname-style-fast-path.md create mode 100644 packages/react-lightning/src/element/LightningViewElement.fastPath.spec.ts diff --git a/.changeset/classname-style-fast-path.md b/.changeset/classname-style-fast-path.md new file mode 100644 index 00000000..24ae7186 --- /dev/null +++ b/.changeset/classname-style-fast-path.md @@ -0,0 +1,6 @@ +--- +'@plextv/react-lightning': patch +'@plextv/react-native-lightning': patch +--- + +add an optional `needsTransform` plugin hook so a plugin can let style-only updates take the fast path per instance, and use it in the className plugin: style-only updates on elements without resolved class styles no longer go through the full transform pipeline diff --git a/packages/react-lightning/src/element/LightningViewElement.fastPath.spec.ts b/packages/react-lightning/src/element/LightningViewElement.fastPath.spec.ts new file mode 100644 index 00000000..b896acee --- /dev/null +++ b/packages/react-lightning/src/element/LightningViewElement.fastPath.spec.ts @@ -0,0 +1,140 @@ +import type { RendererMain } from '@lightningjs/renderer'; +import type { Fiber } from 'react-reconciler'; +import { describe, expect, it, vi } from 'vitest'; + +import type { Plugin } from '../render/Plugin'; +import type { + LightningElement, + LightningViewElementProps, + LightningViewElementStyle, +} from '../types'; +import { LightningViewElement } from './LightningViewElement'; + +const makeNode = (props: Record) => ({ + ...props, + on() {}, + off() {}, + emit() {}, + animate: () => ({ once() {}, start() {} }), + destroy() {}, +}); + +const renderer = { + createNode: makeNode, + createTextNode: makeNode, + createShader: () => ({ props: {} }), + createTexture: () => ({}), + destroyNode() {}, +} as unknown as RendererMain; + +function createElement(plugins: Plugin[]) { + return new LightningViewElement( + { style: { w: 10, h: 10 } } as unknown as LightningViewElementProps, + renderer, + plugins, + {} as Fiber, + ); +} + +function spyPlugin(hooks: Partial>) { + const transformProps = vi.fn((_instance: unknown, props: unknown) => props as object); + + return { plugin: { transformProps, ...hooks } as Plugin, transformProps }; +} + +const flush = () => Promise.resolve(); + +describe('LightningViewElement style fast path', () => { + it('takes the fast path when needsTransform declines the update', async () => { + const needsTransform = vi.fn(() => false); + const { plugin, transformProps } = spyPlugin({ needsTransform }); + const el = createElement([plugin]); + + transformProps.mockClear(); + el.setProps({ style: { alpha: 0.5 } }); + await flush(); + + expect(needsTransform).toHaveBeenCalledTimes(1); + expect(transformProps).not.toHaveBeenCalled(); + expect(el.node.alpha).toBe(0.5); + }); + + it('takes the slow path when needsTransform claims the update', async () => { + const { plugin, transformProps } = spyPlugin({ needsTransform: () => true }); + const el = createElement([plugin]); + + transformProps.mockClear(); + el.setProps({ style: { alpha: 0.5 } }); + await flush(); + + expect(transformProps).toHaveBeenCalledTimes(1); + expect(el.node.alpha).toBe(0.5); + }); + + it('lets needsTransform replace the handledStyleProps check', async () => { + const { plugin, transformProps } = spyPlugin({ + handledStyleProps: new Set(['alpha']), + needsTransform: () => false, + }); + const el = createElement([plugin]); + + transformProps.mockClear(); + el.setProps({ style: { alpha: 0.5 } }); + await flush(); + + expect(transformProps).not.toHaveBeenCalled(); + }); + + it('still forces the slow path for a plugin with neither hook', async () => { + const declines = spyPlugin({ needsTransform: () => false }); + const legacy = spyPlugin({}); + const el = createElement([declines.plugin, legacy.plugin]); + + declines.transformProps.mockClear(); + legacy.transformProps.mockClear(); + el.setProps({ style: { alpha: 0.5 } }); + await flush(); + + expect(declines.transformProps).toHaveBeenCalledTimes(1); + expect(legacy.transformProps).toHaveBeenCalledTimes(1); + }); + + it('still honours handledStyleProps next to a needsTransform plugin', async () => { + const declines = spyPlugin({ needsTransform: () => false }); + const keyed = spyPlugin({ handledStyleProps: new Set(['w']) }); + const el = createElement([declines.plugin, keyed.plugin]); + + keyed.transformProps.mockClear(); + el.setProps({ style: { alpha: 0.5 } }); + await flush(); + expect(keyed.transformProps).not.toHaveBeenCalled(); + + el.setProps({ style: { w: 20 } }); + await flush(); + expect(keyed.transformProps).toHaveBeenCalledTimes(1); + }); + + it('keeps non-style updates on the slow path without asking needsTransform', async () => { + const needsTransform = vi.fn(() => false); + const { plugin, transformProps } = spyPlugin({ needsTransform }); + const el = createElement([plugin]); + + transformProps.mockClear(); + el.setProps({ style: { alpha: 0.5 }, testID: 'x' } as never); + await flush(); + + expect(needsTransform).not.toHaveBeenCalled(); + expect(transformProps).toHaveBeenCalledTimes(1); + }); + + it('keeps shader style keys on the slow path', async () => { + const { plugin, transformProps } = spyPlugin({ needsTransform: () => false }); + const el = createElement([plugin]); + + transformProps.mockClear(); + el.setProps({ style: { borderRadius: 4 } } as never); + await flush(); + + expect(transformProps).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/react-lightning/src/element/LightningViewElement.ts b/packages/react-lightning/src/element/LightningViewElement.ts index 706b731d..5c98ae33 100644 --- a/packages/react-lightning/src/element/LightningViewElement.ts +++ b/packages/react-lightning/src/element/LightningViewElement.ts @@ -1771,15 +1771,29 @@ export class LightningViewElement< if (LightningViewElement._shaderStyleProps.has(key)) { return false; } + } - for (const plugin of this._plugins) { - if (!plugin.transformProps) { - continue; + for (const plugin of this._plugins) { + if (!plugin.transformProps) { + continue; + } + + if (plugin.needsTransform) { + if (plugin.needsTransform(this, payload as TProps)) { + return false; } - const handled = plugin.handledStyleProps; + continue; + } - if (!handled || handled.has(key)) { + const handled = plugin.handledStyleProps; + + if (!handled) { + return false; + } + + for (const key in style) { + if (handled.has(key)) { return false; } } diff --git a/packages/react-lightning/src/render/Plugin.ts b/packages/react-lightning/src/render/Plugin.ts index 4e189b82..1291265c 100644 --- a/packages/react-lightning/src/render/Plugin.ts +++ b/packages/react-lightning/src/render/Plugin.ts @@ -36,4 +36,13 @@ export type Plugin = { * needs to process. */ handledStyleProps?: ReadonlySet; + + /** + * Instance-aware alternative to handledStyleProps for style-only updates. + * Return false when transformProps would leave this update unchanged for + * this instance, so it can take the fast path; true sends it through the + * full transform pipeline. When set, it replaces the handledStyleProps check + * for this plugin. + */ + needsTransform?(instance: SetOptional, props: T['props']): boolean; }; diff --git a/packages/react-native-lightning/src/plugins/cssClassNameTransformPlugin.test.ts b/packages/react-native-lightning/src/plugins/cssClassNameTransformPlugin.test.ts index bbac58ea..11c7e547 100644 --- a/packages/react-native-lightning/src/plugins/cssClassNameTransformPlugin.test.ts +++ b/packages/react-native-lightning/src/plugins/cssClassNameTransformPlugin.test.ts @@ -1,6 +1,15 @@ -import type { LightningElement, Plugin } from '@plextv/react-lightning'; +import type { RendererMain } from '@lightningjs/renderer'; +import type { Fiber } from 'react-reconciler'; import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; +import { + type LightningElement, + type LightningViewElementProps, + LightningViewElement, + type Plugin, +} from '@plextv/react-lightning'; +import { plugin as cssPlugin } from '@plextv/react-lightning-plugin-css-transform'; + import { cssClassNameTransformPlugin } from './cssClassNameTransformPlugin'; type TestProps = { className?: string; style: Record }; @@ -95,3 +104,142 @@ describe('cssClassNameTransformPlugin', () => { expect(out).toBe(props); }); }); + +const makeNode = (props: Record) => ({ + ...props, + on() {}, + off() {}, + emit() {}, + animate: () => ({ once() {}, start() {} }), + destroy() {}, +}); + +const renderer = { + createNode: makeNode, + createTextNode: makeNode, + createShader: () => ({ props: {} }), + createTexture: () => ({}), + destroyNode() {}, +} as unknown as RendererMain; + +// Records the styles that reach the transform pipeline. Claims no style keys +// itself, so a style-only update only goes slow when another plugin asks. +function recorder() { + const styles: Array> = []; + const plugin: Plugin = { + handledStyleProps: new Set(), + transformProps(_instance, props) { + const style = (props as { style?: Record }).style; + + if (style) { + styles.push({ ...style }); + } + + return props; + }, + }; + + return { plugin, styles }; +} + +function createElement(props: Record, plugins: Plugin[]) { + return new LightningViewElement( + props as unknown as LightningViewElementProps, + renderer, + plugins, + {} as Fiber, + ); +} + +const flush = () => Promise.resolve(); + +describe('cssClassNameTransformPlugin style fast path', () => { + it('lets style-only updates skip the pipeline when the element has no className', async () => { + const rec = recorder(); + const el = createElement({ style: { w: 10 } }, [cssClassNameTransformPlugin(), rec.plugin]); + + rec.styles.length = 0; + el.setProps({ style: { alpha: 0.5 } }); + await flush(); + + expect(rec.styles).toEqual([]); + expect(el.node.alpha).toBe(0.5); + }); + + it('keeps merging remembered class styles into style-only updates', async () => { + const rec = recorder(); + const el = createElement({ className: 'r-col', style: { w: 10 } }, [ + cssClassNameTransformPlugin(), + rec.plugin, + ]); + + rec.styles.length = 0; + el.setProps({ style: { alpha: 0.5 } }); + await flush(); + + expect(rec.styles).toEqual([{ flexDirection: 'column', alignItems: 'stretch', alpha: 0.5 }]); + expect(el.props.style).toMatchObject({ flexDirection: 'column', alpha: 0.5 }); + }); + + it('starts merging once a className is added later', async () => { + const rec = recorder(); + const el = createElement({ style: { w: 10 } }, [cssClassNameTransformPlugin(), rec.plugin]); + + el.setProps({ className: 'r-row' } as never); + await flush(); + + rec.styles.length = 0; + el.setProps({ style: { alpha: 0.5 } }); + await flush(); + + expect(rec.styles).toEqual([{ flexDirection: 'row', alpha: 0.5 }]); + }); + + it('goes back to the fast path once the className resolves to nothing', async () => { + const rec = recorder(); + const el = createElement({ className: 'r-col', style: { w: 10 } }, [ + cssClassNameTransformPlugin(), + rec.plugin, + ]); + + el.setProps({ className: '' } as never); + await flush(); + + rec.styles.length = 0; + el.setProps({ style: { alpha: 0.5 } }); + await flush(); + + expect(rec.styles).toEqual([]); + expect(el.node.alpha).toBe(0.5); + }); + + it('runs the pipeline for an update that carries className and style together', async () => { + const rec = recorder(); + const el = createElement({ style: { w: 10 } }, [cssClassNameTransformPlugin(), rec.plugin]); + + rec.styles.length = 0; + el.setProps({ className: 'r-row', style: { alpha: 0.5 } } as never); + await flush(); + + expect(rec.styles).toEqual([{ flexDirection: 'row', alpha: 0.5 }]); + }); + + it('leaves css-transform keys on the slow path', async () => { + const rec = recorder(); + const el = createElement({ style: { w: 10 } }, [ + cssClassNameTransformPlugin(), + cssPlugin(), + rec.plugin, + ]); + + rec.styles.length = 0; + el.setProps({ style: { alpha: 0.5 } }); + await flush(); + expect(rec.styles).toEqual([]); + + el.setProps({ style: { opacity: 0.25 } } as never); + await flush(); + expect(rec.styles).toHaveLength(1); + expect(el.node.alpha).toBe(0.25); + }); +}); diff --git a/packages/react-native-lightning/src/plugins/cssClassNameTransformPlugin.ts b/packages/react-native-lightning/src/plugins/cssClassNameTransformPlugin.ts index 521249b2..33c9b342 100644 --- a/packages/react-native-lightning/src/plugins/cssClassNameTransformPlugin.ts +++ b/packages/react-native-lightning/src/plugins/cssClassNameTransformPlugin.ts @@ -36,6 +36,12 @@ export const cssClassNameTransformPlugin = (): Plugin => { } return { + // Only updates that carry className, or land on an instance with resolved class styles to + // merge back in, need the transform. Everything else can take the style fast path. + needsTransform(instance, props) { + return 'className' in props || resolvedClassStyles.has(instance); + }, + transformProps(instance, props) { if (!('className' in props)) { const remembered = resolvedClassStyles.get(instance); @@ -73,7 +79,19 @@ export const cssClassNameTransformPlugin = (): Plugin => { } } - resolvedClassStyles.set(instance, finalStyle as Record); + // Nothing to merge back for an empty resolution, so later style-only updates stay fast. + let hasClassStyles = false; + + for (const _ in finalStyle) { + hasClassStyles = true; + break; + } + + if (hasClassStyles) { + resolvedClassStyles.set(instance, finalStyle as Record); + } else { + resolvedClassStyles.delete(instance); + } return { ...otherProps, From 458bd919e33613cc142a73e13e7aa946a0b6995d Mon Sep 17 00:00:00 2001 From: Ruud Date: Thu, 15 Oct 2026 14:11:51 +0200 Subject: [PATCH 14/17] perf(css-transform): convert lone transform and opacity updates on the style fast path --- .changeset/css-transform-fast-lane.md | 6 + .../src/convertCSSStyleToLightning.ts | 80 ++++- .../src/convertTransformAndOpacity.spec.ts | 102 ++++++ packages/plugin-css-transform/src/index.ts | 9 +- .../LightningViewElement.fastPath.spec.ts | 63 ++++ .../src/element/LightningViewElement.ts | 65 ++-- packages/react-lightning/src/render/Plugin.ts | 8 + .../cssClassNameTransformPlugin.test.ts | 12 +- .../src/plugins/cssTransformFastPath.test.ts | 302 ++++++++++++++++++ 9 files changed, 605 insertions(+), 42 deletions(-) create mode 100644 .changeset/css-transform-fast-lane.md create mode 100644 packages/plugin-css-transform/src/convertTransformAndOpacity.spec.ts create mode 100644 packages/react-native-lightning/src/plugins/cssTransformFastPath.test.ts diff --git a/.changeset/css-transform-fast-lane.md b/.changeset/css-transform-fast-lane.md new file mode 100644 index 00000000..035cb38b --- /dev/null +++ b/.changeset/css-transform-fast-lane.md @@ -0,0 +1,6 @@ +--- +'@plextv/react-lightning': patch +'@plextv/react-lightning-plugin-css-transform': patch +--- + +add an optional `transformStyle` plugin hook so a plugin can convert the keys it handles inside the style fast path, and use it in css-transform for updates that only carry `transform` and `opacity` (reanimated's pushes), which used to go through the full transform pipeline diff --git a/packages/plugin-css-transform/src/convertCSSStyleToLightning.ts b/packages/plugin-css-transform/src/convertCSSStyleToLightning.ts index 05ec1a54..71db73d6 100644 --- a/packages/plugin-css-transform/src/convertCSSStyleToLightning.ts +++ b/packages/plugin-css-transform/src/convertCSSStyleToLightning.ts @@ -100,6 +100,70 @@ const CONVERTED_KEYS: ReadonlySet = new Set([ 'borderBottomEndRadius', ]); +function applyTransform(finalStyle: LightningElementStyle, transform: AllStyles['transform']) { + const { scaleX, scaleY, rotation, ...translateTransforms } = parseTransform( + transform as Parameters[0], + ); + + if (scaleX != null) { + finalStyle.scaleX = scaleX; + } + + if (scaleY != null) { + finalStyle.scaleY = scaleY; + } + + if (rotation != null) { + finalStyle.rotation = rotation; + } + + finalStyle.transform = translateTransforms; +} + +/** + * The common animated update (reanimated pushes a lone transform or opacity) + * converted without the full pass. Returns the same style as + * convertCSSStyleToLightning, or null when the style has any other key that + * function converts, so the caller falls back to it. + */ +export function convertTransformAndOpacity( + style: Record, +): LightningElementStyle | null { + const finalStyle = {} as LightningElementStyle; + let opacity: unknown; + let transform: unknown; + + for (const key in style) { + const value = style[key]; + + if (key === 'opacity') { + opacity = value; + } else if (key === 'transform') { + transform = value; + } else if (CONVERTED_KEYS.has(key) || key === 'display' || key === 'left' || key === 'top') { + return null; + } else if (value !== undefined) { + (finalStyle as Record)[key] = value; + } + } + + const partial = (style as Record)[PARTIAL_STYLE]; + + if (partial !== undefined) { + (finalStyle as Record)[PARTIAL_STYLE] = partial; + } + + if (typeof opacity === 'number') { + finalStyle.alpha = opacity; + } + + if (transform != null) { + applyTransform(finalStyle, transform as AllStyles['transform']); + } + + return finalStyle; +} + export function convertCSSStyleToLightning( style: AllStyleProps, ): LightningElementStyle | undefined { @@ -292,21 +356,7 @@ export function convertCSSStyleToLightning( } if (transform != null) { - const { scaleX, scaleY, rotation, ...translateTransforms } = parseTransform(transform); - - if (scaleX != null) { - finalStyle.scaleX = scaleX; - } - - if (scaleY != null) { - finalStyle.scaleY = scaleY; - } - - if (rotation != null) { - finalStyle.rotation = rotation; - } - - finalStyle.transform = translateTransforms; + applyTransform(finalStyle, transform); } // Disabled for now as some components set overflow to hidden while not having their size correctly calculated diff --git a/packages/plugin-css-transform/src/convertTransformAndOpacity.spec.ts b/packages/plugin-css-transform/src/convertTransformAndOpacity.spec.ts new file mode 100644 index 00000000..c58348e1 --- /dev/null +++ b/packages/plugin-css-transform/src/convertTransformAndOpacity.spec.ts @@ -0,0 +1,102 @@ +import { describe, expect, it } from 'vitest'; + +import { PARTIAL_STYLE } from '@plextv/react-lightning'; + +import { + convertCSSStyleToLightning, + convertTransformAndOpacity, +} from './convertCSSStyleToLightning'; +import type { AllStyleProps } from './types/ReactStyle'; + +const transforms: unknown[] = [ + [{ translateX: 10 }], + [{ translateY: -24.5 }], + [{ translateX: '50%' }], + [{ translateY: ' -100% ' }], + [{ translate: [4, 8] }], + [{ translate: 6 }], + [{ scale: 1.08 }], + [{ scale: '1.5' }], + [{ scaleX: 2 }], + [{ scaleY: 0.5 }], + [{ rotate: '45deg' }], + [{ rotate: '0.5turn' }], + [{ rotate: '100grad' }], + [{ rotate: '1rad' }], + [{ rotation: 0.25 }], + [{ matrix: [1, 0, 0, 1, 0, 0] }], + [{ translateX: 10 }, { scale: 1.1 }, { rotate: '90deg' }], + [{ scale: 1.1 }, { translateX: 10 }], + [{ scale: 2 }, { scaleX: 3 }], + [{ scaleX: 3 }, { scale: 2 }], + [{ translateX: 5 }, { translateX: 7 }], + [], + { translateX: 12, scaleY: 1.2 }, + 'translateX(10px) scale(1.2) rotate(30deg)', + 'translate(3, 4)', +]; + +const extras: Record[] = [ + {}, + { opacity: 0.5 }, + { opacity: 0 }, + { opacity: '0.5' }, + { opacity: undefined }, + { alpha: 0.3 }, + { alpha: 0.3, opacity: 0.6 }, + { scaleX: 4 }, + { x: 10, y: 20 }, + { pivot: 0.5 }, + { pivotX: 0, pivotY: 1 }, + { zIndex: 2, undefinedKey: undefined }, +]; + +function cases(): Record[] { + const out: Record[] = []; + + for (const extra of extras) { + out.push({ ...extra }); + out.push({ ...extra, [PARTIAL_STYLE]: true }); + + for (const transform of transforms) { + out.push({ ...extra, transform }); + out.push({ transform, ...extra, [PARTIAL_STYLE]: true }); + } + } + + return out; +} + +function snapshot(style: object | null | undefined) { + return style + ? { ...style, partial: (style as Record)[PARTIAL_STYLE] } + : style; +} + +describe('convertTransformAndOpacity', () => { + it.each(cases())('matches convertCSSStyleToLightning for %o', (style) => { + const fast = convertTransformAndOpacity(style as Record); + + expect(fast).not.toBeNull(); + expect(snapshot(fast)).toStrictEqual( + snapshot(convertCSSStyleToLightning(style as AllStyleProps)), + ); + }); + + it.each([ + { opacity: 1, display: 'none' }, + { opacity: 1, display: 'flex' }, + { transform: [{ scale: 1 }], left: 10 }, + { transform: [{ scale: 1 }], top: '5' }, + { opacity: 1, backgroundColor: '#fff' }, + { opacity: 1, width: 10 }, + { opacity: 1, borderRadius: 4 }, + { opacity: 1, borderTopLeftRadius: 4 }, + { opacity: 1, overflow: 'hidden' }, + { opacity: 1, textAlign: 'center' }, + { opacity: 1, fontWeight: '600' }, + { opacity: 1, borderColor: 'red' }, + ])('leaves %o to the full conversion', (style) => { + expect(convertTransformAndOpacity(style)).toBeNull(); + }); +}); diff --git a/packages/plugin-css-transform/src/index.ts b/packages/plugin-css-transform/src/index.ts index 6dc76484..5c14518a 100644 --- a/packages/plugin-css-transform/src/index.ts +++ b/packages/plugin-css-transform/src/index.ts @@ -1,6 +1,9 @@ import type { Plugin } from '@plextv/react-lightning'; -import { convertCSSStyleToLightning } from './convertCSSStyleToLightning'; +import { + convertCSSStyleToLightning, + convertTransformAndOpacity, +} from './convertCSSStyleToLightning'; import type { AllStyleProps } from './types/ReactStyle'; export { convertCSSStyleToLightning }; @@ -41,6 +44,10 @@ export function plugin(): Plugin { return { handledStyleProps: CSS_HANDLED_STYLE_PROPS, + transformStyle(_instance, style) { + return convertTransformAndOpacity(style as Record); + }, + transformProps(_instance, props) { if (!('style' in props)) { return props; diff --git a/packages/react-lightning/src/element/LightningViewElement.fastPath.spec.ts b/packages/react-lightning/src/element/LightningViewElement.fastPath.spec.ts index b896acee..1d47efca 100644 --- a/packages/react-lightning/src/element/LightningViewElement.fastPath.spec.ts +++ b/packages/react-lightning/src/element/LightningViewElement.fastPath.spec.ts @@ -137,4 +137,67 @@ describe('LightningViewElement style fast path', () => { expect(transformProps).toHaveBeenCalledTimes(1); }); + + it('applies the style transformStyle converts without running transformProps', async () => { + const transformStyle = vi.fn((_instance: unknown, style: object) => { + const { opacity, ...rest } = style as { opacity?: number }; + + return { ...rest, alpha: opacity }; + }); + const { plugin, transformProps } = spyPlugin({ + handledStyleProps: new Set(['opacity']), + transformStyle, + }); + const el = createElement([plugin]); + + transformProps.mockClear(); + el.setProps({ style: { opacity: 0.25 } } as never); + await flush(); + + expect(transformStyle).toHaveBeenCalledTimes(1); + expect(transformProps).not.toHaveBeenCalled(); + expect(el.node.alpha).toBe(0.25); + }); + + it('falls back to transformProps when transformStyle declines', async () => { + const { plugin, transformProps } = spyPlugin({ + handledStyleProps: new Set(['opacity']), + transformStyle: () => null, + }); + const el = createElement([plugin]); + + transformProps.mockClear(); + el.setProps({ style: { opacity: 0.25 } } as never); + await flush(); + + expect(transformProps).toHaveBeenCalledTimes(1); + }); + + it('checks later plugins against the converted style', async () => { + const converts = spyPlugin({ + handledStyleProps: new Set(['width']), + transformStyle: () => ({ w: 20 }), + }); + const keyed = spyPlugin({ handledStyleProps: new Set(['w']) }); + const el = createElement([converts.plugin, keyed.plugin]); + + converts.transformProps.mockClear(); + keyed.transformProps.mockClear(); + el.setProps({ style: { width: 20 } } as never); + await flush(); + + expect(converts.transformProps).toHaveBeenCalledTimes(1); + expect(keyed.transformProps).toHaveBeenCalledTimes(1); + }); + + it('keeps style arrays on the slow path', async () => { + const { plugin, transformProps } = spyPlugin({ needsTransform: () => false }); + const el = createElement([plugin]); + + transformProps.mockClear(); + el.setProps({ style: [{ alpha: 0.5 }] } as never); + await flush(); + + expect(transformProps).toHaveBeenCalledTimes(1); + }); }); diff --git a/packages/react-lightning/src/element/LightningViewElement.ts b/packages/react-lightning/src/element/LightningViewElement.ts index 5c98ae33..51de4c0a 100644 --- a/packages/react-lightning/src/element/LightningViewElement.ts +++ b/packages/react-lightning/src/element/LightningViewElement.ts @@ -1585,8 +1585,10 @@ export class LightningViewElement< } // Fast path: style-only updates where no plugin handles the changed properties - if (this._canFastPathStyle(payload)) { - return this._applyStyleFastPath(payload); + const fastPathStyle = this._getFastPathStyle(payload); + + if (fastPathStyle !== null) { + return this._applyStyleFastPath(payload, fastPathStyle); } const transformedProps = this._transformProps(payload) ?? ({} as TProps); @@ -1743,12 +1745,15 @@ export class LightningViewElement< /** * Checks whether a staged update can skip the plugin transform pipeline. - * Returns true when the payload only contains style properties that no - * plugin declares as handled and that don't require shader creation. + * Returns the style to apply when the payload only contains style + * properties that no plugin needs the full transform for and that don't + * require shader creation, or null for the slow path. A plugin that + * implements `transformStyle` can convert the keys it handles in place. */ - private _canFastPathStyle(payload: Partial): boolean { - if (!('style' in payload) || !payload.style) { - return false; + private _getFastPathStyle(payload: Partial): Partial | null { + // Style arrays and strings only become keyed styles in the plugins. + if (!payload.style || typeof payload.style !== 'object' || Array.isArray(payload.style)) { + return null; } // A node that currently has a shader must take the slow path: the update @@ -1756,31 +1761,33 @@ export class LightningViewElement< // recomputes (or clears) the shader. The fast path assigns style keys to // the node verbatim and would leave a stale shader painting. if (this._shaderDef) { - return false; + return null; } for (const key in payload) { if (key !== 'style') { - return false; + return null; } } - const style = payload.style; + let style = payload.style as Partial; for (const key in style) { if (LightningViewElement._shaderStyleProps.has(key)) { - return false; + return null; } } + let current = payload; + for (const plugin of this._plugins) { if (!plugin.transformProps) { continue; } if (plugin.needsTransform) { - if (plugin.needsTransform(this, payload as TProps)) { - return false; + if (plugin.needsTransform(this, current as TProps)) { + return null; } continue; @@ -1789,31 +1796,43 @@ export class LightningViewElement< const handled = plugin.handledStyleProps; if (!handled) { - return false; + return null; } for (const key in style) { if (handled.has(key)) { - return false; + const converted = plugin.transformStyle?.(this, style) as Partial | null; + + if (converted == null) { + return null; + } + + style = converted; + current = { style } as Partial; + break; } } } - return true; + return style; } /** * Applies a style-only update directly to the Lightning node, bypassing * the plugin transform pipeline and full props-merge iteration. */ - private _applyStyleFastPath(payload: Partial): boolean { - const style = payload.style as Partial; - + private _applyStyleFastPath(payload: Partial, style: Partial): boolean { if (LightningViewElement.flattenLayoutViewsEnabled && this._flattened) { - for (const key in style) { - if (LightningViewElement._needsRealNode(key, style[key as keyof TStyleProps])) { - this._materialize(); - break; + // Same rule as the slow path: any transition (reanimated sets one with + // every push) needs a real node to animate. + if (this.props.transition !== undefined) { + this._materialize(); + } else { + for (const key in style) { + if (LightningViewElement._needsRealNode(key, style[key as keyof TStyleProps])) { + this._materialize(); + break; + } } } } diff --git a/packages/react-lightning/src/render/Plugin.ts b/packages/react-lightning/src/render/Plugin.ts index 1291265c..27ed7060 100644 --- a/packages/react-lightning/src/render/Plugin.ts +++ b/packages/react-lightning/src/render/Plugin.ts @@ -45,4 +45,12 @@ export type Plugin = { * for this plugin. */ needsTransform?(instance: SetOptional, props: T['props']): boolean; + + /** + * Converts the style of a style-only update that touches handledStyleProps, + * so it can still take the fast path. Must return the style transformProps + * would produce for that update, or null when the update needs the full + * transform pipeline. + */ + transformStyle?(instance: SetOptional, style: object): object | null; }; diff --git a/packages/react-native-lightning/src/plugins/cssClassNameTransformPlugin.test.ts b/packages/react-native-lightning/src/plugins/cssClassNameTransformPlugin.test.ts index 11c7e547..dcd172df 100644 --- a/packages/react-native-lightning/src/plugins/cssClassNameTransformPlugin.test.ts +++ b/packages/react-native-lightning/src/plugins/cssClassNameTransformPlugin.test.ts @@ -224,7 +224,7 @@ describe('cssClassNameTransformPlugin style fast path', () => { expect(rec.styles).toEqual([{ flexDirection: 'row', alpha: 0.5 }]); }); - it('leaves css-transform keys on the slow path', async () => { + it('converts opacity and transform on the fast path, other css-transform keys go slow', async () => { const rec = recorder(); const el = createElement({ style: { w: 10 } }, [ cssClassNameTransformPlugin(), @@ -237,9 +237,15 @@ describe('cssClassNameTransformPlugin style fast path', () => { await flush(); expect(rec.styles).toEqual([]); - el.setProps({ style: { opacity: 0.25 } } as never); + el.setProps({ style: { opacity: 0.25, transform: [{ scale: 2 }] } } as never); await flush(); - expect(rec.styles).toHaveLength(1); + expect(rec.styles).toEqual([]); expect(el.node.alpha).toBe(0.25); + expect(el.node.scaleX).toBe(2); + + el.setProps({ style: { opacity: 0.75, backgroundColor: '#ff0000' } } as never); + await flush(); + expect(rec.styles).toHaveLength(1); + expect(el.node.alpha).toBe(0.75); }); }); diff --git a/packages/react-native-lightning/src/plugins/cssTransformFastPath.test.ts b/packages/react-native-lightning/src/plugins/cssTransformFastPath.test.ts new file mode 100644 index 00000000..31964ef7 --- /dev/null +++ b/packages/react-native-lightning/src/plugins/cssTransformFastPath.test.ts @@ -0,0 +1,302 @@ +import type { RendererMain } from '@lightningjs/renderer'; +import type { Fiber } from 'react-reconciler'; +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from 'vitest'; + +import { + type LightningElement, + type LightningViewElementProps, + LightningTextElement, + LightningViewElement, + PARTIAL_STYLE, + type Plugin, +} from '@plextv/react-lightning'; +import { plugin as cssPlugin } from '@plextv/react-lightning-plugin-css-transform'; +import { plugin as flexboxPlugin } from '@plextv/react-lightning-plugin-flexbox'; + +import { cssClassNameTransformPlugin } from './cssClassNameTransformPlugin'; + +type Recorded = { animations: unknown[]; transforms: number }; + +function makeRenderer(recorded: Recorded) { + const makeNode = (props: Record) => ({ + ...props, + on() {}, + off() {}, + emit() {}, + animate(animProps: unknown, settings: unknown) { + recorded.animations.push({ animProps, settings }); + + return { once() {}, start() {} }; + }, + destroy() {}, + }); + + return { + createNode: makeNode, + createTextNode: makeNode, + createShader: () => ({ props: {} }), + createTexture: () => ({}), + destroyNode() {}, + } as unknown as RendererMain; +} + +// The react-native stack, with css-transform's fast lane optionally removed so +// every update takes the slow path through transformProps. +async function stack( + renderer: RendererMain, + recorded: Recorded, + fastLane: boolean, +): Promise[]> { + const css = cssPlugin(); + + if (!fastLane) { + delete css.transformStyle; + } + + const counter: Plugin = { + handledStyleProps: new Set(), + transformProps(_instance, props) { + recorded.transforms++; + + return props; + }, + }; + + const flexbox = flexboxPlugin({ useWebWorker: false }) as Plugin; + + await flexbox.init?.(renderer, null as never); + + return [cssClassNameTransformPlugin(), css, flexbox, counter]; +} + +const NODE_KEYS = [ + 'x', + 'y', + 'w', + 'h', + 'alpha', + 'scale', + 'scaleX', + 'scaleY', + 'rotation', + 'pivot', + 'pivotX', + 'pivotY', + 'color', + 'zIndex', + 'transform', + 'opacity', + 'clipping', + 'clipRadius', +]; + +type Snapshot = Record; + +function snapshot(el: LightningViewElement, events: unknown[]): Snapshot { + const node = el.node as unknown as Record; + const nodeProps: Record = {}; + + for (const key of NODE_KEYS) { + nodeProps[key] = node[key]; + } + + const internals = el as unknown as { + _styleTarget: object; + _animTargets?: Map; + _flattened: boolean; + _layoutX: number; + _layoutY: number; + }; + const style = el.props.style as Record | undefined; + + return { + nodeProps, + style: style ? { ...style, partial: style[PARTIAL_STYLE] } : style, + styleTarget: { ...internals._styleTarget }, + animTargets: internals._animTargets ? [...internals._animTargets] : [], + flattened: internals._flattened, + layout: [internals._layoutX, internals._layoutY], + visible: el.visible, + events: [...events], + }; +} + +type Step = + | { style: Record } + | { transition: Record } + | { setProps: Record }; + +async function run(initial: Record, steps: Step[], fastLane: boolean) { + const recorded: Recorded = { animations: [], transforms: 0 }; + const renderer = makeRenderer(recorded); + const el = new LightningViewElement( + // DEV merges every payload into rawProps, which is the initial props object. + { ...initial, style: { ...(initial.style as object) } } as unknown as LightningViewElementProps, + renderer, + await stack(renderer, recorded, fastLane), + {} as Fiber, + ); + const events: unknown[] = []; + + el.on('stylesChanged', (style) => events.push({ ...style })); + recorded.transforms = 0; + + const snapshots: Snapshot[] = []; + + for (const step of steps) { + if ('style' in step) { + el.setProps({ style: { ...step.style } } as never); + } else if ('transition' in step) { + el.setProps({ transition: step.transition } as never); + } else { + el.setProps(step.setProps as never); + } + + await Promise.resolve(); + snapshots.push(snapshot(el, events)); + } + + return { snapshots, animations: recorded.animations, transforms: recorded.transforms }; +} + +const partial = (style: Record) => ({ ...style, [PARTIAL_STYLE]: true }); + +const sequences: Record = { + 'transform pushes': [ + { style: partial({ transform: [{ scale: 1.08 }] }) }, + { style: partial({ transform: [{ scale: 1 }] }) }, + { style: partial({ transform: [{ translateX: 12 }, { translateY: -4 }] }) }, + { style: partial({ transform: [{ translateX: '50%' }] }) }, + { style: partial({ transform: [{ rotate: '45deg' }, { scaleX: 2 }, { scaleY: 0.5 }] }) }, + { style: partial({ transform: [{ scale: 2 }, { scaleX: 3 }] }) }, + { style: partial({ transform: { translateX: 3, scaleY: 1.2 } }) }, + { style: partial({ transform: 'translateX(10px) scale(1.2) rotate(30deg)' }) }, + { style: partial({ transform: [{ matrix: [1, 0, 0, 1, 0, 0] }] }) }, + { style: partial({ transform: [] }) }, + ], + 'opacity pushes': [ + { style: partial({ opacity: 0 }) }, + { style: partial({ opacity: 0.5 }) }, + { style: partial({ opacity: 1 }) }, + { style: partial({ opacity: '0.4' }) }, + ], + 'lightning-shaped reanimated pushes': [ + { style: partial({ alpha: 0.5, x: 10, y: 20 }) }, + { style: partial({ scaleX: 1.1, scaleY: 1.1 }) }, + { style: partial({ transform: { translateX: 40 } }) }, + { style: partial({ alpha: 1, transform: { translateY: 8 } }) }, + { style: partial({ pivotX: 0, pivotY: 1, rotation: 0.3 }) }, + ], + 'mixed and full snapshots': [ + { style: { opacity: 0.6, transform: [{ scale: 1.05 }] } }, + { style: { opacity: 1, transform: [{ scale: 1 }], zIndex: 2 } }, + { style: partial({ opacity: 0.2, backgroundColor: '#ff0000' }) }, + { style: partial({ opacity: 0.7, transform: [{ scale: 1.2 }] }) }, + { style: partial({ alpha: 0.3, opacity: 0.9 }) }, + { style: partial({ opacity: 1, display: 'none' }) }, + { style: partial({ opacity: 1, display: 'flex' }) }, + { setProps: { style: [{ opacity: 0.5 }, { transform: [{ scale: 1.2 }] }] } }, + ], + 'with a transition': [ + { transition: { alpha: { duration: 100 }, scaleX: { duration: 100 }, x: { duration: 50 } } }, + { style: partial({ opacity: 0.5 }) }, + { style: partial({ transform: [{ scale: 1.1 }] }) }, + { style: partial({ transform: [{ scale: 1.1 }] }) }, + { style: partial({ opacity: 0.5, x: 30 }) }, + { style: partial({ opacity: 1 }) }, + ], + 'reanimated pushes that carry their transition': [ + { setProps: { transition: {}, style: partial({ transform: { translateX: 20 } }) } }, + { setProps: { transition: {}, style: partial({ opacity: 1 }) } }, + { + setProps: { + transition: { scaleX: { duration: 80 }, scaleY: { duration: 80 } }, + style: partial({ transform: [{ scale: 1.1 }] }), + }, + }, + ], + 'a non-style prop next to the style': [ + { setProps: { style: partial({ opacity: 0.5 }), focusable: true } }, + { style: partial({ transform: [{ scale: 1.2 }] }) }, + ], +}; + +const initials: Record> = { + plain: { style: { w: 200, h: 300 } }, + positioned: { style: { w: 200, h: 300, x: 10, y: 10, opacity: 0.8 } }, + 'with className styles': { className: 'r-col', style: { w: 200, h: 300 } }, +}; + +describe('css-transform fast lane', () => { + beforeAll(() => { + vi.stubGlobal('document', { + styleSheets: [ + { + ownerNode: { id: 'react-native-stylesheet' }, + cssRules: [{ selectorText: '.r-col', style: { cssText: 'flex-direction: column' } }], + }, + ], + }); + }); + + afterAll(() => { + vi.unstubAllGlobals(); + }); + + afterEach(() => { + LightningViewElement.flattenLayoutViewsEnabled = false; + }); + + for (const [initialName, initial] of Object.entries(initials)) { + for (const [name, steps] of Object.entries(sequences)) { + it(`leaves the same element state as the slow path: ${name}, ${initialName}`, async () => { + const slow = await run(initial, steps, false); + const fast = await run(initial, steps, true); + + expect(fast.snapshots).toStrictEqual(slow.snapshots); + expect(fast.animations).toStrictEqual(slow.animations); + }); + } + } + + it('matches the slow path on a flattened element', async () => { + LightningViewElement.flattenLayoutViewsEnabled = true; + + for (const steps of Object.values(sequences)) { + const slow = await run({ style: { w: 200, h: 300 } }, steps, false); + const fast = await run({ style: { w: 200, h: 300 } }, steps, true); + + expect(fast.snapshots).toStrictEqual(slow.snapshots); + expect(fast.animations).toStrictEqual(slow.animations); + } + }); + + it('keeps lone transform and opacity pushes out of transformProps', async () => { + const steps = [ + ...(sequences['transform pushes'] ?? []), + ...(sequences['opacity pushes'] ?? []), + ]; + + expect((await run({ style: { w: 200, h: 300 } }, steps, false)).transforms).toBe(steps.length); + expect((await run({ style: { w: 200, h: 300 } }, steps, true)).transforms).toBe(0); + }); + + // The slow path restamps the text default color (white) on every partial + // push, because the pushed style carries no color. + it('keeps a text color through an opacity push', async () => { + const recorded: Recorded = { animations: [], transforms: 0 }; + const renderer = makeRenderer(recorded); + const el = new LightningTextElement( + { text: 'a', style: { color: '#ff0000' } } as never, + renderer, + await stack(renderer, recorded, true), + {} as Fiber, + ); + + el.setProps({ style: partial({ opacity: 0.5 }) } as never); + await Promise.resolve(); + + expect(el.node.alpha).toBe(0.5); + expect(el.node.color).toBe(0xff0000ff); + }); +}); From ee3d5dad5fe066c8f2a6dca132dedb76d94c5b55 Mon Sep 17 00:00:00 2001 From: Ruud Date: Thu, 15 Oct 2026 14:40:35 +0200 Subject: [PATCH 15/17] perf(flexbox): skip the layout pass when a style changes nothing yoga reads --- .changeset/flexbox-skip-noop-layout.md | 5 + .../plugin-flexbox/src/LightningManager.ts | 31 ++-- .../src/YogaManager.queueRender.test.ts | 136 ++++++++++++++++++ .../plugin-flexbox/src/YogaManager.spec.ts | 2 + packages/plugin-flexbox/src/YogaManager.ts | 47 +++++- .../plugin-flexbox/src/YogaManagerWorker.ts | 4 +- 6 files changed, 207 insertions(+), 18 deletions(-) create mode 100644 .changeset/flexbox-skip-noop-layout.md create mode 100644 packages/plugin-flexbox/src/YogaManager.queueRender.test.ts diff --git a/.changeset/flexbox-skip-noop-layout.md b/.changeset/flexbox-skip-noop-layout.md new file mode 100644 index 00000000..06bef0e7 --- /dev/null +++ b/.changeset/flexbox-skip-noop-layout.md @@ -0,0 +1,5 @@ +--- +'@plextv/react-lightning-plugin-flexbox': patch +--- + +skip the layout pass when a style or text update changes nothing yoga reads diff --git a/packages/plugin-flexbox/src/LightningManager.ts b/packages/plugin-flexbox/src/LightningManager.ts index f80de786..ee0ce520 100644 --- a/packages/plugin-flexbox/src/LightningManager.ts +++ b/packages/plugin-flexbox/src/LightningManager.ts @@ -279,7 +279,7 @@ export class LightningManager { * anything else (no family, or a non-measurable font like the canvas * `plex-icons` glyph font) is left to the renderer's own sizing. */ - private _syncTextMeasure(element: LightningElement): void { + private _syncTextMeasure(element: LightningElement, force = false): void { if (!this._yogaManager) { return; } @@ -297,17 +297,22 @@ export class LightningManager { } this._measuredText.add(element.id); - this._yogaManager.setTextMeasure(element.id, fontFamily, { - text: (element as LightningTextElement).text ?? '', - fontSize: style.fontSize || 16, - letterSpacing: style.letterSpacing || 0, - // 0 / undefined lineHeight → natural (1× metrics); a value > 3 is px. - lineHeight: style.lineHeight || 1, - maxLines: style.maxLines || 0, - maxHeight: style.maxHeight || 0, - wordBreak: (style.wordBreak as TextMeasureProps['wordBreak']) || 'break-word', - overflowSuffix: style.overflowSuffix ?? '...', - }); + this._yogaManager.setTextMeasure( + element.id, + fontFamily, + { + text: (element as LightningTextElement).text ?? '', + fontSize: style.fontSize || 16, + letterSpacing: style.letterSpacing || 0, + // 0 / undefined lineHeight → natural (1× metrics); a value > 3 is px. + lineHeight: style.lineHeight || 1, + maxLines: style.maxLines || 0, + maxHeight: style.maxHeight || 0, + wordBreak: (style.wordBreak as TextMeasureProps['wordBreak']) || 'break-word', + overflowSuffix: style.overflowSuffix ?? '...', + }, + force, + ); } public trackElement(element: LightningElement): void { @@ -676,7 +681,7 @@ export class LightningManager { if (contextWidth > (this._textContextWidth.get(textId) ?? 0)) { this._textContextWidth.set(textId, contextWidth); - this._syncTextMeasure(textEl); + this._syncTextMeasure(textEl, true); } } } diff --git a/packages/plugin-flexbox/src/YogaManager.queueRender.test.ts b/packages/plugin-flexbox/src/YogaManager.queueRender.test.ts new file mode 100644 index 00000000..82c2e3fc --- /dev/null +++ b/packages/plugin-flexbox/src/YogaManager.queueRender.test.ts @@ -0,0 +1,136 @@ +import { describe, expect, it } from 'vitest'; + +import type { AtlasData } from './text/FontMetricsStore'; +import { YogaManager } from './YogaManager'; + +const atlas: AtlasData = { + info: { size: 10, face: 'Test' }, + common: { lineHeight: 12, base: 8 }, + lightningMetrics: { ascender: 800, descender: -200, lineGap: 0, unitsPerEm: 1000 }, + chars: [ + { id: 97, xadvance: 10, xoffset: 0, yoffset: 0, width: 8, height: 8 }, + { id: 32, xadvance: 5, xoffset: 0, yoffset: 0, width: 0, height: 0 }, + ], + kernings: [], +}; + +const textProps = { + text: 'aa aa', + fontSize: 20, + letterSpacing: 0, + lineHeight: 1, + maxLines: 0, + maxHeight: 0, + wordBreak: 'break-word' as const, + overflowSuffix: '...', +}; + +async function setup() { + const manager = new YogaManager(); + await manager.init(); + ( + manager as unknown as { _fontStore: { register: (f: string, d: AtlasData) => void } } + )._fontStore.register('Test', atlas); + + manager.addNode(1); + manager.applyStyle(1, { w: 300, h: 200, display: 'flex', flexDirection: 'column' }, true); + manager.addIndependentRoot(1); + + manager.addNode(2); + manager.applyStyle(2, { w: 100, h: 50 }, true); + manager.addChildNode(1, 2); + + manager.addNode(3); + manager.addChildNode(1, 3); + manager.setTextMeasure(3, 'Test', textProps); + + manager.queueRender(1); + await Promise.resolve(); + + let renders = 0; + manager.on('render', () => renders++); + + return { + manager, + renders: async () => { + await Promise.resolve(); + + const count = renders; + renders = 0; + + return count; + }, + }; +} + +describe('YogaManager only lays out when a layout input changed', () => { + it('skips the pass for a style that changes nothing yoga reads', async () => { + const { manager, renders } = await setup(); + + manager.applyStyle(2, { w: 100, h: 50, scaleX: 1.1, color: 0xff0000ff } as never); + + expect(await renders()).toBe(0); + }); + + it('lays out when a flex prop changes', async () => { + const { manager, renders } = await setup(); + + manager.applyStyle(2, { w: 120, h: 50 }); + + expect(await renders()).toBe(1); + }); + + it('lays out when an earlier skipRender write left the node dirty', async () => { + const { manager, renders } = await setup(); + + // transformProps writes flex props with skipRender, then stylesChanged + // re-applies the remaining style without them. + manager.applyStyle(2, { w: 100, h: 50, marginTop: 10 }, true); + manager.applyStyle(2, { w: 100, h: 50 }); + + expect(await renders()).toBe(1); + }); + + it('lays out after a tree op even when the re-applied style is unchanged', async () => { + const { manager, renders } = await setup(); + + manager.addNode(4); + manager.applyStyle(4, { w: 10, h: 10 }, true); + manager.addChildNode(1, 4); + manager.applyStyle(1, { w: 300, h: 200, display: 'flex', flexDirection: 'column' }); + + expect(await renders()).toBe(1); + }); + + it('lays out for a percent translate push, which never dirties yoga', async () => { + const { manager, renders } = await setup(); + + manager.applyStyle(2, { transform: { translateX: '-50%' } } as never); + expect(await renders()).toBe(1); + + // Clearing it has to repaint too. + manager.applyStyle(2, { transform: {} } as never); + expect(await renders()).toBe(1); + + manager.applyStyle(2, { transform: {} } as never); + expect(await renders()).toBe(0); + }); + + it('skips re-measuring text whose measure props are unchanged', async () => { + const { manager, renders } = await setup(); + + manager.setTextMeasure(3, 'Test', { ...textProps }); + expect(await renders()).toBe(0); + + manager.setTextMeasure(3, 'Test', { ...textProps, text: 'aa' }); + expect(await renders()).toBe(1); + }); + + it('still re-measures unchanged text when forced', async () => { + const { manager, renders } = await setup(); + + manager.setTextMeasure(3, 'Test', { ...textProps }, true); + + expect(await renders()).toBe(1); + }); +}); diff --git a/packages/plugin-flexbox/src/YogaManager.spec.ts b/packages/plugin-flexbox/src/YogaManager.spec.ts index 95695739..e0da2561 100644 --- a/packages/plugin-flexbox/src/YogaManager.spec.ts +++ b/packages/plugin-flexbox/src/YogaManager.spec.ts @@ -14,6 +14,8 @@ const mockNode = { removeChild: vi.fn(), calculateLayout: vi.fn(), hasNewLayout: vi.fn(), + // The mocked setters don't track state, so treat every node as changed. + isDirty: vi.fn(() => true), getComputedLayout: vi.fn(), getComputedLeft: vi.fn(), getComputedTop: vi.fn(), diff --git a/packages/plugin-flexbox/src/YogaManager.ts b/packages/plugin-flexbox/src/YogaManager.ts index 7522acea..bf50085f 100644 --- a/packages/plugin-flexbox/src/YogaManager.ts +++ b/packages/plugin-flexbox/src/YogaManager.ts @@ -40,6 +40,19 @@ const MAX_SIZEOF_UPDATE = 1024 * 10; const MAX_LAYOUT_PASSES = 10; const TEXT_MEASURE_SLACK = 0.01; +function isSameTextMeasure(a: TextMeasureProps, b: TextMeasureProps): boolean { + return ( + a.text === b.text && + a.fontSize === b.fontSize && + a.letterSpacing === b.letterSpacing && + a.lineHeight === b.lineHeight && + a.maxLines === b.maxLines && + a.maxHeight === b.maxHeight && + a.wordBreak === b.wordBreak && + a.overflowSuffix === b.overflowSuffix + ); +} + export class YogaManager { private _elementMap: Map = new Map(); private _hiddenElements: Set = new Set(); @@ -128,14 +141,33 @@ export class YogaManager { * Set (or refresh) synchronous text measurement for a node. Installs a Yoga * measure function the first time so wrapping/sizing happen during layout. */ - public setTextMeasure(elementId: number, fontFamily: string, props: TextMeasureProps): void { + public setTextMeasure( + elementId: number, + fontFamily: string, + props: TextMeasureProps, + force = false, + ): void { const yogaNode = this._elementMap.get(elementId); if (!yogaNode) { return; } - const isFirst = yogaNode.text === undefined; + const previous = yogaNode.text; + + // Every text style change lands here. A dirty node may have had a w/h + // written since its measure, so only a clean one with the same props skips. + if ( + !force && + previous !== undefined && + previous.fontFamily === fontFamily && + isSameTextMeasure(previous.props, props) && + !yogaNode.node.isDirty() + ) { + return; + } + + const isFirst = previous === undefined; yogaNode.text = { fontFamily, props }; if (isFirst) { @@ -505,6 +537,10 @@ export class YogaManager { } } + // A percent translate lives outside yoga (resolved at readback), so a push + // that sets or clears one has to lay out even though no yoga input moved. + let touchedPercent = false; + if (style.transform) { const { x, y, transform } = style; @@ -517,6 +553,7 @@ export class YogaManager { // A string translate is a percentage of the node's OWN size, which // yoga can't express as a position edge — stash it and resolve at // readback (_getUpdatedStyles). Recomputed on every transform push. + touchedPercent = yogaNode.translatePercent !== undefined; yogaNode.translatePercent = undefined; yogaNode.resolvedTranslate = undefined; @@ -567,10 +604,14 @@ export class YogaManager { applyFlexPropToYoga(yoga, this._yogaOptions, node, edge, value); pixelTranslate.y = translateY != null; } + + touchedPercent ||= yogaNode.translatePercent !== undefined; } } - if (!skipRender) { + // Yoga setters only dirty on a real change, and tree ops dirty the parent, + // so a clean node means nothing layout reads moved (scale, colour). + if (!skipRender && (touchedPercent || yogaNode.node.isDirty())) { this.queueRender(elementId); } } diff --git a/packages/plugin-flexbox/src/YogaManagerWorker.ts b/packages/plugin-flexbox/src/YogaManagerWorker.ts index 1f4a52ab..43f052f1 100644 --- a/packages/plugin-flexbox/src/YogaManagerWorker.ts +++ b/packages/plugin-flexbox/src/YogaManagerWorker.ts @@ -370,12 +370,12 @@ function wrapWorker(worker: Worker): Workerized { removeIndependentRoot: (elementId: number) => nodeOperation('removeIndependentRoot', elementId), // Text measurement ops must land after the node's addNode (and any pending // styles), so flush the buffered pipeline before posting them. - setTextMeasure: (elementId: number, fontFamily: string, props: unknown) => { + setTextMeasure: (elementId: number, fontFamily: string, props: unknown, force?: boolean) => { flushChildOperations(); flushSendStyles(); worker.postMessage({ method: 'setTextMeasure', - args: [elementId, fontFamily, props], + args: [elementId, fontFamily, props, force], }); }, clearTextMeasure: (elementId: number) => { From 3c3bdb8573a14bff200f0ce1dd297acb235740f4 Mon Sep 17 00:00:00 2001 From: Ruud Date: Thu, 15 Oct 2026 14:42:25 +0200 Subject: [PATCH 16/17] perf(flexbox): skip clean roots and prune layout readback to nodes yoga touched --- .changeset/flexbox-prune-readback.md | 5 + .../src/YogaManager.readback.test.ts | 127 ++++++++++++++++++ packages/plugin-flexbox/src/YogaManager.ts | 83 +++++++++--- .../plugin-flexbox/src/types/ManagerNode.ts | 2 + 4 files changed, 202 insertions(+), 15 deletions(-) create mode 100644 .changeset/flexbox-prune-readback.md create mode 100644 packages/plugin-flexbox/src/YogaManager.readback.test.ts diff --git a/.changeset/flexbox-prune-readback.md b/.changeset/flexbox-prune-readback.md new file mode 100644 index 00000000..12a9d517 --- /dev/null +++ b/.changeset/flexbox-prune-readback.md @@ -0,0 +1,5 @@ +--- +'@plextv/react-lightning-plugin-flexbox': patch +--- + +skip clean flex roots and only read back nodes yoga laid out again diff --git a/packages/plugin-flexbox/src/YogaManager.readback.test.ts b/packages/plugin-flexbox/src/YogaManager.readback.test.ts new file mode 100644 index 00000000..3d685f99 --- /dev/null +++ b/packages/plugin-flexbox/src/YogaManager.readback.test.ts @@ -0,0 +1,127 @@ +import { describe, expect, it } from 'vitest'; + +import { YogaManager } from './YogaManager'; + +type Entry = { x: number; y: number; w: number; h: number }; + +function decode(buffer: ArrayBuffer): Map { + const view = new DataView(buffer); + const out = new Map(); + + for (let offset = 0; offset + 20 <= buffer.byteLength; offset += 20) { + out.set(view.getUint32(offset, true), { + x: view.getInt32(offset + 4, true), + y: view.getInt32(offset + 8, true), + w: view.getInt32(offset + 12, true), + h: view.getInt32(offset + 16, true), + }); + } + + return out; +} + +// Two independent roots: 1 > (2 > 3, 4) and 10 > 11. +async function setup() { + const manager = new YogaManager(); + await manager.init(); + + const node = (id: number, style: Record, parent?: number) => { + manager.addNode(id); + manager.applyStyle(id, style, true); + + if (parent !== undefined) { + manager.addChildNode(parent, id); + } + }; + + node(1, { w: 300, h: 200, flexDirection: 'column' }); + node(2, { h: 50, flexDirection: 'row' }, 1); + node(3, { w: 40, h: 20 }, 2); + node(4, { h: 30 }, 1); + node(10, { w: 100, h: 100 }); + node(11, { w: 10, h: 10 }, 10); + manager.addIndependentRoot(1); + manager.addIndependentRoot(10); + + const passes: Map[] = []; + manager.on('render', (buffer) => passes.push(decode(buffer))); + + // Runs the queued pass and returns its entries (a pass flushes once here). + const pass = async () => { + passes.length = 0; + await Promise.resolve(); + expect(passes).toHaveLength(1); + + return passes[0] ?? new Map(); + }; + + manager.queueRender(1); + await pass(); + + return { manager, pass }; +} + +describe('YogaManager readback', () => { + it('reads only the root that changed', async () => { + const { manager, pass } = await setup(); + + manager.applyStyle(11, { w: 20, h: 10 }); + + const entries = await pass(); + + expect([...entries.keys()].sort((a, b) => a - b)).toEqual([10, 11]); + expect(entries.get(11)).toEqual({ x: 0, y: 0, w: 20, h: 10 }); + }); + + it('stops at subtrees yoga did not lay out again', async () => { + const { manager, pass } = await setup(); + + manager.applyStyle(4, { h: 40 }); + + const entries = await pass(); + + // 2 is re-placed by its parent but keeps its cached layout, so its child 3 + // is never visited. + expect(entries.has(3)).toBe(false); + expect(entries.get(4)).toEqual({ x: 0, y: 50, w: 300, h: 40 }); + }); + + it('still emits render for a pass with nothing to lay out', async () => { + const { manager, pass } = await setup(); + + manager.queueRender(1); + + expect((await pass()).size).toBe(0); + }); + + it('resolves a percent translate under a clean, pruned subtree', async () => { + const { manager, pass } = await setup(); + + manager.applyStyle(3, { transform: { translateX: '-50%' } } as never); + + const entries = await pass(); + + expect(entries.get(3)).toEqual({ x: -20, y: 0, w: 40, h: 20 }); + + // Unchanged on the next pass: deduped, not re-emitted. + manager.applyStyle(11, { w: 30, h: 10 }); + expect((await pass()).has(3)).toBe(false); + + // Moves again when the node's own size changes. + manager.applyStyle(3, { w: 80, h: 20 }, true); + manager.applyStyle(3, { transform: { translateX: '-50%' } } as never); + expect((await pass()).get(3)).toEqual({ x: -40, y: 0, w: 80, h: 20 }); + }); + + it('lays out a node once when it becomes a root, even if it is clean', async () => { + const { manager, pass } = await setup(); + + manager.detachChildNode(1, 2); + manager.addIndependentRoot(2); + manager.queueRender(2); + + const entries = await pass(); + + expect(entries.get(2)).toEqual({ x: 0, y: 0, w: 40, h: 50 }); + }); +}); diff --git a/packages/plugin-flexbox/src/YogaManager.ts b/packages/plugin-flexbox/src/YogaManager.ts index bf50085f..dcece76f 100644 --- a/packages/plugin-flexbox/src/YogaManager.ts +++ b/packages/plugin-flexbox/src/YogaManager.ts @@ -77,6 +77,13 @@ export class YogaManager { // Text leaves currently measured by Yoga, so we can re-dirty them when a // font finishes loading. private _textNodes: Set = new Set(); + // Roots added since the last pass. A node that becomes a root can be clean + // (its subtree didn't change) but still needs laying out as a root once. + private _newRoots: Set = new Set(); + // Nodes with a percent translate. It resolves at readback and never dirties + // yoga, so these are read on every pass even where the walk is pruned. + private _percentNodes: Set = new Set(); + private _passId = 0; public on: EventEmitter['on'] = this._eventEmitter.on.bind(this._eventEmitter); public off: EventEmitter['off'] = this._eventEmitter.off.bind( @@ -288,6 +295,8 @@ export class YogaManager { const yogaNode = this._elementMap.get(elementId); if (yogaNode) { + this._percentNodes.delete(yogaNode); + this._newRoots.delete(yogaNode); // Detach from the parent's yoga node before freeing. Splicing only the // ManagerNode children array leaves the freed child in the parent's yoga // child list, so the parent keeps laying it out and a shrink-fit parent @@ -361,6 +370,7 @@ export class YogaManager { if (node) { this._independentRoots.add(node); + this._newRoots.add(node); } } @@ -369,6 +379,7 @@ export class YogaManager { if (node) { this._independentRoots.delete(node); + this._newRoots.delete(node); } } @@ -457,7 +468,15 @@ export class YogaManager { private _runLayoutPass(force: boolean): void { this._initializeArrayBuffer(); + const pass = ++this._passId; + for (const independentRoot of this._independentRoots) { + // A clean root lays out to the same result, so skip it. Yoga dirties + // every ancestor of a changed node, so the root's flag covers its tree. + if (!force && !independentRoot.node.isDirty() && !this._newRoots.has(independentRoot)) { + continue; + } + // undefined available size → yoga uses the root's own w/h (or // shrink-to-fit). Passing 1920×1080 would stretch any unset axis // and break measurement-driven roots like VirtualList cells. @@ -467,12 +486,30 @@ export class YogaManager { // oxlint-disable-next-line typescript/no-non-null-assertion -- Already checked this._yoga above this._yoga!.DIRECTION_LTR, ); - this._getUpdatedStyles(independentRoot, force); + this._getUpdatedStyles(independentRoot, force, pass); + } + + this._newRoots.clear(); + + for (const yogaNode of this._percentNodes) { + if (yogaNode.readbackPass !== pass && this._isInLayoutTree(yogaNode)) { + this._readNode(yogaNode, false, pass); + } } this._flushArrayBuffer(this._dataView.buffer); } + private _isInLayoutTree(yogaNode: ManagerNode): boolean { + let current = yogaNode; + + while (current.parent) { + current = current.parent; + } + + return this._independentRoots.has(current); + } + public applyStyles( styles: Record>, skipRender = false, @@ -605,7 +642,12 @@ export class YogaManager { pixelTranslate.y = translateY != null; } - touchedPercent ||= yogaNode.translatePercent !== undefined; + if (yogaNode.translatePercent === undefined) { + this._percentNodes.delete(yogaNode); + } else { + this._percentNodes.add(yogaNode); + touchedPercent = true; + } } } @@ -644,22 +686,38 @@ export class YogaManager { return yogaNode; } - // Recursion is unconditional — yoga's hasNewLayout is per-node, so a - // child's layout can change even when the parent's didn't (absolute - // children, just-attached subtrees from _reattachChildren). - private _getUpdatedStyles(yogaNode: ManagerNode, force = false) { + // A node without a new layout was served from yoga's cache and its subtree + // wasn't visited. Percent nodes under one are read in _runLayoutPass. + private _getUpdatedStyles(yogaNode: ManagerNode, force: boolean, pass: number) { + if (!this._readNode(yogaNode, force, pass)) { + return; + } + + const children = yogaNode.children; + + for (let i = 0, len = children.length; i < len; i++) { + // oxlint-disable-next-line typescript/no-non-null-assertion -- length-bounded + this._getUpdatedStyles(children[i]!, force, pass); + } + } + + /** Writes the node's layout to the buffer if it changed. Returns whether yoga laid it out. */ + private _readNode(yogaNode: ManagerNode, force: boolean, pass: number): boolean { const skipHiddenNode = !this._yogaOptions.processHiddenNodes && this._hiddenElements.has(yogaNode.id); // A percent translate never dirties yoga (there's no edge to write), so a - // percent node is visited on every pass; the dedupe below keeps it from + // percent node is read on every pass; the dedupe below keeps it from // re-emitting while its resolved position holds. const translatePercent = yogaNode.translatePercent; const hasNewLayout = force || yogaNode.node.hasNewLayout(); + if (translatePercent !== undefined) { + yogaNode.readbackPass = pass; + } + if (!skipHiddenNode && (hasNewLayout || translatePercent !== undefined)) { // Individual getters instead of getComputedLayout() — that allocates - // a {left, top, width, height} object per node, and we recurse the - // entire yoga tree every layout pass. + // a {left, top, width, height} object per node. const node = yogaNode.node; let left = node.getComputedLeft(); @@ -720,11 +778,6 @@ export class YogaManager { } } - const children = yogaNode.children; - - for (let i = 0, len = children.length; i < len; i++) { - // oxlint-disable-next-line typescript/no-non-null-assertion -- length-bounded - this._getUpdatedStyles(children[i]!, force); - } + return hasNewLayout; } } diff --git a/packages/plugin-flexbox/src/types/ManagerNode.ts b/packages/plugin-flexbox/src/types/ManagerNode.ts index 86558cfc..f8d306e0 100644 --- a/packages/plugin-flexbox/src/types/ManagerNode.ts +++ b/packages/plugin-flexbox/src/types/ManagerNode.ts @@ -17,6 +17,8 @@ export type ManagerNode = { translatePercent?: { x?: number; y?: number }; /** Last emitted resolved position for a percent node, to dedupe readback writes. */ resolvedTranslate?: { left: number; top: number }; + /** Last layout pass that read this percent node, so _runLayoutPass reads it once. */ + readbackPass?: number; /** Axes with a pixel translate inset currently written to yoga, so an omit can clear it. */ pixelTranslate?: { x?: boolean; y?: boolean }; }; From bce2ae673dd96c8b91e44bdc194e0559ccce3bcc Mon Sep 17 00:00:00 2001 From: Ruud Date: Thu, 15 Oct 2026 14:45:47 +0200 Subject: [PATCH 17/17] perf(flexbox): seed the grow-only text width from the first layout so mount runs one pass --- .changeset/flexbox-single-mount-pass.md | 5 + packages/plugin-flexbox/bench/baseline.json | 8 +- .../src/LightningManager.textGrow.test.ts | 211 ++++++++++++++++++ .../plugin-flexbox/src/LightningManager.ts | 35 +-- 4 files changed, 239 insertions(+), 20 deletions(-) create mode 100644 .changeset/flexbox-single-mount-pass.md create mode 100644 packages/plugin-flexbox/src/LightningManager.textGrow.test.ts diff --git a/.changeset/flexbox-single-mount-pass.md b/.changeset/flexbox-single-mount-pass.md new file mode 100644 index 00000000..1322fa67 --- /dev/null +++ b/.changeset/flexbox-single-mount-pass.md @@ -0,0 +1,5 @@ +--- +'@plextv/react-lightning-plugin-flexbox': patch +--- + +lay out a new text node once on mount instead of re-measuring it in a second pass diff --git a/packages/plugin-flexbox/bench/baseline.json b/packages/plugin-flexbox/bench/baseline.json index 50411c48..f5dd5e8d 100644 --- a/packages/plugin-flexbox/bench/baseline.json +++ b/packages/plugin-flexbox/bench/baseline.json @@ -1,6 +1,6 @@ { - "home": { "nodeCount": 875, "layoutPasses": 50, "settles": 25, "maxPassesToSettle": 2, "reflows": 0, "textRemeasures": 425 }, - "details": { "nodeCount": 670, "layoutPasses": 40, "settles": 20, "maxPassesToSettle": 2, "reflows": 0, "textRemeasures": 326 }, - "grid": { "nodeCount": 570, "layoutPasses": 60, "settles": 30, "maxPassesToSettle": 2, "reflows": 0, "textRemeasures": 180 }, - "epg": { "nodeCount": 832, "layoutPasses": 64, "settles": 32, "maxPassesToSettle": 2, "reflows": 0, "textRemeasures": 512 } + "home": { "nodeCount": 875, "layoutPasses": 25, "settles": 25, "maxPassesToSettle": 1, "reflows": 0, "textRemeasures": 0 }, + "details": { "nodeCount": 670, "layoutPasses": 20, "settles": 20, "maxPassesToSettle": 1, "reflows": 0, "textRemeasures": 0 }, + "grid": { "nodeCount": 570, "layoutPasses": 30, "settles": 30, "maxPassesToSettle": 1, "reflows": 0, "textRemeasures": 0 }, + "epg": { "nodeCount": 832, "layoutPasses": 32, "settles": 32, "maxPassesToSettle": 1, "reflows": 0, "textRemeasures": 0 } } diff --git a/packages/plugin-flexbox/src/LightningManager.textGrow.test.ts b/packages/plugin-flexbox/src/LightningManager.textGrow.test.ts new file mode 100644 index 00000000..eb3960b2 --- /dev/null +++ b/packages/plugin-flexbox/src/LightningManager.textGrow.test.ts @@ -0,0 +1,211 @@ +import { describe, expect, it, vi } from 'vitest'; + +import type { LightningElement } from '@plextv/react-lightning'; + +import { LightningManager } from './LightningManager'; +import type { AtlasData } from './text/FontMetricsStore'; +import type { YogaManager } from './YogaManager'; + +// Every glyph 10 units wide at size 10: at fontSize 20 a character is 20px and +// a line is 20px tall. +const atlas: AtlasData = { + info: { size: 10, face: 'Test' }, + common: { lineHeight: 12, base: 8 }, + lightningMetrics: { ascender: 800, descender: -200, lineGap: 0, unitsPerEm: 1000 }, + chars: Array.from({ length: 95 }, (_, i) => ({ + id: 32 + i, + xadvance: 10, + xoffset: 0, + yoffset: 0, + width: 8, + height: 8, + })), + kernings: [], +}; + +const atlasDataUrl = `data:application/json;base64,${Buffer.from(JSON.stringify(atlas)).toString('base64')}`; + +type Handler = (...args: unknown[]) => void; + +class FakeElement { + public parent: FakeElement | null = null; + public children: FakeElement[] = []; + public isTextElement = false; + public isImageElement = false; + public text?: string; + public props: { style: Record }; + public rawProps: { style: Record }; + public node: Record = {}; + public hasLayout = false; + + private _handlers = new Map>(); + + constructor( + public id: number, + public style: Record, + text?: string, + ) { + this.props = { style }; + this.rawProps = { style }; + + if (text !== undefined) { + this.isTextElement = true; + this.text = text; + } + } + + public on(event: string, handler: Handler): () => void { + let set = this._handlers.get(event); + + if (!set) { + set = new Set(); + this._handlers.set(event, set); + } + + set.add(handler); + + return () => set.delete(handler); + } + + public emit(event: string, ...args: unknown[]): void { + for (const handler of this._handlers.get(event) ?? []) { + handler(...args); + } + } + + public withholdPaintUntilLayout(): void {} + + public releaseWithheldPaint(): void {} + + public setNodeProp(key: string, value: unknown): boolean { + if (this.node[key] === value) { + return false; + } + + this.node[key] = value; + + return true; + } + + public emitLayoutEvent(): void { + this.hasLayout = true; + } +} + +const asElement = (fake: FakeElement) => fake as unknown as LightningElement; +const textStyle = { fontFamily: 'Test', fontSize: 20, maxLines: 0 }; + +// A hero block: a 1000px column holding a title container whose width starts +// narrow and resolves wider later (the collapsed-then-expanded hero title). +async function setup(containerStyle: Record, extra?: FakeElement) { + const manager = new LightningManager(); + await manager.init({ fonts: [{ fontFamily: 'Test', atlasDataUrl }] }); + + const yoga = (manager as unknown as { _yogaManager: YogaManager })._yogaManager; + const setTextMeasure = vi.spyOn(yoga, 'setTextMeasure'); + let passes = 0; + let settles = 0; + + yoga.on('render', () => passes++); + yoga.on('settled', () => settles++); + + const root = new FakeElement(1, { + w: 1000, + h: 600, + flexDirection: 'column', + alignItems: 'flex-start', + }); + const container = new FakeElement(2, containerStyle); + const title = new FakeElement(3, textStyle, 'aaaa aaaa aaaa aaaa'); + const tree: [FakeElement, FakeElement][] = [ + [root, container], + [container, title], + ]; + + if (extra) { + tree.push([container, extra]); + } + + for (const el of [root, container, title, extra]) { + if (el) { + manager.trackElement(asElement(el)); + manager.applyStyle(el.id, el.style, true); + } + } + + for (const [parent, child] of tree) { + child.parent = parent; + parent.children.push(child); + parent.emit('childAdded', child, parent.children.length - 1); + } + + manager.markFlexRoot(asElement(root)); + + const settle = async () => { + passes = 0; + settles = 0; + setTextMeasure.mockClear(); + + for (let i = 0; i < 20; i++) { + await Promise.resolve(); + } + + return { passes, settles, forced: setTextMeasure.mock.calls.filter((call) => call[3]).length }; + }; + + const resize = (el: FakeElement, style: Record) => { + el.style = style; + el.props.style = style; + el.rawProps.style = style; + manager.applyStyle(el.id, style, false, true); + }; + + return { manager, container, title, settle, resize, mount: await settle() }; +} + +describe('LightningManager grow-only text re-measure', () => { + it('lays a new text node out once on mount', async () => { + const { title, mount } = await setup({ w: 200, flexDirection: 'column' }); + + expect(mount).toEqual({ passes: 1, settles: 1, forced: 0 }); + // "aaaa aaaa" fits 200px: two lines of 180px. + expect(title.node.h).toBe(40); + }); + + it('re-measures the text when its container grows past the width it was measured at', async () => { + const { container, title, settle, resize } = await setup({ w: 200, flexDirection: 'column' }); + + resize(container, { w: 400, flexDirection: 'column' }); + + expect((await settle()).forced).toBe(1); + expect(title.node.h).toBe(20); + expect(title.node.w).toBe(400 + 2); + }); + + it('does not re-measure when the container shrinks, or regrows to a width it already had', async () => { + const { container, settle, resize } = await setup({ w: 400, flexDirection: 'column' }); + + resize(container, { w: 200, flexDirection: 'column' }); + expect((await settle()).forced).toBe(0); + + resize(container, { w: 400, flexDirection: 'column' }); + expect((await settle()).forced).toBe(0); + }); + + it('converges when the container width is derived from the text and a sibling', async () => { + const warn = vi.spyOn(console, 'warn'); + const logo = new FakeElement(4, { w: 120, h: 40 }); + const { settle, resize } = await setup({ flexDirection: 'row', alignSelf: 'flex-start' }, logo); + + for (const w of [500, 120, 700, 60]) { + resize(logo, { w, h: 40 }); + + const { settles } = await settle(); + + expect(settles).toBe(1); + } + + expect(warn).not.toHaveBeenCalled(); + warn.mockRestore(); + }); +}); diff --git a/packages/plugin-flexbox/src/LightningManager.ts b/packages/plugin-flexbox/src/LightningManager.ts index ee0ce520..67bd1ea6 100644 --- a/packages/plugin-flexbox/src/LightningManager.ts +++ b/packages/plugin-flexbox/src/LightningManager.ts @@ -559,6 +559,16 @@ export class LightningManager { } } + private _regrowText = (measuredAt: number, textId: number) => { + const textEl = this._elements.get(textId); + const contextWidth = textEl?.parent?.node.w ?? 0; + + if (textEl && contextWidth > measuredAt) { + this._textContextWidth.set(textId, contextWidth); + this._syncTextMeasure(textEl, true); + } + }; + private _applyUpdates = (buffer: ArrayBuffer) => { // Raw `DataView` over `SimpleDataView` — this is a per-frame hot path // that doesn't need overflow handling or write tracking. @@ -614,6 +624,12 @@ export class LightningManager { // renderer wraps to the same width and textAlign has a box to align in. const isMeasuredText = isText && this._measuredText.has(elementId); + // Its first layout measured it against the parent's width (parents come + // first in the buffer), so that's the baseline. Seeding 0 re-ran mount. + if (isMeasuredText && !this._textContextWidth.has(elementId)) { + this._textContextWidth.set(elementId, el.parent?.node.w ?? 0); + } + if (width !== 0 && (!isText || isMeasuredText)) { if (isMeasuredText) { // `contain` lives on the text node, not the base node type — set it @@ -669,21 +685,8 @@ export class LightningManager { // the widest the container ever offered, which never clips. New text on a // recycled node is handled separately by the `textChanged` → setTextMeasure // measure-func reinstall, so a narrower reuse still re-measures correctly. - if (this._measuredText.size > 0) { - for (const textId of this._measuredText) { - const textEl = this._elements.get(textId); - - if (!textEl) { - continue; - } - - const contextWidth = textEl.parent?.node.w ?? 0; - - if (contextWidth > (this._textContextWidth.get(textId) ?? 0)) { - this._textContextWidth.set(textId, contextWidth); - this._syncTextMeasure(textEl, true); - } - } - } + // Only texts laid out since they were measured have a baseline, the rest + // measure fresh. forEach because for...of allocates per entry in lower JIT tiers. + this._textContextWidth.forEach(this._regrowText); }; }