Skip to content

Commit 8f9efc5

Browse files
authored
fix(runtime): various jsx-runtime behaviours (#6538)
* fix(runtime): stop jsx-runtime adding VDOM `key` attributes to DOM * fix(runtime): jsx-runtime behaviours * chore: revert stupidity * chore: fix test * chore: revert
1 parent 0d3a068 commit 8f9efc5

File tree

8 files changed

+144
-36
lines changed

8 files changed

+144
-36
lines changed

package-lock.json

Lines changed: 5 additions & 23 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/compiler/transformers/static-to-meta/call-expression.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,10 @@ const visitCallExpressionArgs = (
4444
// These have the same signature as h() for metadata purposes
4545
visitCallExpressionArg(m, args[0]);
4646
gatherVdomMeta(m, args);
47+
// TypeScript's jsx transform passes key as the 3rd argument
48+
if (args.length > 2 && args[2]) {
49+
m.hasVdomKey = true;
50+
}
4751
} else if (args.length > 1 && fnName === 'createElementNS') {
4852
visitCallExpressionArg(m, args[1]);
4953
} else if (fnName === 'require' && args.length > 0 && (m as d.Module).originalImports) {

src/runtime/vdom/h.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,8 @@ export const newVNode = (tag: string, text: string) => {
116116
const vnode: d.VNode = {
117117
$flags$: 0,
118118
$tag$: tag,
119-
$text$: text,
119+
// Normalize undefined to null to prevent rendering "undefined" as text
120+
$text$: text ?? null,
120121
$elm$: null,
121122
$children$: null,
122123
};

src/runtime/vdom/jsx-dev-runtime.ts

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,18 +33,34 @@ export function jsxDEV(
3333
_source?: any,
3434
_self?: any,
3535
) {
36-
const { children, ...rest } = props;
36+
const propsObj = props || {};
37+
const { children, ...rest } = propsObj;
3738

38-
// In development mode, key might be passed separately
39-
const vnodeData = key !== undefined ? { ...rest, key } : rest;
39+
// Build vnodeData - key from props takes precedence over parameter
40+
let vnodeData = rest;
41+
if (key !== undefined && !('key' in rest)) {
42+
vnodeData = { ...rest, key };
43+
}
44+
45+
// If vnodeData is empty object, use null instead (matches old transform)
46+
if (vnodeData && Object.keys(vnodeData).length === 0) {
47+
vnodeData = null;
48+
}
4049

4150
// Note: source and self are debug info we could potentially use in the future
4251
// for better developer experience, but for now we ignore them
4352

44-
// If children is an array, spread it; otherwise pass as single child
45-
if (Array.isArray(children)) {
46-
return h(type, vnodeData, ...children);
47-
} else if (children !== undefined) {
53+
if (children !== undefined) {
54+
// If children is already an array, spread it
55+
if (Array.isArray(children)) {
56+
return h(type, vnodeData, ...children);
57+
}
58+
// If single child is a VNode (has $flags$), pass it directly
59+
// Otherwise it gets stringified
60+
if (typeof children === 'object' && children !== null && '$flags$' in children) {
61+
return h(type, vnodeData, children);
62+
}
63+
// For primitive values (strings, numbers), pass directly
4864
return h(type, vnodeData, children);
4965
}
5066

src/runtime/vdom/jsx-runtime.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,18 +25,19 @@ export { Fragment } from '../fragment';
2525
export function jsx(type: any, props: any, key?: string) {
2626
const propsObj = props || {};
2727
const { children, ...rest } = propsObj;
28-
// Build vnodeData - pass null if no props/key (matches old h() behavior)
28+
// Build vnodeData - key from props takes precedence over parameter
2929
let vnodeData = rest;
30-
if (key !== undefined) {
30+
if (key !== undefined && !('key' in rest)) {
3131
vnodeData = { ...rest, key };
3232
}
33+
3334
// If vnodeData is empty object, use null instead (matches old transform)
3435
if (vnodeData && Object.keys(vnodeData).length === 0) {
3536
vnodeData = null;
3637
}
3738

3839
if (children !== undefined) {
39-
// If children is array, spread it
40+
// If children is already an array, spread it
4041
if (Array.isArray(children)) {
4142
return h(type, vnodeData, ...children);
4243
}
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
import { jsx, jsxs } from '../jsx-runtime';
2+
import { jsxDEV } from '../jsx-dev-runtime';
3+
4+
describe('jsx-runtime', () => {
5+
describe('jsx() and jsxs()', () => {
6+
it('should create a vnode with no props', () => {
7+
const vnode = jsx('div', {});
8+
expect(vnode.$tag$).toBe('div');
9+
expect(vnode.$attrs$).toBe(null);
10+
});
11+
12+
it('should create a vnode with basic props', () => {
13+
const vnode = jsx('div', { id: 'test', class: 'foo' });
14+
expect(vnode.$tag$).toBe('div');
15+
expect(vnode.$attrs$).toEqual({ id: 'test', class: 'foo' });
16+
});
17+
18+
it('should handle key passed as separate parameter', () => {
19+
const vnode = jsx('div', { id: 'test' }, 'my-key');
20+
expect(vnode.$tag$).toBe('div');
21+
expect(vnode.$key$).toBe('my-key');
22+
expect(vnode.$attrs$).toEqual({ id: 'test', key: 'my-key' });
23+
});
24+
25+
it('should handle key passed in props', () => {
26+
const vnode = jsx('div', { id: 'test', key: 'props-key' });
27+
expect(vnode.$tag$).toBe('div');
28+
expect(vnode.$key$).toBe('props-key');
29+
expect(vnode.$attrs$).toEqual({ id: 'test', key: 'props-key' });
30+
});
31+
32+
it('should prefer key from props over parameter', () => {
33+
const vnode = jsx('div', { id: 'test', key: 'props-key' }, 'param-key');
34+
expect(vnode.$tag$).toBe('div');
35+
expect(vnode.$key$).toBe('props-key');
36+
expect(vnode.$attrs$).toEqual({ id: 'test', key: 'props-key' });
37+
});
38+
39+
it('should handle ref in props', () => {
40+
const refCallback = jest.fn();
41+
const vnode = jsx('div', { id: 'test', ref: refCallback });
42+
expect(vnode.$tag$).toBe('div');
43+
expect(vnode.$attrs$).toEqual({ id: 'test', ref: refCallback });
44+
});
45+
46+
it('should handle both key and ref', () => {
47+
const refCallback = jest.fn();
48+
const vnode = jsx('div', { id: 'test', key: 'my-key', ref: refCallback }, undefined);
49+
expect(vnode.$tag$).toBe('div');
50+
expect(vnode.$key$).toBe('my-key');
51+
expect(vnode.$attrs$).toEqual({ id: 'test', key: 'my-key', ref: refCallback });
52+
});
53+
54+
it('should handle children as array', () => {
55+
const vnode = jsx('div', { children: ['Hello', 'World'] });
56+
expect(vnode.$tag$).toBe('div');
57+
expect(vnode.$children$?.length).toBe(1);
58+
expect(vnode.$children$![0].$text$).toBe('HelloWorld');
59+
});
60+
61+
it('should handle single child', () => {
62+
const vnode = jsx('div', { children: 'Hello' });
63+
expect(vnode.$tag$).toBe('div');
64+
expect(vnode.$children$?.length).toBe(1);
65+
});
66+
67+
it('should not include children in attrs', () => {
68+
const vnode = jsx('div', { id: 'test', children: 'Hello' });
69+
expect(vnode.$attrs$).toEqual({ id: 'test' });
70+
});
71+
72+
it('jsxs should work the same as jsx', () => {
73+
const vnode = jsxs('div', { id: 'test' }, 'my-key');
74+
expect(vnode.$tag$).toBe('div');
75+
expect(vnode.$attrs$).toEqual({ id: 'test', key: 'my-key' });
76+
});
77+
});
78+
79+
describe('jsxDEV()', () => {
80+
it('should handle key and ref like jsx()', () => {
81+
const refCallback = jest.fn();
82+
const vnode = jsxDEV('div', { id: 'test', key: 'my-key', ref: refCallback });
83+
expect(vnode.$tag$).toBe('div');
84+
expect(vnode.$key$).toBe('my-key');
85+
expect(vnode.$attrs$).toEqual({ id: 'test', key: 'my-key', ref: refCallback });
86+
});
87+
88+
it('should handle key as separate parameter', () => {
89+
const vnode = jsxDEV('div', { id: 'test' }, 'param-key');
90+
expect(vnode.$tag$).toBe('div');
91+
expect(vnode.$key$).toBe('param-key');
92+
expect(vnode.$attrs$).toEqual({ id: 'test', key: 'param-key' });
93+
});
94+
});
95+
});

src/runtime/vdom/test/patch.spec.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -861,4 +861,9 @@ describe('renderer', () => {
861861
}
862862
return ret;
863863
}
864+
865+
it('should normalize undefined $text$ to null in newVNode', () => {
866+
const vnode = newVNode(null, undefined as any);
867+
expect(vnode.$text$).toBe(null);
868+
});
864869
});

src/runtime/vdom/vdom-render.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,8 @@ const createElm = (oldParentVNode: d.VNode, newParentVNode: d.VNode, childIndex:
7272
);
7373
}
7474

75-
if (BUILD.vdomText && newVNode.$text$ !== null) {
75+
// Use loose equality to handle both null and undefined
76+
if (BUILD.vdomText && newVNode.$text$ != null) {
7677
// create text node
7778
elm = newVNode.$elm$ = win.document.createTextNode(newVNode.$text$) as any;
7879
} else if (BUILD.slotRelocation && newVNode.$flags$ & VNODE_FLAGS.isSlotReference) {
@@ -86,6 +87,7 @@ const createElm = (oldParentVNode: d.VNode, newParentVNode: d.VNode, childIndex:
8687
updateElement(null, newVNode, isSvgMode);
8788
}
8889
} else {
90+
// Only create element if we have a valid tag name
8991
if (BUILD.svg && !isSvgMode) {
9092
isSvgMode = newVNode.$tag$ === 'svg';
9193
}
@@ -698,7 +700,8 @@ export const patch = (oldVNode: d.VNode, newVNode: d.VNode, isInitialRender = fa
698700
const text = newVNode.$text$;
699701
let defaultHolder: Comment;
700702

701-
if (!BUILD.vdomText || text === null) {
703+
// Use loose equality to handle both null and undefined
704+
if (!BUILD.vdomText || text == null) {
702705
if (BUILD.svg) {
703706
// test if we're rendering an svg element, or still rendering nodes inside of one
704707
// only add this to the when the compiler sees we're using an svg somewhere
@@ -893,6 +896,7 @@ export const nullifyVNodeRefs = (vNode: d.VNode) => {
893896
* @param parent parent node
894897
* @param newNode element to be inserted
895898
* @param reference anchor element
899+
* @param isInitialLoad whether or not this is the first render
896900
* @returns inserted node
897901
*/
898902
export const insertBefore = (

0 commit comments

Comments
 (0)