Skip to content

Commit 36e22f8

Browse files
Varixowmertens
Varixo
and
wmertens
committed
feat: global map of all effect subscriptions
Co-authored-by: Wout Mertens <Wout.Mertens@gmail.com>
1 parent 269bd8d commit 36e22f8

File tree

12 files changed

+68
-65
lines changed

12 files changed

+68
-65
lines changed

‎packages/qwik/src/core/shared/component-execution.ts‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import { MAX_RETRY_ON_PROMISE_COUNT, isPromise, maybeThen, safeCall } from './ut
2020
import type { ValueOrPromise } from './utils/types';
2121
import type { Container, HostElement } from './types';
2222
import { logWarn } from './utils/log';
23-
import { EffectProperty, isSignal } from '../signal/signal';
23+
import { EffectProperty, isSignal, getSubscriber } from '../signal/signal';
2424
import { vnode_isVNode } from '../client/vnode';
2525
import { clearVNodeEffectDependencies } from '../signal/signal-subscriber';
2626
import { Slot } from '../shared/jsx/slot.public';
@@ -49,13 +49,20 @@ import { Slot } from '../shared/jsx/slot.public';
4949
export const executeComponent = (
5050
container: Container,
5151
renderHost: HostElement,
52-
subscriptionHost: HostElement,
52+
subscriptionHost: HostElement | null,
5353
componentQRL: OnRenderFn<unknown> | QRLInternal<OnRenderFn<unknown>> | null,
5454
props: Props | null
5555
): ValueOrPromise<JSXOutput> => {
56-
const iCtx = newInvokeContext(container.$locale$, subscriptionHost, undefined, RenderEvent);
57-
iCtx.$effectSubscriber$ = [subscriptionHost, EffectProperty.COMPONENT];
58-
iCtx.$container$ = container;
56+
const iCtx = newInvokeContext(
57+
container.$locale$,
58+
subscriptionHost || undefined,
59+
undefined,
60+
RenderEvent
61+
);
62+
if (subscriptionHost) {
63+
iCtx.$effectSubscriber$ = getSubscriber(subscriptionHost, EffectProperty.COMPONENT);
64+
iCtx.$container$ = container;
65+
}
5966
let componentFn: (props: unknown) => ValueOrPromise<JSXOutput>;
6067
container.ensureProjectionResolved(renderHost);
6168
let isInlineComponent = false;

‎packages/qwik/src/core/shared/shared-serialization.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -274,9 +274,9 @@ const inflate = (
274274
}
275275
case TypeIds.Signal: {
276276
const signal = target as Signal<unknown>;
277-
const d = data as [unknown, Set<EffectSubscriptions>];
277+
const d = data as [unknown, ...EffectSubscriptions[]];
278278
signal.$untrackedValue$ = d[0];
279-
signal.$effects$ = d[1];
279+
signal.$effects$ = new Set(d.slice(1) as EffectSubscriptions[]);
280280
break;
281281
}
282282
case TypeIds.WrappedSignal: {
@@ -287,14 +287,14 @@ const inflate = (
287287
Set<Subscriber>,
288288
unknown,
289289
HostElement,
290-
Set<EffectSubscriptions>,
290+
...EffectSubscriptions[],
291291
];
292292
signal.$func$ = container.getSyncFn(d[0]);
293293
signal.$args$ = d[1];
294294
signal.$effectDependencies$ = d[2];
295295
signal.$untrackedValue$ = d[3];
296296
signal.$hostElement$ = d[4];
297-
signal.$effects$ = d[5];
297+
signal.$effects$ = new Set(d.slice(5) as EffectSubscriptions[]);
298298
break;
299299
}
300300
case TypeIds.ComputedSignal: {
@@ -1188,7 +1188,7 @@ function serialize(serializationContext: SerializationContext): void {
11881188
value.$effectDependencies$,
11891189
v,
11901190
value.$hostElement$,
1191-
value.$effects$,
1191+
...(value.$effects$ || []),
11921192
]);
11931193
} else if (value instanceof ComputedSignal) {
11941194
const out = [
@@ -1201,7 +1201,7 @@ function serialize(serializationContext: SerializationContext): void {
12011201
}
12021202
output(TypeIds.ComputedSignal, out);
12031203
} else {
1204-
output(TypeIds.Signal, [v, value.$effects$]);
1204+
output(TypeIds.Signal, [v, ...(value.$effects$ || [])]);
12051205
}
12061206
} else if (value instanceof URL) {
12071207
output(TypeIds.URL, value.href);

‎packages/qwik/src/core/signal/signal-subscriber.ts‎

Lines changed: 9 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,6 @@
11
import { QSubscribers } from '../shared/utils/markers';
2-
import type { ElementVNode, VNode, VirtualVNode } from '../client/types';
3-
import {
4-
ensureMaterialized,
5-
vnode_getProp,
6-
vnode_isElementVNode,
7-
vnode_setProp,
8-
} from '../client/vnode';
2+
import type { VNode } from '../client/types';
3+
import { ensureMaterialized, vnode_getProp, vnode_isElementVNode } from '../client/vnode';
94
import { EffectSubscriptionsProp, WrappedSignal, isSignal, type Signal } from './signal';
105
import type { Container } from '../shared/types';
116
import { StoreHandler, getStoreHandler, isStore, type TargetType } from './store';
@@ -24,48 +19,28 @@ export function clearVNodeEffectDependencies(container: Container, value: VNode)
2419
if (vnode_isElementVNode(value)) {
2520
ensureMaterialized(value);
2621
}
27-
const effects = vnode_getProp<Subscriber[]>(value, QSubscribers, container.$getObjectById$);
22+
const effects = vnode_getProp<Set<Subscriber>>(value, QSubscribers, container.$getObjectById$);
23+
2824
if (!effects) {
2925
return;
3026
}
31-
for (let i = effects.length - 1; i >= 0; i--) {
32-
const subscriber = effects[i];
33-
clearEffects(subscriber, value, effects, i, container);
34-
}
35-
36-
if (effects.length === 0) {
37-
vnode_setProp(value as ElementVNode | VirtualVNode, QSubscribers, null);
27+
for (const subscriber of effects) {
28+
clearEffects(subscriber, value, effects, container);
3829
}
3930
}
4031

4132
export function clearSubscriberEffectDependencies(container: Container, value: Subscriber): void {
4233
if (value.$effectDependencies$) {
4334
for (const subscriber of value.$effectDependencies$) {
44-
let subscriptionRemoved = false;
45-
const seenSet = new Set();
46-
if (subscriber instanceof WrappedSignal) {
47-
subscriptionRemoved = clearSignalEffects(subscriber, value, seenSet);
48-
} else if (container.$storeProxyMap$.has(subscriber)) {
49-
const store = container.$storeProxyMap$.get(subscriber)!;
50-
const handler = getStoreHandler(store)!;
51-
subscriptionRemoved = clearStoreEffects(handler, value);
52-
}
53-
if (subscriptionRemoved) {
54-
value.$effectDependencies$.delete(subscriber);
55-
}
56-
}
57-
58-
if (value.$effectDependencies$.size === 0) {
59-
value.$effectDependencies$.clear();
35+
clearEffects(subscriber, value, value.$effectDependencies$, container);
6036
}
6137
}
6238
}
6339

6440
function clearEffects(
6541
subscriber: Subscriber | TargetType,
6642
value: Subscriber | VNode,
67-
effectArray: (Subscriber | TargetType)[],
68-
indexToRemove: number,
43+
effectArray: Set<Subscriber | TargetType>,
6944
container: Container
7045
) {
7146
let subscriptionRemoved = false;
@@ -78,7 +53,7 @@ function clearEffects(
7853
subscriptionRemoved = clearStoreEffects(handler, value);
7954
}
8055
if (subscriptionRemoved) {
81-
effectArray.splice(indexToRemove, 1);
56+
effectArray.delete(subscriber);
8257
}
8358
}
8459

‎packages/qwik/src/core/signal/signal.ts‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,26 @@ export type EffectSubscriptions = [
134134
| TargetType
135135
)[],
136136
];
137+
138+
/** Global map of all effect subscriptions */
139+
const effectMapMap = new WeakMap<Effect, Map<EffectProperty | string, EffectSubscriptions>>();
140+
export function getSubscriber(effect: Effect, prop: EffectProperty | string, data?: unknown) {
141+
let subMap = effectMapMap.get(effect);
142+
if (!subMap) {
143+
subMap = new Map();
144+
effectMapMap.set(effect, subMap);
145+
}
146+
let sub = subMap.get(prop);
147+
if (!sub) {
148+
sub = [effect, prop];
149+
subMap.set(prop, sub);
150+
}
151+
if (data) {
152+
sub[EffectSubscriptionsProp.FIRST_BACK_REF_OR_DATA] = data;
153+
}
154+
return sub;
155+
}
156+
137157
export const enum EffectSubscriptionsProp {
138158
EFFECT = 0,
139159
PROPERTY = 1,
@@ -250,9 +270,6 @@ export const ensureContainsEffect = (
250270
array: Set<EffectSubscriptions>,
251271
effectSubscriptions: EffectSubscriptions
252272
) => {
253-
if (array.has(effectSubscriptions)) {
254-
return;
255-
}
256273
array.add(effectSubscriptions);
257274
};
258275

@@ -444,7 +461,7 @@ export class ComputedSignal<T> extends Signal<T> {
444461

445462
const ctx = tryGetInvokeContext();
446463
const previousEffectSubscription = ctx?.$effectSubscriber$;
447-
ctx && (ctx.$effectSubscriber$ = [this, EffectProperty.VNODE]);
464+
ctx && (ctx.$effectSubscriber$ = getSubscriber(this, EffectProperty.VNODE));
448465
try {
449466
const untrackedValue = computeQrl.getFn(ctx)() as T;
450467
if (isPromise(untrackedValue)) {

‎packages/qwik/src/core/signal/signal.unit.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { invoke, newInvokeContext } from '../use/use-core';
1313
import { Task } from '../use/use-task';
1414
import {
1515
EffectProperty,
16+
getSubscriber,
1617
type EffectSubscriptions,
1718
type InternalReadonlySignal,
1819
type InternalSignal,
@@ -167,7 +168,8 @@ describe('v2-signal', () => {
167168
} else {
168169
const ctx = newInvokeContext();
169170
ctx.$container$ = container;
170-
const subscriber: EffectSubscriptions = [task, EffectProperty.COMPONENT, ctx];
171+
// TODO is ctx needed?
172+
const subscriber: EffectSubscriptions = getSubscriber(task, EffectProperty.COMPONENT, ctx);
171173
ctx.$effectSubscriber$ = subscriber;
172174
return invoke(ctx, qrl.getFn(ctx));
173175
}

‎packages/qwik/src/core/signal/store.unit.tsx‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { createDocument, getTestPlatform } from '@qwik.dev/core/testing';
33
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
44
import type { Container, HostElement } from '../shared/types';
55
import { StoreFlags, getOrCreateStore, isStore } from './store';
6-
import { EffectProperty } from './signal';
6+
import { EffectProperty, getSubscriber } from './signal';
77
import { invoke } from '../use/use-core';
88
import { newInvokeContext } from '../use/use-core';
99
import { ChoreType } from '../shared/scheduler';
@@ -73,7 +73,8 @@ describe('v2/store', () => {
7373
} else {
7474
const ctx = newInvokeContext();
7575
ctx.$container$ = container;
76-
const subscriber: EffectSubscriptions = [task, EffectProperty.COMPONENT, ctx];
76+
// TODO is ctx needed
77+
const subscriber: EffectSubscriptions = getSubscriber(task, EffectProperty.COMPONENT, ctx);
7778
ctx.$effectSubscriber$ = subscriber;
7879
return invoke(ctx, qrl.getFn(ctx));
7980
}

‎packages/qwik/src/core/ssr/ssr-render-component.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import type { JSXOutput } from '../shared/jsx/types/jsx-node';
1010

1111
export const applyInlineComponent = (
1212
ssr: SSRContainer,
13-
componentHost: ISsrNode,
13+
componentHost: ISsrNode | null,
1414
inlineComponentFunction: OnRenderFn<any>,
1515
jsx: JSXNode
1616
) => {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ function processJSXNode(
268268
: inlineComponentProps
269269
);
270270
enqueue(ssr.closeFragment);
271-
const component = ssr.getComponentFrame(0)!;
271+
const component = ssr.getComponentFrame(0);
272272
const jsxOutput = applyInlineComponent(
273273
ssr,
274274
component && component.componentNode,

‎packages/qwik/src/core/tests/use-signal.spec.tsx‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,8 @@ describe.each([
354354

355355
expect(
356356
// wrapped signal on the pre element
357-
(globalThis as any).signal.$effects$[0][EffectSubscriptionsProp.EFFECT].$effects$
357+
(globalThis as any).signal.$effects$.values().next().value[EffectSubscriptionsProp.EFFECT]
358+
.$effects$
358359
).toHaveLength(1);
359360
expect((globalThis as any).signal.$effects$).toHaveLength(1);
360361

@@ -364,7 +365,8 @@ describe.each([
364365
await trigger(container.element, 'button', 'click'); // <-- this should not add another subscriber
365366
expect((globalThis as any).signal.$effects$).toHaveLength(1);
366367
expect(
367-
(globalThis as any).signal.$effects$[0][EffectSubscriptionsProp.EFFECT].$effects$
368+
(globalThis as any).signal.$effects$.values().next().value[EffectSubscriptionsProp.EFFECT]
369+
.$effects$
368370
).toHaveLength(1);
369371

370372
await trigger(container.element, 'button', 'click');
@@ -373,7 +375,8 @@ describe.each([
373375
await trigger(container.element, 'button', 'click'); // <-- this should not add another subscriber
374376
expect((globalThis as any).signal.$effects$).toHaveLength(1);
375377
expect(
376-
(globalThis as any).signal.$effects$[0][EffectSubscriptionsProp.EFFECT].$effects$
378+
(globalThis as any).signal.$effects$.values().next().value[EffectSubscriptionsProp.EFFECT]
379+
.$effects$
377380
).toHaveLength(1);
378381

379382
(globalThis as any).signal = undefined;

‎packages/qwik/src/core/use/use-core.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { _getQContainerElement, getDomContainer } from '../client/dom-container'
2020
import { type ContainerElement } from '../client/types';
2121
import {
2222
WrappedSignal,
23+
getSubscriber,
2324
type EffectPropData,
2425
type EffectSubscriptions,
2526
type EffectSubscriptionsProp,
@@ -243,10 +244,7 @@ export const trackSignal = <T>(
243244
const previousSubscriber = trackInvocation.$effectSubscriber$;
244245
const previousContainer = trackInvocation.$container$;
245246
try {
246-
trackInvocation.$effectSubscriber$ = [subscriber, property];
247-
if (data) {
248-
trackInvocation.$effectSubscriber$.push(data);
249-
}
247+
trackInvocation.$effectSubscriber$ = getSubscriber(subscriber, property, data);
250248
trackInvocation.$container$ = container;
251249
return invoke(trackInvocation, fn);
252250
} finally {

0 commit comments

Comments
 (0)