Skip to content
Draft
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
367 changes: 250 additions & 117 deletions packages/qwik/src/core/client/vnode-diff.ts

Large diffs are not rendered by default.

24 changes: 24 additions & 0 deletions packages/qwik/src/core/client/vnode-diff.unit.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,30 @@ describe('vNode-diff', () => {
expect(b1).toBe(selectB1());
expect(b2).toBe(selectB2());
});

it('should remove or add keyed nodes', () => {
const { vNode, vParent, container } = vnode_fromJSX(
_jsxSorted(
'test',
{},
null,
[_jsxSorted('b', {}, null, '1', 0, '1'), _jsxSorted('b', {}, null, '2', 0, null)],
0,
'KA_6'
)
);
const test = _jsxSorted(
'test',
{},
null,
[_jsxSorted('b', {}, null, '2', 0, null), _jsxSorted('b', {}, null, '2', 0, '2')],
0,
'KA_6'
);
vnode_diff(container, test, vParent, null);
vnode_applyJournal(container.$journal$);
expect(vNode).toMatchVDOM(test);
});
});
describe('fragments', () => {
it('should not rerender signal wrapper fragment', async () => {
Expand Down
8 changes: 6 additions & 2 deletions packages/qwik/src/core/client/vnode-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
import type { ChoreArray } from './chore-array';
import { _EFFECT_BACK_REF } from '../reactive-primitives/types';
import { BackRef } from '../reactive-primitives/cleanup';
import { isDev } from '@qwik.dev/core/build';

/** @internal */
export abstract class VNode extends BackRef {
Expand Down Expand Up @@ -91,8 +92,11 @@ export abstract class VNode extends BackRef {
}
}

toString() {
return vnode_toString.call(this);
toString(): string {
if (isDev) {
return vnode_toString.call(this);
}
return String(this);
}
}

Expand Down
24 changes: 20 additions & 4 deletions packages/qwik/src/core/client/vnode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,8 +183,9 @@ export const enum VNodeJournalOpCode {
SetText = 1, // ------ [SetAttribute, target, text]
SetAttribute = 2, // - [SetAttribute, target, ...(key, values)]]
HoistStyles = 3, // -- [HoistStyles, document]
Remove = 4, // ------- [Insert, target(parent), ...nodes]
Insert = 5, // ------- [Insert, target(parent), reference, ...nodes]
Remove = 4, // ------- [Remove, target(parent), ...nodes]
RemoveAll = 5, // ------- [RemoveAll, target(parent)]
Insert = 6, // ------- [Insert, target(parent), reference, ...nodes]
}

export type VNodeJournal = Array<
Expand Down Expand Up @@ -968,6 +969,15 @@ export const vnode_applyJournal = (journal: VNodeJournal) => {
idx++;
}
break;
case VNodeJournalOpCode.RemoveAll:
const removeAllParent = journal[idx++] as Element;
if (removeAllParent.replaceChildren) {
removeAllParent.replaceChildren();
} else {
// fallback if replaceChildren is not supported
removeAllParent.textContent = '';
}
break;
case VNodeJournalOpCode.Insert:
const insertParent = journal[idx++] as Element;
const insertBefore = journal[idx++] as Element | Text | null;
Expand Down Expand Up @@ -1220,8 +1230,14 @@ export const vnode_truncate = (
) => {
assertDefined(vDelete, 'Missing vDelete.');
const parent = vnode_getDomParent(vParent);
const children = vnode_getDOMChildNodes(journal, vDelete);
parent && children.length && journal.push(VNodeJournalOpCode.Remove, parent, ...children);
if (parent) {
if (vnode_isElementVNode(vParent)) {
journal.push(VNodeJournalOpCode.RemoveAll, parent);
} else {
const children = vnode_getDOMChildNodes(journal, vParent);
children.length && journal.push(VNodeJournalOpCode.Remove, parent, ...children);
}
}
const vPrevious = vDelete.previousSibling;
if (vPrevious) {
vPrevious.nextSibling = null;
Expand Down
2 changes: 2 additions & 0 deletions packages/qwik/src/core/debug.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ export function qwikDebugToString(value: any): any {
return 'Store';
} else if (isJSXNode(value)) {
return jsxToString(value);
} else if (vnode_isVNode(value)) {
return '(' + value.getProp(DEBUG_TYPE, null) + ')';
}
} finally {
stringifyPath.pop();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
import type { Signal } from '../signal.public';
import { SignalFlags, type EffectSubscription } from '../types';
import { ChoreType } from '../../shared/util-chore-type';
import type { WrappedSignalImpl } from './wrapped-signal-impl';

const DEBUG = false;
// eslint-disable-next-line no-console
Expand All @@ -23,8 +24,8 @@ export class SignalImpl<T = any> implements Signal<T> {

/** Store a list of effects which are dependent on this signal. */
$effects$: null | Set<EffectSubscription> = null;

$container$: Container | null = null;
$wrappedSignal$: WrappedSignalImpl<T> | null = null;

constructor(container: Container | null, value: T) {
this.$container$ = container;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type { Container, HostElement } from '../../shared/types';
import { ChoreType } from '../../shared/util-chore-type';
import { trackSignal } from '../../use/use-core';
import type { BackRef } from '../cleanup';
import { getValueProp } from '../internal-api';
import type { AllSignalFlags, EffectSubscription } from '../types';
import {
_EFFECT_BACK_REF,
Expand All @@ -12,7 +13,7 @@ import {
SignalFlags,
WrappedSignalFlags,
} from '../types';
import { scheduleEffects } from '../utils';
import { isSignal, scheduleEffects } from '../utils';
import { SignalImpl } from './signal-impl';

export class WrappedSignalImpl<T> extends SignalImpl<T> implements BackRef {
Expand Down Expand Up @@ -106,6 +107,13 @@ export class WrappedSignalImpl<T> extends SignalImpl<T> implements BackRef {
this.$untrackedValue$ = untrackedValue;
}
}

$unwrapIfSignal$(): SignalImpl<T> | WrappedSignalImpl<T> {
return this.$func$ === getValueProp && isSignal(this.$args$[0])
? (this.$args$[0] as SignalImpl<T>)
: this;
}

// Make this signal read-only
set value(_: any) {
throw qError(QError.wrappedReadOnly);
Expand Down
24 changes: 20 additions & 4 deletions packages/qwik/src/core/reactive-primitives/internal-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,34 @@ import { _CONST_PROPS, _IMMUTABLE } from '../shared/utils/constants';
import { assertEqual } from '../shared/error/assert';
import { isObject } from '../shared/utils/types';
import { isSignal, type Signal } from './signal.public';
import { getStoreTarget } from './impl/store';
import { getStoreTarget, isStore } from './impl/store';
import { isPropsProxy } from '../shared/jsx/jsx-runtime';
import { WrappedSignalFlags } from './types';
import { WrappedSignalImpl } from './impl/wrapped-signal-impl';
import { AsyncComputedSignalImpl } from './impl/async-computed-signal-impl';
import type { SignalImpl } from './impl/signal-impl';

// Keep these properties named like this so they're the same as from wrapSignal
const getValueProp = <T>(p0: { value: T }) => p0.value;
export const getValueProp = <T>(p0: { value: T }) => p0.value;
const getProp = <T extends object, P extends keyof T>(p0: T, p1: P) => p0[p1];

const getWrapped = <T extends object>(args: [T, (keyof T | undefined)?]) =>
new WrappedSignalImpl(null, args.length === 1 ? getValueProp : getProp, args, null);
const getWrapped = <T extends object>(args: [T, (keyof T | undefined)?]) => {
if (args.length === 1) {
if (isSignal(args[0])) {
return ((args[0] as unknown as SignalImpl).$wrappedSignal$ ||= new WrappedSignalImpl(
null,
getValueProp,
args,
null
));
} else if (isStore(args[0])) {
return new WrappedSignalImpl(null, getValueProp, args, null);
}
return (args[0] as { value: T }).value;
} else {
return new WrappedSignalImpl(null, getProp, args, null);
}
};

type PropType<T extends object, P extends keyof T> = P extends keyof T
? T[P]
Expand Down
2 changes: 1 addition & 1 deletion packages/qwik/src/core/shared/component-execution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ export const executeComponent = (
container.setHostProp(renderHost, USE_ON_LOCAL_SEQ_IDX, null);
}

if (vnode_isVNode(renderHost)) {
if (retryCount > 0 && vnode_isVNode(renderHost)) {
clearAllEffects(container, renderHost);
}

Expand Down
5 changes: 4 additions & 1 deletion packages/qwik/src/core/ssr/ssr-render-jsx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,11 @@ function processJSXNode(
} else if (isSignal(value)) {
ssr.openFragment(isDev ? [DEBUG_TYPE, VirtualType.WrappedSignal] : EMPTY_ARRAY);
const signalNode = ssr.getOrCreateLastNode();
const unwrappedSignal = value instanceof WrappedSignalImpl ? value.$unwrapIfSignal$() : value;
enqueue(ssr.closeFragment);
enqueue(() => trackSignalAndAssignHost(value, signalNode, EffectProperty.VNODE, ssr));
enqueue(() =>
trackSignalAndAssignHost(unwrappedSignal, signalNode, EffectProperty.VNODE, ssr)
);
enqueue(MaybeAsyncSignal);
} else if (isPromise(value)) {
ssr.openFragment(isDev ? [DEBUG_TYPE, VirtualType.Awaited] : EMPTY_ARRAY);
Expand Down
168 changes: 168 additions & 0 deletions packages/qwik/src/core/tests/component.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,49 @@ describe.each([
);
});

it('should render component with key', async () => {
(globalThis as any).componentExecuted = [];
const Cmp = component$(() => {
(globalThis as any).componentExecuted.push('Cmp');
return <div></div>;
});

const Parent = component$(() => {
const counter = useSignal(0);
return (
<>
<Cmp key={counter.value} />
<button id="counter" onClick$={() => counter.value++}></button>
</>
);
});

const { vNode, document } = await render(<Parent />, { debug });
expect((globalThis as any).componentExecuted).toEqual(['Cmp']);
expect(vNode).toMatchVDOM(
<Component ssr-required>
<Fragment ssr-required>
<Component>
<div></div>
</Component>
<button id="counter"></button>
</Fragment>
</Component>
);
await trigger(document.body, 'button', 'click');
expect((globalThis as any).componentExecuted).toEqual(['Cmp', 'Cmp']);
expect(vNode).toMatchVDOM(
<Component ssr-required>
<Fragment ssr-required>
<Component>
<div></div>
</Component>
<button id="counter"></button>
</Fragment>
</Component>
);
});

it('should handle null as empty string', async () => {
const MyComp = component$(() => {
return (
Expand Down Expand Up @@ -2410,6 +2453,131 @@ describe.each([
await expect(document.querySelector('div')).toMatchDOM(<div />);
});

it('should correctly remove all children for empty array', async () => {
const Cmp = component$(() => {
const list = useSignal([1, 2, 3]);
return (
<main>
<button onClick$={() => (list.value = [])}>Remove</button>
{list.value.map((item) => (
<div>{item}</div>
))}
</main>
);
});
const { vNode, document } = await render(<Cmp />, { debug });
expect(vNode).toMatchVDOM(
<Component>
<main>
<button>Remove</button>
<div>1</div>
<div>2</div>
<div>3</div>
</main>
</Component>
);
await trigger(document.body, 'button', 'click');
expect(vNode).toMatchVDOM(
<Component>
<main>
<button>Remove</button>
</main>
</Component>
);
expect(document.querySelector('main')).toMatchDOM(
<main>
<button>Remove</button>
</main>
);
});

it('should correctly remove all children for empty array - case 2', async () => {
const Cmp = component$(() => {
const list = useSignal([1, 2, 3]);
return (
<main>
<button onClick$={() => (list.value = [])}>Remove</button>
<div>
{list.value.map((item) => (
<div>{item}</div>
))}
</div>
</main>
);
});
const { vNode, document } = await render(<Cmp />, { debug });
expect(vNode).toMatchVDOM(
<Component>
<main>
<button>Remove</button>
<div>
<div>1</div>
<div>2</div>
<div>3</div>
</div>
</main>
</Component>
);
await trigger(document.body, 'button', 'click');
expect(vNode).toMatchVDOM(
<Component>
<main>
<button>Remove</button>
<div></div>
</main>
</Component>
);
expect(document.querySelector('main')).toMatchDOM(
<main>
<button>Remove</button>
<div></div>
</main>
);
});

it('should correctly remove all children for empty array within virtual node', async () => {
const Cmp = component$(() => {
const list = useSignal([1, 2, 3]);
return (
<main>
<button onClick$={() => (list.value = [])}>Remove</button>
<>
{list.value.map((item) => (
<div>{item}</div>
))}
</>
</main>
);
});
const { vNode, document } = await render(<Cmp />, { debug });
expect(vNode).toMatchVDOM(
<Component>
<main>
<button>Remove</button>
<Fragment ssr-required>
<div>1</div>
<div>2</div>
<div>3</div>
</Fragment>
</main>
</Component>
);
await trigger(document.body, 'button', 'click');
expect(vNode).toMatchVDOM(
<Component>
<main>
<button>Remove</button>
<Fragment ssr-required></Fragment>
</main>
</Component>
);
await expect(document.querySelector('main')).toMatchDOM(
<main>
<button>Remove</button>
</main>
);
});

describe('regression', () => {
it('#3643', async () => {
const Issue3643 = component$(() => {
Expand Down
Loading