Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: replicate Svelte 4 props update detection in legacy mode #11577

Merged
merged 6 commits into from
May 13, 2024
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/empty-flowers-change.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"svelte": patch
---

fix: replicate Svelte 4 props update detection in legacy mode
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,7 @@ export function serialize_get_binding(node, state) {
}

if (binding.kind === 'prop' || binding.kind === 'bindable_prop') {
if (
state.analysis.accessors ||
(state.analysis.immutable ? binding.reassigned : binding.mutated) ||
binding.initial
) {
if (!state.analysis.runes || binding.reassigned || binding.initial) {
return b.call(node);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,24 +93,17 @@ export const javascript_visitors_legacy = {
state.scope.get(declarator.id.name)
);

if (
state.analysis.accessors ||
(state.analysis.immutable ? binding.reassigned : binding.mutated) ||
declarator.init
) {
declarations.push(
b.declarator(
declarator.id,
get_prop_source(
binding,
state,
binding.prop_alias ?? declarator.id.name,
declarator.init &&
/** @type {import('estree').Expression} */ (visit(declarator.init))
)
declarations.push(
b.declarator(
declarator.id,
get_prop_source(
binding,
state,
binding.prop_alias ?? declarator.id.name,
declarator.init && /** @type {import('estree').Expression} */ (visit(declarator.init))
)
);
}
)
);

continue;
}
Expand Down
3 changes: 2 additions & 1 deletion packages/svelte/src/internal/client/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@ export const MAYBE_DIRTY = 1 << 10;
export const INERT = 1 << 11;
export const DESTROYED = 1 << 12;
export const EFFECT_RAN = 1 << 13;

/** 'Transparent' effects do not create a transition boundary */
export const EFFECT_TRANSPARENT = 1 << 14;
/** Svelte 4 legacy mode props need to be handled with deriveds and be recognized elsewhere, hence the dedicated flag */
export const LEGACY_DERIVED_PROP = 1 << 15;

export const STATE_SYMBOL = Symbol('$state');
37 changes: 24 additions & 13 deletions packages/svelte/src/internal/client/reactivity/props.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@ import {
} from '../../../constants.js';
import { get_descriptor, is_function } from '../utils.js';
import { mutable_source, set, source } from './sources.js';
import { derived } from './deriveds.js';
import { derived, derived_safe_equal } from './deriveds.js';
import { get, is_signals_recorded, untrack, update } from '../runtime.js';
import { safe_equals } from './equality.js';
import { inspect_fn } from '../dev/inspect.js';
import * as e from '../errors.js';
import { LEGACY_DERIVED_PROP } from '../constants.js';

/**
* @param {((value?: number) => number)} fn
Expand Down Expand Up @@ -236,18 +237,28 @@ export function prop(props, key, flags, fallback) {
if (setter) setter(prop_value);
}

var getter = runes
? () => {
var value = /** @type {V} */ (props[key]);
if (value === undefined) return get_fallback();
fallback_dirty = true;
return value;
}
: () => {
var value = /** @type {V} */ (props[key]);
if (value !== undefined) fallback_value = /** @type {V} */ (undefined);
return value === undefined ? fallback_value : value;
};
/** @type {() => V} */
var getter;
if (runes) {
getter = () => {
var value = /** @type {V} */ (props[key]);
if (value === undefined) return get_fallback();
fallback_dirty = true;
return value;
};
} else {
// Svelte 4 did not trigger updates when a primitive value was updated to the same value.
// Replicate that behavior through using a derived
var derived_getter = (immutable ? derived : derived_safe_equal)(
() => /** @type {V} */ (props[key])
);
derived_getter.f |= LEGACY_DERIVED_PROP;
getter = () => {
var value = get(derived_getter);
if (value !== undefined) fallback_value = /** @type {V} */ (undefined);
return value === undefined ? fallback_value : value;
};
}

// easy mode — prop is never written to
if ((flags & PROPS_IS_UPDATED) === 0) {
Expand Down
14 changes: 12 additions & 2 deletions packages/svelte/src/internal/client/runtime.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ import {
BRANCH_EFFECT,
STATE_SYMBOL,
BLOCK_EFFECT,
ROOT_EFFECT
ROOT_EFFECT,
LEGACY_DERIVED_PROP
} from './constants.js';
import { flush_tasks } from './dom/task.js';
import { add_owner } from './dev/ownership.js';
Expand Down Expand Up @@ -797,7 +798,16 @@ export function invalidate_inner_signals(fn) {
captured_signals = previous_captured_signals;
}
for (signal of captured) {
mutate(signal, null /* doesnt matter */);
// Go one level up because derived signals created as part of props in legacy mode
dummdidumm marked this conversation as resolved.
Show resolved Hide resolved
if ((signal.f & LEGACY_DERIVED_PROP) !== 0) {
for (const dep of /** @type {import('#client').Derived} */ (signal).deps || []) {
if ((dep.f & DERIVED) === 0) {
mutate(dep, null /* doesnt matter */);
}
}
} else {
mutate(signal, null /* doesnt matter */);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<script>
export let primitive;
export let object;
$: primitive && console.log('primitive');
$: object && console.log('object');
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { test } from '../../test';

export default test({
async test({ assert, logs, target }) {
assert.deepEqual(logs, ['primitive', 'object']);
await target.querySelector('button')?.click();
assert.deepEqual(logs, ['primitive', 'object', 'object']);
}
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<script>
import Nested from './Nested.svelte';

let value = { count: 1 };
</script>

<button on:click={() => value = { count: 1 }}>reassign</button>
<Nested primitive={value.count} object={value} />
4 changes: 2 additions & 2 deletions packages/svelte/tests/sourcemaps/samples/basic/_config.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { test } from '../../test';

export default test({
client: ['foo.bar.baz'],
server: ['foo.bar.baz']
client: ['bar.baz'],
server: ['bar.baz']
});