Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/lucky-moons-remount.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@qwik.dev/core': patch
---

fix: a component without its own dom element no longer rebuilds its children on re-render and keeps its `useOn` document and window events
43 changes: 7 additions & 36 deletions packages/qwik/src/core/shared/component-execution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@ import { invokeApply, newRenderInvokeContext, type RenderInvokeContext } from '.
import { type EventQRL, type UseOnEvent, type UseOnMap } from '../use/use-on';
import { isQwikComponent, type OnRenderFn } from './component.public';
import { assertDefined } from './error/assert';
import { Fragment, type Props } from './jsx/jsx-runtime';
import { _jsxSorted } from './jsx/jsx-internal';
import { type Props } from './jsx/jsx-runtime';
import { JSXNodeImpl, isJSXNode } from './jsx/jsx-node';
import type { JSXNodeInternal, JSXOutput } from './jsx/types/jsx-node';
import type { KnownEventNames } from './jsx/types/jsx-qwik-events';
Expand All @@ -23,9 +22,10 @@ import {
OnRenderProp,
USE_ON_LOCAL,
USE_ON_LOCAL_SEQ_IDX,
USE_ON_PLACEHOLDER_KEY,
} from './utils/markers';
import { MAX_RETRY_ON_PROMISE_COUNT, isPromise, maybeThen, safeCall } from './utils/promises';
import { isArray, isPrimitiveOrNullUndefined, type ValueOrPromise } from './utils/types';
import { isArray, type ValueOrPromise } from './utils/types';
import { getSubscriber } from '../reactive-primitives/subscriber';
import { EffectProperty } from '../reactive-primitives/types';
import { EventNameHtmlScope, getEventDataFromHtmlAttribute } from './utils/event-names';
Expand Down Expand Up @@ -340,44 +340,15 @@ function findFirstElementNode(jsx: JSXOutput): ValueOrPromise<JSXNodeInternal<st
* This is necessary for headless components (components that don't render a real DOM element) to
* have an anchor point for `useOn` event listeners that target the document or window.
*
* @param jsx The JSX output to modify.
* @param jsx The JSX output of the component.
* @param placeholder The placeholder element to inject.
* @returns The modified JSX output.
* @returns The JSX output with the placeholder as its last sibling.
*/
function injectPlaceholderElement(jsx: JSXOutput, placeholder: JSXNodeInternal<string>): JSXOutput {
// For regular JSX nodes, we can append the placeholder to its children.
if (isJSXNode(jsx)) {
// Inline components may ignore children, while component$ children become projections.
if (jsx.type !== Fragment) {
return _jsxSorted(Fragment, null, null, [jsx, placeholder], 0, null);
}

if (jsx.children == null) {
jsx.children = placeholder;
} else if (isArray(jsx.children)) {
jsx.children.push(placeholder);
} else {
jsx.children = [jsx.children, placeholder];
}
return jsx;
}

// For primitives, we can't add children, so we wrap them in a fragment.
if (isPrimitiveOrNullUndefined(jsx)) {
return _jsxSorted(Fragment, null, null, [jsx, placeholder], 0, null);
}

// For an array of nodes, we inject the placeholder into the first element.
if (isArray(jsx) && jsx.length > 0) {
injectPlaceholderElement(jsx[0], placeholder);
return jsx;
}

// For anything else we do nothing.
return jsx;
return isArray(jsx) ? [...jsx, placeholder] : [jsx, placeholder];
}

/** @returns An empty <script> element for adding qwik metadata attributes to */
function createPlaceholderScriptNode(): JSXNodeInternal<string> {
return new JSXNodeImpl('script', null, { hidden: '' }, null, 0, null);
return new JSXNodeImpl('script', null, { hidden: '' }, null, 0, USE_ON_PLACEHOLDER_KEY);
}
1 change: 1 addition & 0 deletions packages/qwik/src/core/shared/utils/markers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ export const QDefaultSlot = '';
export const ELEMENT_ID = 'q:id';
export const ELEMENT_KEY = 'q:key';
export const ELEMENT_PROPS = 'q:props';
export const USE_ON_PLACEHOLDER_KEY = ':useOn';
/** @internal */
export const ELEMENT_SEQ = 'q:seq';
export const ELEMENT_SEQ_IDX = 'q:seqIdx';
Expand Down
56 changes: 26 additions & 30 deletions packages/qwik/src/core/tests/use-on.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -363,17 +363,19 @@ describe.each([
expect(vNode).toMatchVDOM(
<Component ssr-required>
<Fragment ssr-required>
Count: <Signal ssr-required>{'123'}</Signal>!<script hidden></script>
Count: <Signal ssr-required>{'123'}</Signal>!
</Fragment>
<script hidden></script>
</Component>
);

await trigger(container.element, 'script', 'd:click');
expect(vNode).toMatchVDOM(
<Component ssr-required>
<Fragment ssr-required>
Count: <Signal ssr-required>{'124'}</Signal>!<script hidden></script>
Count: <Signal ssr-required>{'124'}</Signal>!
</Fragment>
<script hidden></script>
</Component>
);
});
Expand Down Expand Up @@ -412,28 +414,24 @@ describe.each([
const { vNode, container } = await render(<Counter initial={123} />, { debug });
expect(vNode).toMatchVDOM(
<Component ssr-required>
<Fragment ssr-required>
<InlineComponent ssr-required>
<Fragment ssr-required>
Count: <Signal ssr-required>{'123'}</Signal>!
</Fragment>
</InlineComponent>
<script hidden />
</Fragment>
<InlineComponent ssr-required>
<Fragment ssr-required>
Count: <Signal ssr-required>{'123'}</Signal>!
</Fragment>
</InlineComponent>
<script hidden />
</Component>
);

await trigger(container.element, 'script', 'd:click');
expect(vNode).toMatchVDOM(
<Component ssr-required>
<Fragment ssr-required>
<InlineComponent ssr-required>
<Fragment ssr-required>
Count: <Signal ssr-required>{'124'}</Signal>!
</Fragment>
</InlineComponent>
<script hidden />
</Fragment>
<InlineComponent ssr-required>
<Fragment ssr-required>
Count: <Signal ssr-required>{'124'}</Signal>!
</Fragment>
</InlineComponent>
<script hidden />
</Component>
);
});
Expand Down Expand Up @@ -625,17 +623,19 @@ describe.each([
expect(vNode).toMatchVDOM(
<Component ssr-required>
<Fragment ssr-required>
Count: <Signal ssr-required>{'123'}</Signal>!<script hidden></script>
Count: <Signal ssr-required>{'123'}</Signal>!
</Fragment>
<script hidden></script>
</Component>
);

await trigger(container.element, 'script', 'w:click');
expect(vNode).toMatchVDOM(
<Component ssr-required>
<Fragment ssr-required>
Count: <Signal ssr-required>{'124'}</Signal>!<script hidden></script>
Count: <Signal ssr-required>{'124'}</Signal>!
</Fragment>
<script hidden></script>
</Component>
);
});
Expand Down Expand Up @@ -1059,11 +1059,9 @@ describe.each([
<Component ssr-required>
<Component ssr-required>
<Component ssr-required>
<Component ssr-required>
<div>test</div>
</Component>
<script hidden></script>
<div>test</div>
</Component>
<script hidden></script>
</Component>
</Component>
);
Expand All @@ -1089,12 +1087,10 @@ describe.each([
expect(vNode).toMatchVDOM(
<Component ssr-required>
<Component ssr-required>
<Component ssr-required>
<Projection ssr-required>
<div>test</div>
</Projection>
<script hidden></script>
</Component>
<Projection ssr-required>
<div>test</div>
</Projection>
<script hidden></script>
</Component>
</Component>
);
Expand Down
Loading
Loading