Skip to content

Commit

Permalink
refactor(jsx): reduce code size and improve maintainability (#2956)
Browse files Browse the repository at this point in the history
* refactor(jsx): Refactored an utility method to reduce code size and improve maintainability

* refactor: tweaks shared namespace string for svg and math for reduced size

* fix: degration of c0fade3
  • Loading branch information
usualoma authored Jun 11, 2024
1 parent 32ba5ba commit ad04fbf
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 101 deletions.
10 changes: 3 additions & 7 deletions src/jsx/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, any>
Expand Down Expand Up @@ -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') {
Expand Down Expand Up @@ -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)
}
}
Expand Down
4 changes: 0 additions & 4 deletions src/jsx/dom/jsx-dev-runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
15 changes: 8 additions & 7 deletions src/jsx/dom/render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand All @@ -17,8 +17,8 @@ const eventAliasMap: Record<string, string> = {
} as const

const nameSpaceMap: Record<string, string> = {
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<string> = new Set(['children'])
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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])
Expand Down Expand Up @@ -509,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 = 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,
},
},
Expand Down
77 changes: 10 additions & 67 deletions src/jsx/utils.test.ts
Original file line number Diff line number Diff line change
@@ -1,70 +1,13 @@
import { normalizeIntrinsicElementProps, styleObjectForEach } from './utils'

describe('normalizeIntrinsicElementProps', () => {
it('should convert className to class', () => {
const props: Record<string, unknown> = {
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<string, unknown> = {
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<string, unknown> = {
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<string, unknown> = {
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<string, unknown> = {}

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)
})
})

Expand Down
22 changes: 6 additions & 16 deletions src/jsx/utils.ts
Original file line number Diff line number Diff line change
@@ -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<string, unknown>): 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<string, string | number>,
Expand Down

0 comments on commit ad04fbf

Please sign in to comment.