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-spiders-wonder.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@qwik.dev/core': patch
---

fix: duplicate projected element children when a component throws a promise on first render
14 changes: 8 additions & 6 deletions packages/qwik/src/core/client/vnode-diff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -409,9 +409,7 @@ function diff(
diffContext,
diffContext.$jsxValue$.children,
true,
// special case for projection, we don't want to expect no children
// because the projection's children are not removed
false
/* isProjection */ true
);
} else if (type === SSRComment) {
expectNoMore(diffContext);
Expand Down Expand Up @@ -538,15 +536,17 @@ function advance(diffContext: DiffContext) {
*
* In the above example all nodes are on same level so we don't `descendVNode` even thought there is
* an array produced by the `map` function.
* @param isProjection - True when descending into a Projection, whose children survive re-renders:
* they are diffed against (never re-created) and kept when the new JSX has no children.
*/
function descend(
diffContext: DiffContext,
children: JSXChildren,
descendVNode: boolean,
shouldExpectNoChildren: boolean = true
isProjection: boolean = false
) {
if (
shouldExpectNoChildren &&
!isProjection &&
(children == null || (descendVNode && isArray(children) && children.length === 0))
) {
expectNoChildren(diffContext);
Expand All @@ -560,7 +560,9 @@ function descend(
'Expecting vCurrent to be defined.'
);
const creationMode =
diffContext.$isCreationMode$ ||
// a projection keeps its children across re-renders, so it never inherits
// creation mode from a not-yet-rendered host; create only if new or empty
(!isProjection && diffContext.$isCreationMode$) ||
!!diffContext.$vNewNode$ ||
!vnode_getFirstChild(diffContext.$vCurrent$!);

Expand Down
71 changes: 70 additions & 1 deletion packages/qwik/src/core/tests/render-promise.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Fragment as Component, Fragment, component$, useSignal } from '@qwik.dev/core';
import { Fragment as Component, Fragment, Slot, component$, useSignal } from '@qwik.dev/core';
import { domRender, ssrRenderToDom } from '@qwik.dev/core/testing';
import { describe, expect, it } from 'vitest';

Expand Down Expand Up @@ -43,4 +43,73 @@ describe.each([
const { document } = await render(<Cmp />, { debug });
expect(document.querySelector('div')).toHaveProperty('textContent', 'child');
});

it('should not duplicate projected element children when component throws a promise on first render', async () => {
const Wrapper = component$(() => (
<form>
<Slot />
</form>
));
const Title = component$(() => <header>title</header>);
const Footer = component$(() => <footer>footer</footer>);
const Cmp = component$(() => {
// The thrown promise writes a tracked signal, scheduling a second diff
// pass before <Wrapper> has rendered its <Slot>. The trailing <Footer />
// is required: matching it moves the cursor past the stale <div>, so
// expectNoMore() would not clean it up.
const store = useSignal<string>();
if (!store.value) {
throw Promise.resolve().then(() => {
store.value = 'resolved';
});
}
return (
<Wrapper>
<Title />
<div class="fields">{store.value}</div>
<Footer />
</Wrapper>
);
});
const { document } = await render(<Cmp />, { debug });
expect(document.querySelectorAll('div.fields')).toHaveLength(1);
expect(document.querySelectorAll('header')).toHaveLength(1);
expect(document.querySelectorAll('footer')).toHaveLength(1);
});

it('should not duplicate named slot content when component throws a promise on first render', async () => {
const Wrapper = component$(() => (
<form>
<section>
<Slot name="top" />
</section>
<Slot />
</form>
));
const Title = component$(() => <header>title</header>);
const Footer = component$(() => <footer>footer</footer>);
const Cmp = component$(() => {
const store = useSignal<string>();
if (!store.value) {
throw Promise.resolve().then(() => {
store.value = 'resolved';
});
}
return (
<Wrapper>
<div q:slot="top" class="banner">
banner
</div>
<Title />
<div class="fields">{store.value}</div>
<Footer />
</Wrapper>
);
});
const { document } = await render(<Cmp />, { debug });
expect(document.querySelectorAll('div.banner')).toHaveLength(1);
expect(document.querySelectorAll('div.fields')).toHaveLength(1);
expect(document.querySelectorAll('header')).toHaveLength(1);
expect(document.querySelectorAll('footer')).toHaveLength(1);
});
});
Loading