Skip to content

Commit

Permalink
feat(v2): include children in props for backward compatibility (#6848)
Browse files Browse the repository at this point in the history
* include children in props for backward compatibility
  • Loading branch information
Varixo committed Aug 30, 2024
1 parent 404d34e commit b6ac7d3
Show file tree
Hide file tree
Showing 12 changed files with 160 additions and 38 deletions.
2 changes: 1 addition & 1 deletion packages/docs/src/routes/api/qwik/api.json
Original file line number Diff line number Diff line change
Expand Up @@ -1214,7 +1214,7 @@
}
],
"kind": "Interface",
"content": "A JSX Node, an internal structure. You probably want to use `JSXOutput` instead.\n\n\n```typescript\nexport interface JSXNode<T extends string | FunctionComponent | unknown = unknown> \n```\n\n\n<table><thead><tr><th>\n\nProperty\n\n\n</th><th>\n\nModifiers\n\n\n</th><th>\n\nType\n\n\n</th><th>\n\nDescription\n\n\n</th></tr></thead>\n<tbody><tr><td>\n\n[children](#)\n\n\n</td><td>\n\n\n</td><td>\n\n[JSXChildren](#jsxchildren) \\| null\n\n\n</td><td>\n\n\n</td></tr>\n<tr><td>\n\n[constProps](#)\n\n\n</td><td>\n\n\n</td><td>\n\nRecord&lt;any, unknown&gt; \\| null\n\n\n</td><td>\n\n\n</td></tr>\n<tr><td>\n\n[dev?](#)\n\n\n</td><td>\n\n\n</td><td>\n\n[DevJSX](#devjsx)\n\n\n</td><td>\n\n_(Optional)_\n\n\n</td></tr>\n<tr><td>\n\n[flags](#)\n\n\n</td><td>\n\n\n</td><td>\n\nnumber\n\n\n</td><td>\n\n\n</td></tr>\n<tr><td>\n\n[key](#)\n\n\n</td><td>\n\n\n</td><td>\n\nstring \\| null\n\n\n</td><td>\n\n\n</td></tr>\n<tr><td>\n\n[props](#)\n\n\n</td><td>\n\n\n</td><td>\n\nT extends [FunctionComponent](#functioncomponent)<!-- -->&lt;infer P&gt; ? P : Record&lt;any, unknown&gt;\n\n\n</td><td>\n\n\n</td></tr>\n<tr><td>\n\n[propsC](#)\n\n\n</td><td>\n\n\n</td><td>\n\nT extends [FunctionComponent](#functioncomponent)<!-- -->&lt;infer P&gt; ? P : Record&lt;any, unknown&gt;\n\n\n</td><td>\n\n\n</td></tr>\n<tr><td>\n\n[type](#)\n\n\n</td><td>\n\n\n</td><td>\n\nT\n\n\n</td><td>\n\n\n</td></tr>\n<tr><td>\n\n[varProps](#)\n\n\n</td><td>\n\n\n</td><td>\n\nRecord&lt;any, unknown&gt;\n\n\n</td><td>\n\n\n</td></tr>\n</tbody></table>",
"content": "A JSX Node, an internal structure. You probably want to use `JSXOutput` instead.\n\n\n```typescript\nexport interface JSXNode<T extends string | FunctionComponent | unknown = unknown> \n```\n\n\n<table><thead><tr><th>\n\nProperty\n\n\n</th><th>\n\nModifiers\n\n\n</th><th>\n\nType\n\n\n</th><th>\n\nDescription\n\n\n</th></tr></thead>\n<tbody><tr><td>\n\n[children](#)\n\n\n</td><td>\n\n\n</td><td>\n\n[JSXChildren](#jsxchildren) \\| null\n\n\n</td><td>\n\n\n</td></tr>\n<tr><td>\n\n[constProps](#)\n\n\n</td><td>\n\n\n</td><td>\n\nRecord&lt;any, unknown&gt; \\| null\n\n\n</td><td>\n\n\n</td></tr>\n<tr><td>\n\n[dev?](#)\n\n\n</td><td>\n\n\n</td><td>\n\n[DevJSX](#devjsx)\n\n\n</td><td>\n\n_(Optional)_\n\n\n</td></tr>\n<tr><td>\n\n[flags](#)\n\n\n</td><td>\n\n\n</td><td>\n\nnumber\n\n\n</td><td>\n\n\n</td></tr>\n<tr><td>\n\n[key](#)\n\n\n</td><td>\n\n\n</td><td>\n\nstring \\| null\n\n\n</td><td>\n\n\n</td></tr>\n<tr><td>\n\n[props](#)\n\n\n</td><td>\n\n\n</td><td>\n\nT extends [FunctionComponent](#functioncomponent)<!-- -->&lt;infer P&gt; ? P : Record&lt;any, unknown&gt;\n\n\n</td><td>\n\n\n</td></tr>\n<tr><td>\n\n[type](#)\n\n\n</td><td>\n\n\n</td><td>\n\nT\n\n\n</td><td>\n\n\n</td></tr>\n<tr><td>\n\n[varProps](#)\n\n\n</td><td>\n\n\n</td><td>\n\nRecord&lt;any, unknown&gt;\n\n\n</td><td>\n\n\n</td></tr>\n</tbody></table>",
"editUrl": "https://github.com/QwikDev/qwik/tree/main/packages/qwik/src/core/render/jsx/types/jsx-node.ts",
"mdFile": "qwik.jsxnode.md"
},
Expand Down
13 changes: 0 additions & 13 deletions packages/docs/src/routes/api/qwik/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -2972,19 +2972,6 @@ T extends [FunctionComponent](#functioncomponent)&lt;infer P&gt; ? P : Record&lt
</td></tr>
<tr><td>
[propsC](#)
</td><td>
</td><td>
T extends [FunctionComponent](#functioncomponent)&lt;infer P&gt; ? P : Record&lt;any, unknown&gt;
</td><td>
</td></tr>
<tr><td>
[type](#)
</td><td>
Expand Down
8 changes: 5 additions & 3 deletions packages/qwik/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@
"vite": "^5"
},
"peerDependencies": {
"vitest": "^2",
"prettier": "*"
"prettier": "*",
"vitest": "^2"
},
"peerDependenciesMeta": {
"vitest": {
Expand All @@ -44,7 +44,9 @@
"@builder.io/qwik": "workspace:^",
"@builder.io/qwik-dom": "workspace:^",
"image-size": "1.1.1",
"kleur": "4.1.5"
"kleur": "4.1.5",
"vitest": "2.0.5",
"prettier": "3.3.3"
},
"engines": {
"node": "^18.17.0 || ^20.3.0 || >=21.0.0"
Expand Down
2 changes: 0 additions & 2 deletions packages/qwik/src/core/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -563,8 +563,6 @@ export interface JSXNode<T extends string | FunctionComponent | unknown = unknow
// (undocumented)
props: T extends FunctionComponent<infer P> ? P : Record<any, unknown>;
// (undocumented)
propsC: T extends FunctionComponent<infer P> ? P : Record<any, unknown>;
// (undocumented)
type: T;
// (undocumented)
varProps: Record<any, unknown>;
Expand Down
52 changes: 40 additions & 12 deletions packages/qwik/src/core/render/jsx/jsx-runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,21 @@ export const jsx = <T extends string | FunctionComponent<any>>(
return _jsxSplit(type, props, null, null, 0, key || null);
};

export const flattenArray = <T>(array: (T | T[])[], dst?: T[]): T[] => {
// Yes this function is just Array.flat, but we need to run on old versions of Node.
if (!dst) {
dst = [];
}
for (const item of array) {
if (isArray(item)) {
flattenArray(item, dst);
} else {
dst.push(item);
}
}
return dst;
};

/**
* The legacy transform, used in special cases like `<div {...props} key="key" />`. Note that the
* children are spread arguments, instead of a prop like in jsx() calls.
Expand All @@ -168,7 +183,24 @@ export function h<TYPE extends string | FunctionComponent<PROPS>, PROPS extends
props?: PROPS | null,
...children: any[]
): JSXNode<TYPE> {
return _jsxSplit(type, props!, null, children, 0, null);
const normalizedProps: any = {
children: arguments.length > 2 ? flattenArray(children) : null,
};

let key: any = null;

for (const i in props) {
if (i == 'key') {
key = (props as Record<string, any>)[i];
} else {
normalizedProps[i] = (props as Record<string, any>)[i];
}
}

if (typeof type === 'string' && !key && 'dangerouslySetInnerHTML' in normalizedProps) {
key = 'innerhtml';
}
return _jsxSplit(type, props!, null, normalizedProps.children, 0, key);
}

export const SKIP_RENDER_TYPE = ':skipRender';
Expand Down Expand Up @@ -201,13 +233,6 @@ export class JSXNodeImpl<T> implements JSXNode<T> {

private _proxy: Props | null = null;
get props(): T extends FunctionComponent<infer PROPS> ? PROPS : Props {
// We use a proxy to merge the constProps if they exist and to evaluate derived signals
if (!this._proxy) {
this._proxy = createPropsProxy(this.varProps, this.constProps, undefined);
}
return this._proxy as typeof this.props;
}
get propsC(): T extends FunctionComponent<infer PROPS> ? PROPS : Props {
// We use a proxy to merge the constProps if they exist and to evaluate derived signals
if (!this._proxy) {
this._proxy = createPropsProxy(this.varProps, this.constProps, this.children);
Expand Down Expand Up @@ -481,7 +506,7 @@ class PropsProxyHandler implements ProxyHandler<any> {
if (prop === _VAR_PROPS) {
return this.$varProps$;
}
if (this.$children$ !== undefined && prop === 'children') {
if (this.$children$ != null && prop === 'children') {
return this.$children$;
}
const value =
Expand Down Expand Up @@ -515,11 +540,14 @@ class PropsProxyHandler implements ProxyHandler<any> {
if (this.$constProps$) {
didDelete = delete this.$constProps$[prop as string] || didDelete;
}
if (this.$children$ != null && prop === 'children') {
this.$children$ = null;
}
return didDelete;
}
has(_: any, prop: string | symbol) {
const hasProp =
(prop === 'children' && this.$children$ !== undefined) ||
(prop === 'children' && this.$children$ != null) ||
prop === _CONST_PROPS ||
prop === _VAR_PROPS ||
prop in this.$varProps$ ||
Expand All @@ -528,7 +556,7 @@ class PropsProxyHandler implements ProxyHandler<any> {
}
getOwnPropertyDescriptor(target: any, p: string | symbol): PropertyDescriptor | undefined {
const value =
p === 'children' && this.$children$ !== undefined
p === 'children' && this.$children$ != null
? this.$children$
: this.$constProps$ && p in this.$constProps$
? this.$constProps$[p as string]
Expand All @@ -541,7 +569,7 @@ class PropsProxyHandler implements ProxyHandler<any> {
}
ownKeys() {
const out = Object.keys(this.$varProps$);
if (this.$children$ !== undefined) {
if (this.$children$ != null && out.indexOf('children') === -1) {
out.push('children');
}
if (this.$constProps$) {
Expand Down
1 change: 0 additions & 1 deletion packages/qwik/src/core/render/jsx/types/jsx-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ export interface DevJSX {
export interface JSXNode<T extends string | FunctionComponent | unknown = unknown> {
type: T;
props: T extends FunctionComponent<infer P> ? P : Record<any, unknown>;
propsC: T extends FunctionComponent<infer P> ? P : Record<any, unknown>;
varProps: Record<any, unknown>;
constProps: Record<any, unknown> | null;
children: JSXChildren | null;
Expand Down
16 changes: 12 additions & 4 deletions packages/qwik/src/core/v2/client/vnode-diff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1023,7 +1023,7 @@ export const vnode_diff = (
vCurrent && getInsertBefore()
);
isDev && vnode_setProp(vNewNode, DEBUG_TYPE, VirtualType.InlineComponent);
vnode_setProp(vNewNode, ELEMENT_PROPS, jsxValue.propsC);
vnode_setProp(vNewNode, ELEMENT_PROPS, jsxValue.props);

host = vNewNode;
let component$Host: VNode | null = host;
Expand All @@ -1041,7 +1041,7 @@ export const vnode_diff = (
host,
(component$Host || container.rootVNode) as HostElement,
component as OnRenderFn<unknown>,
jsxValue.propsC
jsxValue.props
);
asyncQueue.push(jsxOutput, host);
}
Expand Down Expand Up @@ -1162,8 +1162,8 @@ function propsDiffer(src: Record<string, any>, dst: Record<string, any>): boolea
if (!src || !dst) {
return true;
}
let srcKeys = Object.keys(src);
let dstKeys = Object.keys(dst);
let srcKeys = removeChildrenKey(Object.keys(src));
let dstKeys = removeChildrenKey(Object.keys(dst));
if (srcKeys.length !== dstKeys.length) {
return true;
}
Expand All @@ -1179,6 +1179,14 @@ function propsDiffer(src: Record<string, any>, dst: Record<string, any>): boolea
return false;
}

function removeChildrenKey(keys: string[]): string[] {
const childrenIdx = keys.indexOf('children');
if (childrenIdx !== -1) {
keys.splice(childrenIdx, 1);
}
return keys;
}

/**
* If vnode is removed, it is necessary to release all subscriptions associated with it.
*
Expand Down
3 changes: 3 additions & 0 deletions packages/qwik/src/core/v2/shared/component-execution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ export const executeComponent2 = (
}
if (isQrl(componentQRL)) {
props = props || container.getHostProp(renderHost, ELEMENT_PROPS) || EMPTY_OBJ;
if (props && props.children) {
delete props.children;
}
componentFn = componentQRL.getFn(iCtx);
} else if (isQwikComponent(componentQRL)) {
const qComponentFn = componentQRL as (
Expand Down
5 changes: 4 additions & 1 deletion packages/qwik/src/core/v2/ssr/ssr-render-component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export const applyInlineComponent = (
jsx: JSXNode
) => {
const host = ssr.getLastNode();
return executeComponent2(ssr, host, component$Host, component, jsx.propsC);
return executeComponent2(ssr, host, component$Host, component, jsx.props);
};

export const applyQwikComponentBody = (
Expand All @@ -27,6 +27,9 @@ export const applyQwikComponentBody = (
const host = ssr.getLastNode();
const [componentQrl] = (component as any)[SERIALIZABLE_STATE] as [QRLInternal<OnRenderFn<any>>];
const srcProps = jsx.props;
if (srcProps && srcProps.children) {
delete srcProps.children;
}
const scheduler = ssr.$scheduler$;
host.setProp(OnRenderProp, componentQrl);
host.setProp(ELEMENT_PROPS, srcProps);
Expand Down
2 changes: 1 addition & 1 deletion packages/qwik/src/core/v2/ssr/ssr-render-jsx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ function processJSXNode(
projectionAttrs.push(QSlot, slotName);
enqueue(new SetScopedStyle(styleScoped));
enqueue(ssr.closeProjection);
const slotDefaultChildren = (jsx.children || null) as JSXChildren | null;
const slotDefaultChildren: JSXChildren | null = jsx.children || null;
const slotChildren =
componentFrame.consumeChildrenForSlot(node, slotName) || slotDefaultChildren;
if (slotDefaultChildren && slotChildren !== slotDefaultChildren) {
Expand Down
87 changes: 87 additions & 0 deletions packages/qwik/src/core/v2/tests/component.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {
$,
Fragment as Component,
Fragment,
Fragment as InlineComponent,
SSRComment,
Fragment as Signal,
SkipRender,
Expand All @@ -24,6 +25,10 @@ import { delay } from '../../util/promises';
const debug = false; //true;
Error.stackTraceLimit = 100;

function Hola(props: any) {
return <div {...props}></div>;
}

describe.each([
{ render: ssrRenderToDom }, //
{ render: domRender }, //
Expand Down Expand Up @@ -228,6 +233,36 @@ describe.each([
expect(log).toEqual(['no children']);
});

it('should remove children from child component$', async () => {
const log: string[] = [];
const ChildMyComp = component$((props: any) => {
log.push('children' in props ? 'children' : 'no children');
return <span>Hello world</span>;
});
const MyComp = component$((props: any) => {
log.push('children' in props ? 'children' : 'no children');
return (
<span>
<ChildMyComp />
Hello world
</span>
);
});

const { vNode } = await render(<MyComp>CHILDREN</MyComp>, { debug });
expect(vNode).toMatchVDOM(
<Component>
<span>
<Component>
<span>Hello world</span>
</Component>
Hello world
</span>
</Component>
);
expect(log).toEqual(['no children', 'no children']);
});

it('should NOT remove children from inline component', async () => {
const log: string[] = [];
const MyComp = (props: any) => {
Expand All @@ -244,6 +279,58 @@ describe.each([
expect(log).toEqual(['has children']);
});

it('should render children from dynamic props', async () => {
const IssueChildrenSpread = component$(() => {
const signal = useSignal({
type: 'div',
children: ['Hello'],
});
const Type = signal.value.type;
return (
<div>
<button
onClick$={() => {
signal.value = {
type: 'div',
children: ['Changed'],
};
}}
>
Change
</button>
<Hola>
<div>1</div>
<div>2</div>
</Hola>
<div>
<Type {...(signal.value as any)}></Type>
</div>
</div>
);
});

const { vNode } = await render(<IssueChildrenSpread />, { debug });

const props = { type: 'div' };

expect(vNode).toMatchVDOM(
<Component>
<div>
<button>Change</button>
<InlineComponent>
<div>
<div>1</div>
<div>2</div>
</div>
</InlineComponent>
<div>
<div {...props}>Hello</div>
</div>
</div>
</Component>
);
});

it('should insert dangerouslySetInnerHTML', async () => {
const Cmp = component$(() => {
const htmlSignal = useSignal("<h2><span>I'm a signal value!</span></h2>");
Expand Down
7 changes: 7 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit b6ac7d3

Please sign in to comment.