From 975b892bdb470be06182d172d9fb83c8c77cdaad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Teemu=20P=C3=A4=C3=A4kk=C3=B6nen?= Date: Sun, 10 Dec 2023 15:36:40 +0200 Subject: [PATCH] Rework lazy property system to not create unnecessary lambdas --- build.js | 7 +- src/kaiku.ts | 242 +++++++++++++++++++++++++++++++++++---------------- 2 files changed, 171 insertions(+), 78 deletions(-) diff --git a/build.js b/build.js index 48b8aab..af41303 100644 --- a/build.js +++ b/build.js @@ -22,7 +22,8 @@ fs.writeFileSync( FragmentTag: '"Fragment"', TextNodeTag: '"TextNode"', EffectTag: '"EffectTag"', - LazyUpdateTag: '"LazyUpdateTag"', + LazyPropUpdateTag: '"LazyPropUpdateTag"', + LazyStyleUpdateTag: '"LazyStyleUpdateTag"', }, }).code ) @@ -44,7 +45,8 @@ fs.writeFileSync( FragmentTag: tagId++, TextNodeTag: tagId++, EffectTag: tagId++, - LazyUpdateTag: tagId++, + LazyPropUpdateTag: tagId++, + LazyStyleUpdateTag: tagId++, }, }).code ) @@ -80,7 +82,6 @@ terser 'componentWillUnmount', 'Fragment', 'state', - 'props', ], }, }, diff --git a/src/kaiku.ts b/src/kaiku.ts index e618ded..9bda4a8 100644 --- a/src/kaiku.ts +++ b/src/kaiku.ts @@ -26,7 +26,8 @@ declare const HtmlElementTag = 'HtmlElement' declare const FragmentTag = 'Fragment' declare const TextNodeTag = 'TextNode' declare const EffectTag = 'EffectTag' -declare const LazyUpdateTag = 'LazyUpdateTag' +declare const LazyPropUpdateTag = 'LazyPropUpdateTag' +declare const LazyStyleUpdateTag = 'LazyStyleUpdateTag' const CLASS_COMPONENT_FLAG = Symbol() const IMMUTABLE_FLAG = Symbol() @@ -46,7 +47,7 @@ type HtmlElementTagName = | keyof HTMLElementTagNameMap | keyof SVGElementTagNameMap type HtmlElementProperties = Record & { - style?: Record + style?: Record string)> className?: ClassNames } @@ -55,7 +56,6 @@ export type WithIntrinsicProps = T & { children?: Child | Children key?: string } -type LazyProperty = T | (() => T) type ClassComponentDescriptor< PropertiesT extends DefaultProps, @@ -142,7 +142,7 @@ type HtmlElementInstance = { parentElement_: HtmlElementInstance | null nextSibling_: NodeInstance | null children_: FragmentInstance | null - lazyUpdates: LazyUpdate[] + lazyUpdates: (LazyPropUpdate | LazyStyleUpdate)[] } type TextInstance = { @@ -216,12 +216,22 @@ type Effect = { unsubscribe_?: () => void } -type LazyUpdate = { +type LazyPropUpdate = { id_: DependeeId - tag_: typeof LazyUpdateTag - prop: () => T - handler: (value: T) => void - lastValue: T | undefined + tag_: typeof LazyPropUpdateTag + element_: HTMLElement | SVGElement + property: string + callback: () => T + previousValue: T | undefined +} + +type LazyStyleUpdate = { + id_: DependeeId + tag_: typeof LazyStyleUpdateTag + element_: HTMLElement + property: string + callback: () => T + previousValue: T | undefined } type Ref = { @@ -232,7 +242,8 @@ type Dependee = | ClassComponentInstance | FunctionComponentInstance | Effect - | LazyUpdate + | LazyPropUpdate + | LazyStyleUpdate type Render = ( rootDescriptor: NodeDescriptor, @@ -293,8 +304,12 @@ const updateDependee = (dependee: Dependee) => { runEffect(dependee) break } - case LazyUpdateTag: { - runLazyUpdate(dependee) + case LazyPropUpdateTag: { + runLazyPropUpdate(dependee) + break + } + case LazyStyleUpdateTag: { + runLazyStyleUpdate(dependee) break } } @@ -1008,37 +1023,109 @@ const createTextInstance = (descriptor: TextDescriptor): TextInstance => { // /////////////// -const runLazyUpdate = (lazyUpdate: LazyUpdate) => { - const { prop, handler, lastValue } = lazyUpdate +// TODO: This should probably be inlined somehow, but esbuild does not preserve comments +// to be passed down to Terser, so /*@__INLINE__*/ comments don't work. +const updateElementClassName = (element: HTMLElement, value: ClassNames) => { + element.className = stringifyClassNames(value ?? '') +} + +// TODO: Should be inlined. See above. +const updateElementValue = (element: HTMLElement | SVGElement, value: any) => { + ;(element as HTMLInputElement).value = value +} - const value = trackedExecute(lazyUpdate, prop) - if (value !== lastValue) { - lazyUpdate.lastValue = value - handler(value) +// TODO: Should be inlined. See above. +const updateElementProperty = ( + element: HTMLElement | SVGElement, + property: string, + value: any +) => { + if (typeof value === 'undefined' || value === false || value === null) { + element.removeAttribute(property) + } else { + element.setAttribute(property, value) } } -const lazy = ( +// TODO: Should be inlined. See above. +const updateElementStyle = ( + element: HTMLElement | SVGElement, + property: string, + value: any +) => { + element.style[property as any] = value +} + +const registerLazyPropUpdate = ( instance: HtmlElementInstance, - prop: LazyProperty, - handler: (value: T) => void + property: string, + callback: () => T ) => { - if (typeof prop !== 'function') { - handler(prop) - return + const propUpdate: LazyPropUpdate = { + id_: ++nextDependeeId as DependeeId, + tag_: LazyPropUpdateTag, + element_: instance.element_, + property, + callback, + previousValue: undefined, } - const lazyUpdate: LazyUpdate = { + instance.lazyUpdates.push(propUpdate) + runLazyPropUpdate(propUpdate) +} + +const runLazyPropUpdate = (propUpdate: LazyPropUpdate) => { + const value = trackedExecute(propUpdate, propUpdate.callback) + + if (value !== propUpdate.previousValue) { + propUpdate.previousValue = value + + switch (propUpdate.property) { + case 'value': { + updateElementValue(propUpdate.element_, value) + break + } + case 'class': + case 'className': { + updateElementClassName( + propUpdate.element_ as HTMLElement, + value as ClassNames + ) + break + } + default: { + updateElementProperty(propUpdate.element_, propUpdate.property, value) + break + } + } + } +} + +const registerLazyStyleUpdate = ( + instance: HtmlElementInstance, + property: string, + callback: () => T +) => { + const styleUpdate: LazyStyleUpdate = { id_: ++nextDependeeId as DependeeId, - tag_: LazyUpdateTag, - lastValue: undefined, - prop: prop as () => T, - handler, + tag_: LazyStyleUpdateTag, + element_: instance.element_ as HTMLElement, + property, + callback, + previousValue: undefined, } - runLazyUpdate(lazyUpdate) + instance.lazyUpdates.push(styleUpdate) + runLazyStyleUpdate(styleUpdate) +} + +const runLazyStyleUpdate = (styleUpdate: LazyStyleUpdate) => { + const value = trackedExecute(styleUpdate, styleUpdate.callback) - instance.lazyUpdates.push(lazyUpdate) + if (value !== styleUpdate.previousValue) { + styleUpdate.previousValue = value + updateElementStyle(styleUpdate.element_, styleUpdate.property, value) + } } const destroyLazyUpdates = (instance: HtmlElementInstance) => { @@ -1127,8 +1214,32 @@ const updateHtmlElementInstance = ( ) => { const keys = unionOfKeys(nextProps, instance.props) + // Handle the style prop + const properties = unionOfKeys( + nextProps.style || + (EMPTY_OBJECT as Exclude), + instance.props.style || + (EMPTY_OBJECT as Exclude) + ) + + for (const property of properties) { + const prevValue = instance.props.style?.[property] + const value = nextProps.style?.[property] + + if (prevValue !== value) { + if (typeof value === 'function') { + registerLazyStyleUpdate(instance, property, value) + } else { + updateElementStyle(instance.element_, property, value) + } + } + } + + // Handle properties other than `style` for (const key of keys) { - // TODO: Special case access to style and classsnames + if (key === 'style') continue + + // TODO: Special case access to classsnames if (instance.props[key] === nextProps[key]) continue if (key === 'key') continue @@ -1158,57 +1269,38 @@ const updateHtmlElementInstance = ( } } else { switch (key) { - case 'style': { - const properties = unionOfKeys( - nextProps.style || EMPTY_OBJECT, - instance.props.style || EMPTY_OBJECT - ) - - for (const property of properties) { - if ( - nextProps.style?.[property] !== instance.props.style?.[property] - ) { - lazy(instance, nextProps.style?.[property] ?? '', (value) => { - instance.element_.style[property as any] = value - }) - } - } - continue - } case 'value': { - lazy(instance, nextProps[key] ?? '', (value) => { - ;(instance.element_ as HTMLInputElement).value = value - }) - continue + if (typeof nextProps[key] === 'function') { + registerLazyPropUpdate(instance, key, nextProps[key]) + } else { + updateElementValue(instance.element_ as HTMLElement, nextProps[key]) + } + break } + case 'class': case 'className': { - lazy(instance, nextProps[key], (value) => { - ;(instance.element_.className as any) = stringifyClassNames( - value ?? '' + if (typeof nextProps[key] === 'function') { + registerLazyPropUpdate(instance, key, nextProps[key]) + } else { + updateElementClassName( + instance.element_ as HTMLElement, + nextProps[key] ) - }) - continue + } + break } - } - - if (key in nextProps) { - lazy( - instance, - nextProps[key] as LazyProperty, - (value) => { - if ( - typeof value === 'undefined' || - value === false || - value === null - ) { - instance.element_.removeAttribute(key) + default: { + if (key in nextProps) { + if (typeof nextProps[key] === 'function') { + registerLazyPropUpdate(instance, key, nextProps[key]) } else { - instance.element_.setAttribute(key, value) + updateElementProperty(instance.element_, key, nextProps[key]) } + } else { + instance.element_.removeAttribute(key) } - ) - } else { - instance.element_.removeAttribute(key) + break + } } } }