Skip to content

Commit 313d53e

Browse files
committed
perf: create wrapped signal only once for vnode diff
# Conflicts: # packages/qwik/src/core/reactive-primitives/internal-api.ts
1 parent 5eb1cd6 commit 313d53e

File tree

6 files changed

+55
-20
lines changed

6 files changed

+55
-20
lines changed

packages/qwik/src/core/client/vnode-diff.ts

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,10 @@ import { clearAllEffects } from '../reactive-primitives/cleanup';
8383
import { serializeAttribute } from '../shared/utils/styles';
8484
import { QError, qError } from '../shared/error/error';
8585
import { getFileLocationFromJsx } from '../shared/utils/jsx-filename';
86-
import { EffectProperty } from '../reactive-primitives/types';
86+
import { EffectProperty, EffectSubscriptionProp } from '../reactive-primitives/types';
8787
import { SubscriptionData } from '../reactive-primitives/subscription-data';
8888
import { WrappedSignalImpl } from '../reactive-primitives/impl/wrapped-signal-impl';
89-
import { _CONST_PROPS, _VAR_PROPS } from '../internal';
89+
import { _CONST_PROPS, _EFFECT_BACK_REF, _VAR_PROPS } from '../internal';
9090
import { isSyncQrl } from '../shared/qrl/qrl-utils';
9191
import type { ElementVNode, TextVNode, VirtualVNode, VNode } from './vnode-impl';
9292

