From 461c346d6acf13782ef9cac905f0b62a274c5ef2 Mon Sep 17 00:00:00 2001 From: kasperskei Date: Mon, 6 Jan 2025 02:59:55 +0200 Subject: [PATCH] fix(jsx): props are set as properties --- packages/jsx/src/index.test.tsx | 28 +++++------- packages/jsx/src/index.ts | 79 ++++++++++++++++++++++----------- 2 files changed, 63 insertions(+), 44 deletions(-) diff --git a/packages/jsx/src/index.test.tsx b/packages/jsx/src/index.test.tsx index 5c65dbf1..ec2f57b9 100644 --- a/packages/jsx/src/index.test.tsx +++ b/packages/jsx/src/index.test.tsx @@ -38,27 +38,21 @@ it('static props & children', setup((ctx, h, hf, mount, parent) => { })) it('dynamic props', setup((ctx, h, hf, mount, parent) => { - const val = atom('val', 'val') const prp = atom('prp', 'prp') const atr = atom('atr', 'atr') - const element =
+ const element =
mount(parent, element) - assert.is(element.id, 'val') - // @ts-expect-error `dunno` can't be inferred - assert.is(element.prp, 'prp') - assert.is(element.getAttribute('atr'), 'atr') + assert.is(element.id, 'prp') + assert.is(element.getAttribute('class'), 'atr') - val(ctx, 'val1') prp(ctx, 'prp1') atr(ctx, 'atr1') - assert.is(element.id, 'val1') - // @ts-expect-error `dunno` can't be inferred - assert.is(element.prp, 'prp1') - assert.is(element.getAttribute('atr'), 'atr1') + assert.is(element.id, 'prp1') + assert.is(element.getAttribute('class'), 'atr1') })) it('children updates', setup((ctx, h, hf, mount, parent) => { @@ -390,12 +384,12 @@ it('same arguments in ref mount and unmount hooks', setup(async (ctx, h, hf, mou assert.is(unmountArgs[1], component) })) -it('css property and class attribute', setup(async (ctx, h, hf, mount, parent) => { +it('css property and className property', setup(async (ctx, h, hf, mount, parent) => { const cls = 'class' const css = 'color: red;' - const ref1 = (
) - const ref2 = (
) + const ref1 = (
) + const ref2 = (
) const component = (
@@ -445,7 +439,7 @@ it('css custom property', setup(async (ctx, h, hf, mount, parent) => { assert.is(component.style.getPropertyValue('--secondProperty'), '') })) -it('class and className attribute', setup(async (ctx, h, hf, mount, parent) => { +it('class and className properties', setup(async (ctx, h, hf, mount, parent) => { const classAtom = atom('' as string | undefined) const ref1 = (
) @@ -473,8 +467,8 @@ it('class and className attribute', setup(async (ctx, h, hf, mount, parent) => { classAtom(ctx, undefined) assert.is(ref1.className, '') assert.is(ref2.className, '') - assert.ok(!ref1.hasAttribute('class')) - assert.ok(!ref2.hasAttribute('class')) + assert.ok(ref1.hasAttribute('class')) + assert.ok(ref2.hasAttribute('class')) })) it('ref mount and unmount callbacks order', setup(async (ctx, h, hf, mount, parent) => { diff --git a/packages/jsx/src/index.ts b/packages/jsx/src/index.ts index 97d42d1e..bf2c4217 100644 --- a/packages/jsx/src/index.ts +++ b/packages/jsx/src/index.ts @@ -16,6 +16,25 @@ type DomApis = Pick< 'document' | 'Node' | 'Text' | 'Element' | 'MutationObserver' | 'HTMLElement' | 'DocumentFragment' > +/** + * @see https://github.com/preactjs/preact/blob/d16a34e275e31afd6738a9f82b5ba2fb9dbf032b/src/diff/props.js#L107 + * @see https://www.measurethat.net/Benchmarks/Show/7818 + */ +const propertiesAsAttribute = new Set([ + 'width', + 'height', + 'href', + 'list', + 'form', + /** Default value in browsers is `-1` and an empty string is cast to `0` instead */ + 'tabIndex', + 'download', + 'rowSpan', + 'colSpan', + 'role', + 'popover', +]) + const isSkipped = (value: unknown): value is boolean | '' | null | undefined => typeof value === 'boolean' || value === '' || value == null @@ -83,12 +102,21 @@ const walkLinkedList = (ctx: Ctx, el: JSX.Element, list: Atom { - const StylesheetId = 'reatom-jsx-styles' - let styles: Rec = {} - let stylesheet: HTMLStyleElement | undefined +export const reatomJsx = (ctx: Ctx, DOM: DomApis = globalThis.window, options?: { + /** + * Adds a style element containing styles from the `css` property to the document. + * @default true + */ + appendStylesheet?: boolean +}) => { let name = '' + /** @see https://www.measurethat.net/Benchmarks/Show/33272 */ + let styles: Rec = {} + let stylesheet: HTMLStyleElement = DOM.document.createElement('style') + stylesheet.id = 'reatom-jsx-styles' + if (options?.appendStylesheet !== false) DOM.document.head.appendChild(stylesheet) + let set = (element: JSX.Element, key: string, val: any) => { if (key.startsWith('on:')) { key = key.slice(3) @@ -100,13 +128,6 @@ export const reatomJsx = (ctx: Ctx, DOM: DomApis = globalThis.window) => { if (val == null) element.style.removeProperty(key) else element.style.setProperty(key, String(val)) } else if (key === 'css') { - stylesheet ??= DOM.document.getElementById(StylesheetId) as any - if (!stylesheet) { - stylesheet = DOM.document.createElement('style') - stylesheet.id = StylesheetId - DOM.document.head.appendChild(stylesheet) - } - let styleId = styles[val] if (!styleId) { styleId = styles[val] = `${name ? name + '.' : ''}${random(0, 1e6).toString()}` @@ -119,24 +140,28 @@ export const reatomJsx = (ctx: Ctx, DOM: DomApis = globalThis.window) => { if (val[key] == null) element.style.removeProperty(key) else element.style.setProperty(key, val[key]) } - } else if (key.startsWith('prop:')) { - // @ts-expect-error - element[key.slice(5)] = val + } else if ( + !propertiesAsAttribute.has(key) + && element instanceof DOM.HTMLElement + && (key in element || key === 'class') + ) { + if (key === 'class') key = 'className' + // @ts-ignore + element[key] = val == null ? '' : val } else { - if (key.startsWith('attr:')) { - key = key.slice(5) - } if (key === 'className') key = 'class' - if (val == null || val === false) element.removeAttribute(key) - else { - val = val === true ? '' : String(val) - /** - * @see https://measurethat.net/Benchmarks/Show/54 - * @see https://measurethat.net/Benchmarks/Show/31249 - */ - if (key === 'class' && element instanceof HTMLElement) element.className = val - else element.setAttribute(key, val) - } + if (key.startsWith('attr:')) key = key.slice(5) + + /** + * @note aria- and data- attributes have no boolean representation. + * A `false` value is different from the attribute not being + * present, so we can't remove it. For non-boolean aria + * attributes we could treat false as a removal, but the + * amount of exceptions would cost too many bytes. On top of + * that other frameworks generally stringify `false`. + */ + if (val == null || (val === false && key[4] !== '-')) element.removeAttribute(key) + else element.setAttribute(key, key == 'popover' && val == true ? '' : val) } }