fix(runtime): throw error when functions are passed as JSX children#8496
fix(runtime): throw error when functions are passed as JSX children#8496restareaByWeezy wants to merge 1 commit intoQwikDev:mainfrom
Conversation
Closes QwikDev#8425 Add explicit function check in processData() for both SSR and DOM rendering paths. Previously, function children silently rendered nothing in production builds. Now they throw with a helpful error message.
🦋 Changeset detectedLatest commit: a7a3679 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
built with Refined Cloudflare Pages Action⚡ Cloudflare Pages Deployment
|
| return new ProcessedJSXNodeImpl(SKIP_RENDER_TYPE, EMPTY_OBJ, null, EMPTY_ARRAY, 0, null); | ||
| } else if (isFunction(node)) { | ||
| throw new Error( | ||
| `Functions are not valid JSX children. ` + |
There was a problem hiding this comment.
This isn't quite true: functions are valid JSX children, but only when the parent is an Inline Component that will use children() somehow.
so the message should be something like JSX Function values cannot be rendered.
Furthermore, make sure that the error output is sufficient to find where it is defined. Maybe throwing is enough, but maybe you need to find the file from the dev data, or console.error the node or something like that.
| await render(fixture.host, <div>{() => <span>hello</span>}</div>); | ||
| assert.fail('Expected an error to be thrown'); | ||
| } catch (e: any) { | ||
| assert.match(e.message, /is not an accepted value|Functions are not valid JSX children/); |
There was a problem hiding this comment.
here, ensure that it's easy to find where the wrong definition was.
| return node.then((node) => processData(node, rCtx, ssrCtx, stream, flags, beforeClose)); | ||
| } else if (isFunction(node)) { | ||
| throw new Error( | ||
| `Functions are not valid JSX children. ` + |
|
Thanks for the PR, but I think we should try to render the example from the issue as inline component. |
|
@Varixo thanks for the feedback. Just to clarify the direction -- when you say "render as inline component", do you mean the runtime should automatically invoke |
if you go to the playground and open segments for this example, you can see this: export const app_component_QbmWc3TlyqA = ()=>{
return /*#__PURE__*/ _jsxSorted("div", null, null, [
/*#__PURE__*/ _jsxSorted("h1", null, null, "Hello from Qwik!", 3, null, {
fileName: "../app.tsx",
lineNumber: 6,
columnNumber: 7
}),
()=>/*#__PURE__*/ _jsxSorted("div", null, null, "hiiii", 3, "ga_0", {
fileName: "../app.tsx",
lineNumber: 7,
columnNumber: 14
})
], 3, "ga_1", {
fileName: "../app.tsx",
lineNumber: 5,
columnNumber: 5
});This means the optimizer is wrong, because it should be wrapped like this: export const app_component_QbmWc3TlyqA = ()=>{
return /*#__PURE__*/ _jsxSorted("div", null, null, [
/*#__PURE__*/ _jsxSorted("h1", null, null, "Hello from Qwik!", 3, null, {
fileName: "../app.tsx",
lineNumber: 6,
columnNumber: 7
}),
/*#__PURE__*/ _jsxSorted(()=>/*#__PURE__*/ _jsxSorted("div", null, null, "hiiii", 3, "ga_0", {
fileName: "../app.tsx",
lineNumber: 7,
columnNumber: 14
}), null, null, null, 3, "ga_1", {
fileName: "../app.tsx",
lineNumber: 9,
columnNumber: 7
})
], 3, "ga_1", {
fileName: "../app.tsx",
lineNumber: 5,
columnNumber: 5
});(this is just an example so lineNumber, and columnNumber will be different) And if the optimizer will do this, then everything should be fine without js changes but if we want to fix it in v2 then this PR should be closed and you need to do changes againt build/v2 branch. |
What is it?
Description
Closes #8425
Right now if you do something like
{() => <div>hi</div>}inside a component, it just... renders nothing. No error, no warning in the console (unless you're in dev mode). Pretty confusing when you're staring at your code wondering where your content went.In v1 this actually threw an SSR error, which was way more helpful. The v2
processData()in both SSR and DOM paths has a catch-allelsethat logs a warning and moves on, so functions just get silently swallowed.The fix adds a
typeof node === 'function'check before that catch-all in bothrender-ssr.tsandrender-dom.ts. Now you get a clear error telling you to either call the function ({fn()}) or wrap it withcomponent$().Dev mode already catches this at JSX creation time via
validateJSXNode, but that's behind aqDevguard and gets stripped in prod. This ensures the error shows up regardless.One thing worth discussing
This technically turns a silent no-op into a runtime error. If someone has
{myFn}in production code today, it renders nothing but doesn't crash. After this change, it would crash.In practice the impact should be minimal -- dev mode and ESLint both already flag this pattern, so it's unlikely to survive into prod unnoticed. But if you'd prefer a softer approach (like
logErrorinstead ofthrow), happy to adjust.Checklist
pnpm change