@@ -184,15 +184,22 @@ export const vnode_diff = (
184184
descend(jsxValue, false);
185185
} else if (isSignal(jsxValue)) {
186186
expectVirtual(VirtualType.WrappedSignal, null);
187-
descend(
188-
trackSignalAndAssignHost(
189-
jsxValue as Signal,
190-
(vNewNode || vCurrent)!,
191-
EffectProperty.VNODE,
192-
container
193-
),
194-
true
195-
);
187+
const unwrappedSignal =
188+
jsxValue instanceof WrappedSignalImpl ? jsxValue.$unwrapIfSignal$() : jsxValue;
189+
const currentSignal = vCurrent?.[_EFFECT_BACK_REF]?.get(EffectProperty.VNODE)?.[
190+
EffectSubscriptionProp.CONSUMER
191+
];
192+
if (currentSignal !== unwrappedSignal) {
193+
descend(
194+
trackSignalAndAssignHost(
195+
unwrappedSignal,
196+
(vNewNode || vCurrent)!,
197+
EffectProperty.VNODE,
198+
container
199+
),
200+
true
201+
);
202+
}
196203
} else if (isPromise(jsxValue)) {
197204
expectVirtual(VirtualType.Awaited, null);
198205
asyncQueue.push(jsxValue, vNewNode || vCurrent);

packages/qwik/src/core/reactive-primitives/impl/signal-impl.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
import type { Signal } from '../signal.public';
1414
import { SignalFlags, type EffectSubscription } from '../types';
1515
import { ChoreType } from '../../shared/util-chore-type';
16+
import type { WrappedSignalImpl } from './wrapped-signal-impl';
1617

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

2425
/** Store a list of effects which are dependent on this signal. */
2526
$effects$: null | Set<EffectSubscription> = null;
26-
2727
$container$: Container | null = null;
28+
$wrappedSignal$: WrappedSignalImpl<T> | null = null;
2829

2930
constructor(container: Container | null, value: T) {
3031
this.$container$ = container;

packages/qwik/src/core/reactive-primitives/impl/wrapped-signal-impl.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import type { Container, HostElement } from '../../shared/types';
44
import { ChoreType } from '../../shared/util-chore-type';
55
import { trackSignal } from '../../use/use-core';
66
import type { BackRef } from '../cleanup';
7+
import { getValueProp } from '../internal-api';
78
import type { AllSignalFlags, EffectSubscription } from '../types';
89
import {
910
_EFFECT_BACK_REF,
@@ -12,7 +13,7 @@ import {
1213
SignalFlags,
1314
WrappedSignalFlags,
1415
} from '../types';
15-
import { scheduleEffects } from '../utils';
16+
import { isSignal, scheduleEffects } from '../utils';
1617
import { SignalImpl } from './signal-impl';
1718

1819
export class WrappedSignalImpl<T> extends SignalImpl<T> implements BackRef {
@@ -106,6 +107,13 @@ export class WrappedSignalImpl<T> extends SignalImpl<T> implements BackRef {
106107
this.$untrackedValue$ = untrackedValue;
107108
}
108109
}
110+
111+
$unwrapIfSignal$(): SignalImpl<T> | WrappedSignalImpl<T> {
112+
return this.$func$ === getValueProp && isSignal(this.$args$[0])
113+
? (this.$args$[0] as SignalImpl<T>)
114+
: this;
115+
}
116+
109117
// Make this signal read-only
110118
set value(_: any) {
111119
throw qError(QError.wrappedReadOnly);

packages/qwik/src/core/reactive-primitives/internal-api.ts

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,34 @@ import { _CONST_PROPS, _IMMUTABLE } from '../shared/utils/constants';
22
import { assertEqual } from '../shared/error/assert';
33
import { isObject } from '../shared/utils/types';
44
import { isSignal, type Signal } from './signal.public';
5-
import { getStoreTarget } from './impl/store';
5+
import { getStoreTarget, isStore } from './impl/store';
66
import { isPropsProxy } from '../shared/jsx/jsx-runtime';
77
import { WrappedSignalFlags } from './types';
88
import { WrappedSignalImpl } from './impl/wrapped-signal-impl';
99
import { AsyncComputedSignalImpl } from './impl/async-computed-signal-impl';
10+
import type { SignalImpl } from './impl/signal-impl';
1011

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

15-
const getWrapped = <T extends object>(args: [T, (keyof T | undefined)?]) =>
16-
new WrappedSignalImpl(null, args.length === 1 ? getValueProp : getProp, args, null);
16+
const getWrapped = <T extends object>(args: [T, (keyof T | undefined)?]) => {
17+
if (args.length === 1) {
18+
if (isSignal(args[0])) {
19+
return ((args[0] as SignalImpl).$wrappedSignal$ ||= new WrappedSignalImpl(
20+
null,
21+
getValueProp,
22+
args,
23+
null
24+
));
25+
} else if (isStore(args[0])) {
26+
return new WrappedSignalImpl(null, getValueProp, args, null);
27+
}
28+
return args[0].value;
29+
} else {
30+
return new WrappedSignalImpl(null, getProp, args, null);
31+
}
32+
};
1733

1834
type PropType<T extends object, P extends keyof T> = P extends keyof T
1935
? T[P]

packages/qwik/src/core/ssr/ssr-render-jsx.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,8 +129,11 @@ function processJSXNode(
129129
} else if (isSignal(value)) {
130130
ssr.openFragment(isDev ? [DEBUG_TYPE, VirtualType.WrappedSignal] : EMPTY_ARRAY);
131131
const signalNode = ssr.getOrCreateLastNode();
132+
const unwrappedSignal = value instanceof WrappedSignalImpl ? value.$unwrapIfSignal$() : value;
132133
enqueue(ssr.closeFragment);
133-
enqueue(() => trackSignalAndAssignHost(value, signalNode, EffectProperty.VNODE, ssr));
134+
enqueue(() =>
135+
trackSignalAndAssignHost(unwrappedSignal, signalNode, EffectProperty.VNODE, ssr)
136+
);
134137
enqueue(MaybeAsyncSignal);
135138
} else if (isPromise(value)) {
136139
ssr.openFragment(isDev ? [DEBUG_TYPE, VirtualType.Awaited] : EMPTY_ARRAY);

packages/qwik/src/core/tests/render-api.spec.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -752,7 +752,7 @@ describe('render api', () => {
752752
}}
753753
>
754754
<Resource value={rsrc} onResolved={(v) => <span>{v}</span>} />
755-
{sig.value}
755+
{sig.value + 'test'}
756756
</button>
757757
);
758758
});
@@ -964,7 +964,7 @@ describe('render api', () => {
964964
streaming,
965965
});
966966
// This can change when the size of the output changes
967-
expect(stream.write).toHaveBeenCalledTimes(7);
967+
expect(stream.write).toHaveBeenCalledTimes(5);
968968
});
969969
});
970970
});

0 commit comments

Comments
 (0)