From 3d783f18ad6fb76b241b145acd4110bbe04e291d Mon Sep 17 00:00:00 2001 From: Taku Amano Date: Mon, 25 Nov 2024 08:18:14 +0900 Subject: [PATCH] fix: add "key" attribute to suppress react "key" warnings (#238) Fixes #233 * fix: add key attribute to placeholder template element To suppress react key warnings in SSR. * fix(client): add key attribute to each nested element in runtime To suppress react key warnings in runtime. * refactor: use `createElement` instead of `jsx` for compatibility with react --- package.json | 1 + src/client/client.ts | 5 ++--- src/client/runtime.test.ts | 21 +++++++++++++++++++++ src/client/runtime.ts | 13 +++++++++++-- src/vite/components/honox-island.test.tsx | 18 ++++++++++++++++++ src/vite/components/honox-island.tsx | 2 +- vitest.config.ts | 1 + yarn.lock | 21 ++++++++++++++++++++- 8 files changed, 75 insertions(+), 7 deletions(-) create mode 100644 src/client/runtime.test.ts create mode 100644 src/vite/components/honox-island.test.tsx diff --git a/package.json b/package.json index a0376ce..13e3bdc 100644 --- a/package.json +++ b/package.json @@ -129,6 +129,7 @@ "@types/node": "^20.10.5", "eslint": "^9.10.0", "glob": "^10.3.10", + "happy-dom": "^15.11.6", "hono": "4.4.13", "np": "7.7.0", "prettier": "^3.1.1", diff --git a/src/client/client.ts b/src/client/client.ts index 3f7197e..52bf38b 100644 --- a/src/client/client.ts +++ b/src/client/client.ts @@ -1,5 +1,4 @@ -import { render } from 'hono/jsx/dom' -import { jsx as jsxFn } from 'hono/jsx/dom/jsx-runtime' +import { render, createElement as createElementHono } from 'hono/jsx/dom' import { COMPONENT_EXPORT, COMPONENT_NAME, @@ -61,7 +60,7 @@ export const createClient = async (options?: ClientOptions) => { const props = JSON.parse(serializedProps ?? '{}') as Record const hydrate = options?.hydrate ?? render - const createElement = options?.createElement ?? jsxFn + const createElement = options?.createElement ?? createElementHono let maybeTemplate = element.childNodes[element.childNodes.length - 1] while (maybeTemplate?.nodeName === 'TEMPLATE') { diff --git a/src/client/runtime.test.ts b/src/client/runtime.test.ts new file mode 100644 index 0000000..1c3daf9 --- /dev/null +++ b/src/client/runtime.test.ts @@ -0,0 +1,21 @@ +import { buildCreateChildrenFn } from './runtime' + +describe('buildCreateChildrenFn', () => { + it('should set key for children', async () => { + const createElement = vi.fn() + const importComponent = vi.fn() + const createChildren = buildCreateChildrenFn(createElement, importComponent) + + const div = document.createElement('div') + div.innerHTML = 'test
test2
' + const result = await createChildren(div.childNodes) + expect(createElement).toHaveBeenNthCalledWith(1, 'SPAN', { + children: ['test'], + key: 1, + }) + expect(createElement).toHaveBeenNthCalledWith(2, 'DIV', { + children: ['test2'], + key: 2, + }) + }) +}) diff --git a/src/client/runtime.ts b/src/client/runtime.ts index 18929b9..e4739bf 100644 --- a/src/client/runtime.ts +++ b/src/client/runtime.ts @@ -7,6 +7,7 @@ export const buildCreateChildrenFn = ( createElement: CreateElement, importComponent: ImportComponent ): CreateChildren => { + let keyIndex = 0 const setChildrenFromTemplate = async (props: { children?: Node[] }, element: HTMLElement) => { const maybeTemplate = element.childNodes[element.childNodes.length - 1] if ( @@ -26,7 +27,10 @@ export const buildCreateChildrenFn = ( for (let i = 0; i < attributes.length; i++) { props[attributes[i].name] = attributes[i].value } - return createElement(element.nodeName, props) + return createElement(element.nodeName, { + key: ++keyIndex, + ...props, + }) } const createChildren = async (childNodes: NodeListOf): Promise => { const children = [] @@ -117,7 +121,12 @@ export const buildCreateChildrenFn = ( if (component) { const props = JSON.parse(child.getAttribute(DATA_SERIALIZED_PROPS) || '{}') await setChildrenFromTemplate(props, child) - children.push(await createElement(component, props)) + children.push( + await createElement(component, { + key: ++keyIndex, + ...props, + }) + ) } else { children.push(await createElementFromHTMLElement(child)) } diff --git a/src/vite/components/honox-island.test.tsx b/src/vite/components/honox-island.test.tsx new file mode 100644 index 0000000..1055852 --- /dev/null +++ b/src/vite/components/honox-island.test.tsx @@ -0,0 +1,18 @@ +import { HonoXIsland } from './honox-island' + +const TestComponent = () =>
Test
+ +describe('HonoXIsland', () => { + it('should set key for children', () => { + const element = HonoXIsland({ + componentName: 'Test', + componentExport: 'Test', + Component: () =>
Test
, + props: { + children: , + }, + }) + // XXX: tested by internal implementation + expect((element as any).children[1][0].key).toBe('children') + }) +}) diff --git a/src/vite/components/honox-island.tsx b/src/vite/components/honox-island.tsx index 7016c2f..cd0413c 100644 --- a/src/vite/components/honox-island.tsx +++ b/src/vite/components/honox-island.tsx @@ -57,7 +57,7 @@ export const HonoXIsland = ({ {Object.entries(elementProps).map(([key, children]) => ( -