From 6d89cf13b27887bd61885748734cf9e244cef8c0 Mon Sep 17 00:00:00 2001 From: Taku Amano Date: Sun, 9 Jun 2024 16:48:54 +0900 Subject: [PATCH 1/3] refactor(jsx): Refactored an utility method to reduce code size and improve maintainability --- src/jsx/base.ts | 10 ++--- src/jsx/dom/jsx-dev-runtime.ts | 4 -- src/jsx/dom/render.ts | 8 ++-- src/jsx/utils.test.ts | 77 +++++----------------------------- src/jsx/utils.ts | 22 +++------- 5 files changed, 24 insertions(+), 97 deletions(-) diff --git a/src/jsx/base.ts b/src/jsx/base.ts index 36b430545..c250a9123 100644 --- a/src/jsx/base.ts +++ b/src/jsx/base.ts @@ -7,7 +7,7 @@ import type { JSX as HonoJSX, IntrinsicElements as IntrinsicElementsDefined, } from './intrinsic-elements' -import { normalizeIntrinsicElementProps, styleObjectForEach } from './utils' +import { normalizeIntrinsicElementKey, styleObjectForEach } from './utils' // eslint-disable-next-line @typescript-eslint/no-explicit-any export type Props = Record @@ -152,11 +152,8 @@ export class JSXNode implements HtmlEscaped { buffer[0] += `<${tag}` - const propsKeys = Object.keys(props || {}) - - for (let i = 0, len = propsKeys.length; i < len; i++) { - const key = propsKeys[i] - const v = props[key] + for (let [key, v] of Object.entries(props)) { + key = normalizeIntrinsicElementKey(key) if (key === 'children') { // skip children } else if (key === 'style' && typeof v === 'object') { @@ -283,7 +280,6 @@ export const jsxFn = ( if (typeof tag === 'function') { return new JSXFunctionNode(tag, props, children) } else { - normalizeIntrinsicElementProps(props) return new JSXNode(tag, props, children) } } diff --git a/src/jsx/dom/jsx-dev-runtime.ts b/src/jsx/dom/jsx-dev-runtime.ts index 31548b7aa..858e607fc 100644 --- a/src/jsx/dom/jsx-dev-runtime.ts +++ b/src/jsx/dom/jsx-dev-runtime.ts @@ -4,13 +4,9 @@ */ import type { JSXNode, Props } from '../base' -import { normalizeIntrinsicElementProps } from '../utils' import { newJSXNode } from './utils' export const jsxDEV = (tag: string | Function, props: Props, key?: string): JSXNode => { - if (typeof tag === 'string') { - normalizeIntrinsicElementProps(props) - } return newJSXNode({ tag, props, diff --git a/src/jsx/dom/render.ts b/src/jsx/dom/render.ts index 530b0eb1a..ddbaccfc7 100644 --- a/src/jsx/dom/render.ts +++ b/src/jsx/dom/render.ts @@ -5,7 +5,7 @@ import type { Context as JSXContext } from '../context' import { globalContexts as globalJSXContexts, useContext } from '../context' import type { EffectData } from '../hooks' import { STASH_EFFECT } from '../hooks' -import { styleObjectForEach } from '../utils' +import { normalizeIntrinsicElementKey, styleObjectForEach } from '../utils' import { createContext } from './context' // import dom-specific versions import { newJSXNode } from './utils' @@ -120,8 +120,9 @@ const applyProps = ( oldAttributes?: Props ): void => { attributes ||= {} - for (const [key, value] of Object.entries(attributes)) { + for (let [key, value] of Object.entries(attributes)) { if (!skipProps.has(key) && (!oldAttributes || oldAttributes[key] !== value)) { + key = normalizeIntrinsicElementKey(key) const eventSpec = getEventSpec(key) if (eventSpec) { if (oldAttributes) { @@ -191,8 +192,9 @@ const applyProps = ( } } if (oldAttributes) { - for (const [key, value] of Object.entries(oldAttributes)) { + for (let [key, value] of Object.entries(oldAttributes)) { if (!skipProps.has(key) && !(key in attributes)) { + key = normalizeIntrinsicElementKey(key) const eventSpec = getEventSpec(key) if (eventSpec) { container.removeEventListener(eventSpec[0], value, eventSpec[1]) diff --git a/src/jsx/utils.test.ts b/src/jsx/utils.test.ts index 5bf488e61..5d78e009b 100644 --- a/src/jsx/utils.test.ts +++ b/src/jsx/utils.test.ts @@ -1,70 +1,13 @@ -import { normalizeIntrinsicElementProps, styleObjectForEach } from './utils' - -describe('normalizeIntrinsicElementProps', () => { - it('should convert className to class', () => { - const props: Record = { - className: 'test-class', - id: 'test-id', - } - - normalizeIntrinsicElementProps(props) - - expect(props).toEqual({ - class: 'test-class', - id: 'test-id', - }) - }) - - it('should convert htmlFor to for', () => { - const props: Record = { - htmlFor: 'test-for', - name: 'test-name', - } - - normalizeIntrinsicElementProps(props) - - expect(props).toEqual({ - for: 'test-for', - name: 'test-name', - }) - }) - - it('should convert multiple attribute aliases', () => { - const props: Record = { - className: 'test-class', - htmlFor: 'test-for', - type: 'text', - } - - normalizeIntrinsicElementProps(props) - - expect(props).toEqual({ - class: 'test-class', - for: 'test-for', - type: 'text', - }) - }) - - it('should not modify props without className or htmlFor', () => { - const props: Record = { - id: 'test-id', - name: 'test-name', - } - - normalizeIntrinsicElementProps(props) - - expect(props).toEqual({ - id: 'test-id', - name: 'test-name', - }) - }) - - it('should handle empty props', () => { - const props: Record = {} - - normalizeIntrinsicElementProps(props) - - expect(props).toEqual({}) +import { normalizeIntrinsicElementKey, styleObjectForEach } from './utils' + +describe('normalizeIntrinsicElementKey', () => { + test.each` + key | expected + ${'className'} | ${'class'} + ${'htmlFor'} | ${'for'} + ${'href'} | ${'href'} + `('should convert $key to $expected', ({ key, expected }) => { + expect(normalizeIntrinsicElementKey(key)).toBe(expected) }) }) diff --git a/src/jsx/utils.ts b/src/jsx/utils.ts index 716ffcb3a..331a280bd 100644 --- a/src/jsx/utils.ts +++ b/src/jsx/utils.ts @@ -1,19 +1,9 @@ -/** - * Normalizes intrinsic element properties by converting JSX element properties - * to their corresponding HTML attributes. - * - * @param props - JSX element properties. - */ -export const normalizeIntrinsicElementProps = (props: Record): void => { - if (props && 'className' in props) { - props['class'] = props['className'] - delete props['className'] - } - if (props && 'htmlFor' in props) { - props['for'] = props['htmlFor'] - delete props['htmlFor'] - } -} +const normalizeElementKeyMap = new Map([ + ['className', 'class'], + ['htmlFor', 'for'], +]) +export const normalizeIntrinsicElementKey = (key: string): string => + normalizeElementKeyMap.get(key) || key export const styleObjectForEach = ( style: Record, From c0fade3c7c5ad3525a4d00906a80dde35e393633 Mon Sep 17 00:00:00 2001 From: Taku Amano Date: Mon, 10 Jun 2024 08:45:02 +0900 Subject: [PATCH 2/3] refactor: tweaks shared namespace string for svg and math for reduced size --- src/jsx/dom/render.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/jsx/dom/render.ts b/src/jsx/dom/render.ts index ddbaccfc7..b4bd9fb4c 100644 --- a/src/jsx/dom/render.ts +++ b/src/jsx/dom/render.ts @@ -17,8 +17,8 @@ const eventAliasMap: Record = { } as const const nameSpaceMap: Record = { - svg: 'http://www.w3.org/2000/svg', - math: 'http://www.w3.org/1998/Math/MathML', + svg: '2000/svg', + math: '1998/Math/MathML', } as const const skipProps: Set = new Set(['children']) @@ -511,7 +511,7 @@ export const buildNode = (node: Child): Node | undefined => { } else { const ns = nameSpaceMap[(node as JSXNode).tag as string] if (ns) { - ;(node as NodeObject).n = ns + ;(node as NodeObject).n = `http://www.w3.org/${ns}` nameSpaceContext ||= createContext('') ;(node as JSXNode).props.children = [ { From f4e0fb3a635ad7ba196cb864dd5c90f5c4b7e202 Mon Sep 17 00:00:00 2001 From: Taku Amano Date: Tue, 11 Jun 2024 05:44:30 +0900 Subject: [PATCH 3/3] fix: degration of c0fade3 --- src/jsx/dom/render.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/jsx/dom/render.ts b/src/jsx/dom/render.ts index b4bd9fb4c..ca05ad6d8 100644 --- a/src/jsx/dom/render.ts +++ b/src/jsx/dom/render.ts @@ -511,13 +511,12 @@ export const buildNode = (node: Child): Node | undefined => { } else { const ns = nameSpaceMap[(node as JSXNode).tag as string] if (ns) { - ;(node as NodeObject).n = `http://www.w3.org/${ns}` nameSpaceContext ||= createContext('') ;(node as JSXNode).props.children = [ { tag: nameSpaceContext.Provider, props: { - value: ns, + value: ((node as NodeObject).n = `http://www.w3.org/${ns}`), children: (node as JSXNode).props.children, }, },