Skip to content

Commit 927f3d2

Browse files
committed
feedback
1 parent 679929a commit 927f3d2

File tree

5 files changed

+39
-39
lines changed

5 files changed

+39
-39
lines changed

packages/svelte/src/internal/client/reactivity/deriveds.js

Lines changed: 18 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -35,25 +35,24 @@ import { component_context } from '../context.js';
3535
/*#__NO_SIDE_EFFECTS__*/
3636
export function derived(fn) {
3737
var flags = DERIVED | DIRTY;
38+
var parent_derived =
39+
active_reaction !== null && (active_reaction.f & DERIVED) !== 0
40+
? /** @type {Derived} */ (active_reaction)
41+
: null;
3842

39-
if (active_effect === null) {
43+
if (active_effect === null || (parent_derived !== null && (parent_derived.f & UNOWNED) !== 0)) {
4044
flags |= UNOWNED;
4145
} else {
4246
// Since deriveds are evaluated lazily, any effects created inside them are
4347
// created too late to ensure that the parent effect is added to the tree
4448
active_effect.f |= EFFECT_HAS_DERIVED;
4549
}
4650

47-
var parent_derived =
48-
active_reaction !== null && (active_reaction.f & DERIVED) !== 0
49-
? /** @type {Derived} */ (active_reaction)
50-
: null;
51-
5251
/** @type {Derived<V>} */
5352
const signal = {
54-
children: null,
5553
ctx: component_context,
5654
deps: null,
55+
effects: null,
5756
equals,
5857
f: flags,
5958
fn,
@@ -87,19 +86,14 @@ export function derived_safe_equal(fn) {
8786
* @param {Derived} derived
8887
* @returns {void}
8988
*/
90-
export function destroy_derived_children(derived) {
91-
var children = derived.children;
92-
93-
if (children !== null) {
94-
derived.children = null;
95-
96-
for (var i = 0; i < children.length; i += 1) {
97-
var child = children[i];
98-
if ((child.f & DERIVED) !== 0) {
99-
destroy_derived(/** @type {Derived} */ (child));
100-
} else {
101-
destroy_effect(/** @type {Effect} */ (child));
102-
}
89+
export function destroy_derived_effects(derived) {
90+
var effects = derived.effects;
91+
92+
if (effects !== null) {
93+
derived.effects = null;
94+
95+
for (var i = 0; i < effects.length; i += 1) {
96+
destroy_effect(/** @type {Effect} */ (effects[i]));
10397
}
10498
}
10599
}
@@ -147,7 +141,7 @@ export function execute_derived(derived) {
147141

148142
stack.push(derived);
149143

150-
destroy_derived_children(derived);
144+
destroy_derived_effects(derived);
151145
value = update_reaction(derived);
152146
} finally {
153147
set_active_effect(prev_active_effect);
@@ -156,7 +150,7 @@ export function execute_derived(derived) {
156150
}
157151
} else {
158152
try {
159-
destroy_derived_children(derived);
153+
destroy_derived_effects(derived);
160154
value = update_reaction(derived);
161155
} finally {
162156
set_active_effect(prev_active_effect);
@@ -188,9 +182,9 @@ export function update_derived(derived) {
188182
* @returns {void}
189183
*/
190184
export function destroy_derived(derived) {
191-
destroy_derived_children(derived);
185+
destroy_derived_effects(derived);
192186
remove_reactions(derived, 0);
193187
set_signal_status(derived, DESTROYED);
194188

195-
derived.v = derived.children = derived.deps = derived.ctx = derived.reactions = null;
189+
derived.v = derived.deps = derived.ctx = derived.reactions = null;
196190
}

packages/svelte/src/internal/client/reactivity/effects.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ export function validate_effect(rune) {
5353
e.effect_orphan(rune);
5454
}
5555

56-
if (active_reaction !== null && (active_reaction.f & UNOWNED) !== 0) {
56+
if (active_reaction !== null && (active_reaction.f & UNOWNED) !== 0 && active_effect === null) {
5757
e.effect_in_unowned_derived();
5858
}
5959

@@ -153,7 +153,7 @@ function create_effect(type, fn, sync, push = true) {
153153
// if we're in a derived, add the effect there too
154154
if (active_reaction !== null && (active_reaction.f & DERIVED) !== 0) {
155155
var derived = /** @type {Derived} */ (active_reaction);
156-
(derived.children ??= []).push(effect);
156+
(derived.effects ??= []).push(effect);
157157
}
158158
}
159159

packages/svelte/src/internal/client/reactivity/types.d.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@ export interface Reaction extends Signal {
3636
export interface Derived<V = unknown> extends Value<V>, Reaction {
3737
/** The derived function */
3838
fn: () => V;
39-
/** Reactions created inside this signal */
40-
children: null | Reaction[];
39+
/** Effects created inside this signal */
40+
effects: null | Effect[];
4141
/** Parent effect or derived */
4242
parent: Effect | Derived | null;
4343
}

packages/svelte/src/internal/client/runtime.js

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import { flush_tasks } from './dom/task.js';
3030
import { internal_set, set } from './reactivity/sources.js';
3131
import {
3232
destroy_derived,
33-
destroy_derived_children,
33+
destroy_derived_effects,
3434
execute_derived,
3535
update_derived
3636
} from './reactivity/deriveds.js';
@@ -419,11 +419,10 @@ export function update_reaction(reaction) {
419419
(flags & UNOWNED) !== 0 &&
420420
(!is_flushing_effect ||
421421
// If we were previously not in a reactive context and we're reading an unowned derived
422-
// that was created inside another derived, then we don't fully know the real owner and thus
422+
// that was created inside another reaction, then we don't fully know the real owner and thus
423423
// we need to skip adding any reactions for this unowned
424424
((previous_reaction === null || previous_untracking) &&
425-
(/** @type {Derived} */ (reaction).parent !== null &&
426-
(/** @type {Derived} */ (reaction).parent.f & DERIVED) !== 0)));
425+
/** @type {Derived} */ (reaction).parent !== null));
427426

428427
derived_sources = null;
429428
set_component_context(reaction.ctx);
@@ -533,7 +532,7 @@ function remove_reaction(signal, dependency) {
533532
dependency.f ^= DISCONNECTED;
534533
}
535534
// Disconnect any reactions owned by this reaction
536-
destroy_derived_children(/** @type {Derived} **/ (dependency));
535+
destroy_derived_effects(/** @type {Derived} **/ (dependency));
537536
remove_reactions(/** @type {Derived} **/ (dependency), 0);
538537
}
539538
}
@@ -951,11 +950,18 @@ export function get(signal) {
951950
new_deps.push(signal);
952951
}
953952
}
954-
} else if (is_derived && /** @type {Derived} */ (signal).deps === null) {
953+
} else if (
954+
is_derived &&
955+
/** @type {Derived} */ (signal).deps === null &&
956+
/** @type {Derived} */ (signal).effects === null
957+
) {
955958
var derived = /** @type {Derived} */ (signal);
956959
var parent = derived.parent;
957960

958961
if (parent !== null) {
962+
// if ((parent.f & UNOWNED) === 0) {
963+
// derived.f ^= UNOWNED;
964+
// }
959965
// If the derived is owned by another derived then mark it as unowned
960966
// as the derived value might have been referenced in a different context
961967
// since and thus its parent might not be its true owner anymore

packages/svelte/tests/signals/test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -928,7 +928,7 @@ describe('signals', () => {
928928
});
929929

930930
assert.deepEqual(a.deriveds?.length, 1);
931-
assert.deepEqual(b?.children, null);
931+
assert.deepEqual(b?.effects, null);
932932

933933
destroy();
934934

@@ -957,14 +957,14 @@ describe('signals', () => {
957957
});
958958
});
959959

960-
assert.equal(c!.children?.length, 1);
960+
assert.equal(c!.effects?.length, 1);
961961
assert.equal(a.first, a.last);
962962

963963
set(b, 1);
964964

965965
flushSync();
966966

967-
assert.equal(c!.children?.length, 1);
967+
assert.equal(c!.effects?.length, 1);
968968
assert.equal(a.first, a.last);
969969

970970
destroy();
@@ -1001,14 +1001,14 @@ describe('signals', () => {
10011001
});
10021002
});
10031003

1004-
assert.equal(c!.children?.length, 1);
1004+
assert.equal(c!.effects?.length, 1);
10051005
assert.equal(a.first, a.last);
10061006

10071007
set(b, 1);
10081008

10091009
flushSync();
10101010

1011-
assert.equal(c!.children?.length, 1);
1011+
assert.equal(c!.effects?.length, 1);
10121012
assert.equal(a.first, a.last);
10131013

10141014
destroy();

0 commit comments

Comments
 (0)