partial merge

pull/15844/head
Rich Harris 2 months ago
parent c57e673c84
commit 6afb9c6028

@ -1,5 +0,0 @@
---
'svelte': patch
---
fix: re-evaluate derived props during teardown

@ -8,9 +8,17 @@ jobs:
trigger:
runs-on: ubuntu-latest
if: github.repository == 'sveltejs/svelte' && github.event.issue.pull_request && startsWith(github.event.comment.body, '/ecosystem-ci run')
permissions:
issues: write # to add / delete reactions
pull-requests: read # to read PR data
actions: read # to check workflow status
contents: read # to clone the repo
steps:
- uses: GitHubSecurityLab/actions-permissions/monitor@v1
- uses: actions/github-script@v6
- name: monitor action permissions
uses: GitHubSecurityLab/actions-permissions/monitor@v1
- name: check user authorization # user needs triage permission
uses: actions/github-script@v7
id: check-permissions
with:
script: |
const user = context.payload.sender.login
@ -29,7 +37,7 @@ jobs:
}
if (hasTriagePermission) {
console.log('Allowed')
console.log('User is allowed. Adding +1 reaction.')
await github.rest.reactions.createForIssueComment({
owner: context.repo.owner,
repo: context.repo.repo,
@ -37,16 +45,18 @@ jobs:
content: '+1',
})
} else {
console.log('Not allowed')
console.log('User is not allowed. Adding -1 reaction.')
await github.rest.reactions.createForIssueComment({
owner: context.repo.owner,
repo: context.repo.repo,
comment_id: context.payload.comment.id,
content: '-1',
})
throw new Error('not allowed')
throw new Error('User does not have the necessary permissions.')
}
- uses: actions/github-script@v6
- name: Get PR Data
uses: actions/github-script@v7
id: get-pr-data
with:
script: |
@ -59,21 +69,27 @@ jobs:
return {
num: context.issue.number,
branchName: pr.head.ref,
commit: pr.head.sha,
repo: pr.head.repo.full_name
}
- id: generate-token
uses: tibdex/github-app-token@b62528385c34dbc9f38e5f4225ac829252d1ea92 #keep pinned for security reasons, currently 1.8.0
- name: Generate Token
id: generate-token
uses: actions/create-github-app-token@v2
with:
app_id: ${{ secrets.ECOSYSTEM_CI_GITHUB_APP_ID }}
private_key: ${{ secrets.ECOSYSTEM_CI_GITHUB_APP_PRIVATE_KEY }}
repository: '${{ github.repository_owner }}/svelte-ecosystem-ci'
- uses: actions/github-script@v6
app-id: ${{ secrets.ECOSYSTEM_CI_GITHUB_APP_ID }}
private-key: ${{ secrets.ECOSYSTEM_CI_GITHUB_APP_PRIVATE_KEY }}
repositories: |
svelte
svelte-ecosystem-ci
- name: Trigger Downstream Workflow
uses: actions/github-script@v7
id: trigger
env:
COMMENT: ${{ github.event.comment.body }}
with:
github-token: ${{ steps.generate-token.outputs.token }}
result-encoding: string
script: |
const comment = process.env.COMMENT.trim()
const prData = ${{ steps.get-pr-data.outputs.result }}
@ -89,6 +105,7 @@ jobs:
prNumber: '' + prData.num,
branchName: prData.branchName,
repo: prData.repo,
commit: prData.commit,
suite: suite === '' ? '-' : suite
}
})

@ -1,5 +1,13 @@
# svelte
## 5.35.5
### Patch Changes
- fix: associate sources in Spring/Tween/SvelteMap/SvelteSet with correct reaction ([#16325](https://github.com/sveltejs/svelte/pull/16325))
- fix: re-evaluate derived props during teardown ([#16278](https://github.com/sveltejs/svelte/pull/16278))
## 5.35.4
### Patch Changes

@ -2,7 +2,7 @@
"name": "svelte",
"description": "Cybernetically enhanced web apps",
"license": "MIT",
"version": "5.35.4",
"version": "5.35.5",
"type": "module",
"types": "./types/index.d.ts",
"engines": {

@ -146,20 +146,15 @@ export function getAllContexts() {
* @returns {void}
*/
export function push(props, runes = false, fn) {
var ctx = (component_context = {
component_context = {
p: component_context,
c: null,
d: false,
e: null,
m: false,
s: props,
x: null,
l: legacy_mode_flag && !runes ? { s: null, u: null, $: [] } : null
});
teardown(() => {
/** @type {ComponentContext} */ (ctx).d = true;
});
};
if (DEV) {
// component function

@ -209,21 +209,14 @@ export function transition(flags, element, get_fn, get_params) {
var outro;
function get_options() {
var previous_reaction = active_reaction;
var previous_effect = active_effect;
set_active_reaction(null);
set_active_effect(null);
try {
return without_reactive_context(() => {
// If a transition is still ongoing, we use the existing options rather than generating
// new ones. This ensures that reversible transitions reverse smoothly, rather than
// jumping to a new spot because (for example) a different `duration` was used
return (current_options ??= get_fn()(element, get_params?.() ?? /** @type {P} */ ({}), {
direction
}));
} finally {
set_active_reaction(previous_reaction);
set_active_effect(previous_effect);
}
});
}
/** @type {TransitionManager} */

@ -8,7 +8,7 @@ import {
is_array,
object_prototype
} from '../shared/utils.js';
import { state as source, set } from './reactivity/sources.js';
import { state as source, set, increment } from './reactivity/sources.js';
import { PROXY_PATH_SYMBOL, STATE_SYMBOL } from '#client/constants';
import { UNINITIALIZED } from '../../constants.js';
import * as e from './errors.js';
@ -118,7 +118,7 @@ export function proxy(value) {
if (prop in target) {
const s = with_parent(() => source(UNINITIALIZED, stack));
sources.set(prop, s);
update_version(version);
increment(version);
if (DEV) {
tag(s, get_label(path, prop));
@ -136,7 +136,7 @@ export function proxy(value) {
}
}
set(s, UNINITIALIZED);
update_version(version);
increment(version);
}
return true;
@ -304,7 +304,7 @@ export function proxy(value) {
}
}
update_version(version);
increment(version);
}
return true;
@ -343,14 +343,6 @@ function get_label(path, prop) {
return /^\d+$/.test(prop) ? `${path}[${prop}]` : `${path}['${prop}']`;
}
/**
* @param {Source<number>} signal
* @param {1 | -1} [d]
*/
function update_version(signal, d = 1) {
set(signal, signal.v + d);
}
/**
* @param {any} value
*/

@ -268,14 +268,6 @@ export function spread_props(...props) {
return new Proxy({ props }, spread_props_handler);
}
/**
* @param {Derived} current_value
* @returns {boolean}
*/
function has_destroyed_component_ctx(current_value) {
return current_value.ctx?.d ?? false;
}
/**
* This function is responsible for synchronizing a possibly bound prop with the inner component state.
* It is used whenever the compiler sees that the component writes to the prop, or when it has a default prop_value.

@ -264,6 +264,14 @@ export function update_pre(source, d = 1) {
return set(source, d === 1 ? ++value : --value);
}
/**
* Silently (without using `get`) increment a source
* @param {Source<number>} source
*/
export function increment(source) {
set(source, source.v + 1);
}
/**
* @param {Value} signal
* @param {number} status should be DIRTY or MAYBE_DIRTY

@ -133,6 +133,8 @@ export let write_version = 1;
/** @type {number} Used to version each read of a source of derived to avoid duplicating depedencies inside a reaction */
let read_version = 0;
export let update_version = read_version;
// If we are working with a get() chain that has no active container,
// to prevent memory leaks, we skip adding the reaction.
export let skip_reaction = false;
@ -237,17 +239,17 @@ function schedule_possible_effect_self_invalidation(signal, effect, root = true)
var reactions = signal.reactions;
if (reactions === null) return;
if (
!async_mode_flag &&
source_ownership?.reaction === active_reaction &&
source_ownership.sources.includes(signal)
) {
return;
}
for (var i = 0; i < reactions.length; i++) {
var reaction = reactions[i];
if (
!async_mode_flag &&
source_ownership?.reaction === active_reaction &&
source_ownership.sources.includes(signal)
) {
continue;
}
if ((reaction.f & DERIVED) !== 0) {
schedule_possible_effect_self_invalidation(/** @type {Derived} */ (reaction), effect, false);
} else if (effect === reaction) {
@ -271,6 +273,7 @@ export function update_reaction(reaction) {
var previous_reaction_sources = source_ownership;
var previous_component_context = component_context;
var previous_untracking = untracking;
var previous_update_version = update_version;
var flags = reaction.f;
@ -284,7 +287,7 @@ export function update_reaction(reaction) {
source_ownership = null;
set_component_context(reaction.ctx);
untracking = false;
read_version++;
update_version = ++read_version;
if (reaction.ac !== null) {
reaction.ac.abort(STALE_REACTION);
@ -376,6 +379,7 @@ export function update_reaction(reaction) {
source_ownership = previous_reaction_sources;
set_component_context(previous_component_context);
untracking = previous_untracking;
update_version = previous_update_version;
}
}

@ -14,8 +14,6 @@ export type ComponentContext = {
p: null | ComponentContext;
/** context */
c: null | Map<unknown, unknown>;
/** destroyed */
d: boolean;
/** deferred effects */
e: null | Array<{
fn: () => void | (() => void);

@ -5,7 +5,7 @@ import { writable } from '../store/shared/index.js';
import { loop } from '../internal/client/loop.js';
import { raf } from '../internal/client/timing.js';
import { is_date } from './utils.js';
import { set, source } from '../internal/client/reactivity/sources.js';
import { set, state } from '../internal/client/reactivity/sources.js';
import { render_effect } from '../internal/client/reactivity/effects.js';
import { tag } from '../internal/client/dev/tracing.js';
import { get } from '../internal/client/runtime.js';
@ -170,9 +170,9 @@ export function spring(value, opts = {}) {
* @since 5.8.0
*/
export class Spring {
#stiffness = source(0.15);
#damping = source(0.8);
#precision = source(0.01);
#stiffness = state(0.15);
#damping = state(0.8);
#precision = state(0.01);
#current;
#target;
@ -194,8 +194,8 @@ export class Spring {
* @param {SpringOpts} [options]
*/
constructor(value, options = {}) {
this.#current = DEV ? tag(source(value), 'Spring.current') : source(value);
this.#target = DEV ? tag(source(value), 'Spring.target') : source(value);
this.#current = DEV ? tag(state(value), 'Spring.current') : state(value);
this.#target = DEV ? tag(state(value), 'Spring.target') : state(value);
if (typeof options.stiffness === 'number') this.#stiffness.v = clamp(options.stiffness, 0, 1);
if (typeof options.damping === 'number') this.#damping.v = clamp(options.damping, 0, 1);

@ -6,7 +6,7 @@ import { raf } from '../internal/client/timing.js';
import { loop } from '../internal/client/loop.js';
import { linear } from '../easing/index.js';
import { is_date } from './utils.js';
import { set, source } from '../internal/client/reactivity/sources.js';
import { set, state } from '../internal/client/reactivity/sources.js';
import { tag } from '../internal/client/dev/tracing.js';
import { get, render_effect } from 'svelte/internal/client';
import { DEV } from 'esm-env';
@ -191,8 +191,8 @@ export class Tween {
* @param {TweenedOptions<T>} options
*/
constructor(value, options = {}) {
this.#current = source(value);
this.#target = source(value);
this.#current = state(value);
this.#target = state(value);
this.#defaults = options;
if (DEV) {

@ -1,8 +1,7 @@
import { get, tick, untrack } from '../internal/client/runtime.js';
import { effect_tracking, render_effect } from '../internal/client/reactivity/effects.js';
import { source } from '../internal/client/reactivity/sources.js';
import { source, increment } from '../internal/client/reactivity/sources.js';
import { tag } from '../internal/client/dev/tracing.js';
import { increment } from './utils.js';
import { DEV } from 'esm-env';
/**

@ -1,9 +1,8 @@
/** @import { Source } from '#client' */
import { DEV } from 'esm-env';
import { set, source, state } from '../internal/client/reactivity/sources.js';
import { set, source, state, increment } from '../internal/client/reactivity/sources.js';
import { label, tag } from '../internal/client/dev/tracing.js';
import { get } from '../internal/client/runtime.js';
import { increment } from './utils.js';
import { get, update_version } from '../internal/client/runtime.js';
/**
* A reactive version of the built-in [`Map`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map) object.
@ -56,6 +55,7 @@ export class SvelteMap extends Map {
#sources = new Map();
#version = state(0);
#size = state(0);
#update_version = update_version || -1;
/**
* @param {Iterable<readonly [K, V]> | null | undefined} [value]
@ -79,6 +79,19 @@ export class SvelteMap extends Map {
}
}
/**
* If the source is being created inside the same reaction as the SvelteMap instance,
* we use `state` so that it will not be a dependency of the reaction. Otherwise we
* use `source` so it will be.
*
* @template T
* @param {T} value
* @returns {Source<T>}
*/
#source(value) {
return update_version === this.#update_version ? state(value) : source(value);
}
/** @param {K} key */
has(key) {
var sources = this.#sources;
@ -87,7 +100,7 @@ export class SvelteMap extends Map {
if (s === undefined) {
var ret = super.get(key);
if (ret !== undefined) {
s = source(0);
s = this.#source(0);
if (DEV) {
tag(s, `SvelteMap get(${label(key)})`);
@ -123,7 +136,7 @@ export class SvelteMap extends Map {
if (s === undefined) {
var ret = super.get(key);
if (ret !== undefined) {
s = source(0);
s = this.#source(0);
if (DEV) {
tag(s, `SvelteMap get(${label(key)})`);
@ -154,7 +167,7 @@ export class SvelteMap extends Map {
var version = this.#version;
if (s === undefined) {
s = source(0);
s = this.#source(0);
if (DEV) {
tag(s, `SvelteMap get(${label(key)})`);
@ -219,8 +232,7 @@ export class SvelteMap extends Map {
if (this.#size.v !== sources.size) {
for (var key of super.keys()) {
if (!sources.has(key)) {
var s = source(0);
var s = this.#source(0);
if (DEV) {
tag(s, `SvelteMap get(${label(key)})`);
}

@ -1,9 +1,8 @@
/** @import { Source } from '#client' */
import { DEV } from 'esm-env';
import { source, set, state } from '../internal/client/reactivity/sources.js';
import { source, set, state, increment } from '../internal/client/reactivity/sources.js';
import { label, tag } from '../internal/client/dev/tracing.js';
import { get } from '../internal/client/runtime.js';
import { increment } from './utils.js';
import { get, update_version } from '../internal/client/runtime.js';
var read_methods = ['forEach', 'isDisjointFrom', 'isSubsetOf', 'isSupersetOf'];
var set_like_methods = ['difference', 'intersection', 'symmetricDifference', 'union'];
@ -50,6 +49,7 @@ export class SvelteSet extends Set {
#sources = new Map();
#version = state(0);
#size = state(0);
#update_version = update_version || -1;
/**
* @param {Iterable<T> | null | undefined} [value]
@ -75,6 +75,19 @@ export class SvelteSet extends Set {
if (!inited) this.#init();
}
/**
* If the source is being created inside the same reaction as the SvelteSet instance,
* we use `state` so that it will not be a dependency of the reaction. Otherwise we
* use `source` so it will be.
*
* @template T
* @param {T} value
* @returns {Source<T>}
*/
#source(value) {
return update_version === this.#update_version ? state(value) : source(value);
}
// We init as part of the first instance so that we can treeshake this class
#init() {
inited = true;
@ -116,7 +129,7 @@ export class SvelteSet extends Set {
return false;
}
s = source(true);
s = this.#source(true);
if (DEV) {
tag(s, `SvelteSet has(${label(value)})`);

@ -1,9 +1,8 @@
import { DEV } from 'esm-env';
import { state } from '../internal/client/reactivity/sources.js';
import { state, increment } from '../internal/client/reactivity/sources.js';
import { tag } from '../internal/client/dev/tracing.js';
import { get } from '../internal/client/runtime.js';
import { get_current_url } from './url.js';
import { increment } from './utils.js';
export const REPLACE = Symbol();

@ -1,7 +0,0 @@
/** @import { Source } from '#client' */
import { set } from '../internal/client/reactivity/sources.js';
/** @param {Source<number>} source */
export function increment(source) {
set(source, source.v + 1);
}

@ -4,5 +4,5 @@
* The current version, as set in package.json.
* @type {string}
*/
export const VERSION = '5.35.4';
export const VERSION = '5.35.5';
export const PUBLIC_VERSION = '5';

@ -8,7 +8,8 @@ export default test({
},
test({ assert, target }) {
const [button1, button2] = target.querySelectorAll('button');
const [button1, button2, button3, button4, button5, button6, button7, button8] =
target.querySelectorAll('button');
assert.throws(() => {
button1?.click();
@ -19,5 +20,35 @@ export default test({
button2?.click();
flushSync();
});
assert.throws(() => {
button3?.click();
flushSync();
}, /state_unsafe_mutation/);
assert.doesNotThrow(() => {
button4?.click();
flushSync();
});
assert.throws(() => {
button5?.click();
flushSync();
}, /state_unsafe_mutation/);
assert.doesNotThrow(() => {
button6?.click();
flushSync();
});
assert.throws(() => {
button7?.click();
flushSync();
}, /state_unsafe_mutation/);
assert.doesNotThrow(() => {
button8?.click();
flushSync();
});
}
});

@ -1,27 +1,101 @@
<script>
import { SvelteMap } from 'svelte/reactivity';
let visibleExternal = $state(false);
let external = new SvelteMap();
const throws = $derived.by(() => {
external.set(1, 1);
return external;
let outside_basic = $state(false);
let outside_basic_map = new SvelteMap();
const throw_basic = $derived.by(() => {
outside_basic_map.set(1, 1);
return outside_basic_map;
});
let visibleInternal = $state(false);
const works = $derived.by(() => {
let internal = new SvelteMap();
internal.set(1, 1);
return internal;
let inside_basic = $state(false);
const works_basic = $derived.by(() => {
let inside = new SvelteMap();
inside.set(1, 1);
return inside;
});
let outside_has = $state(false);
let outside_has_map = new SvelteMap([[1, 1]]);
const throw_has = $derived.by(() => {
outside_has_map.has(1);
outside_has_map.set(1, 2);
return outside_has_map;
});
let inside_has = $state(false);
const works_has = $derived.by(() => {
let inside = new SvelteMap([[1, 1]]);
inside.has(1);
inside.set(1, 1);
return inside;
});
let outside_get = $state(false);
let outside_get_map = new SvelteMap([[1, 1]]);
const throw_get = $derived.by(() => {
outside_get_map.get(1);
outside_get_map.set(1, 2);
return outside_get_map;
});
let inside_get = $state(false);
const works_get = $derived.by(() => {
let inside = new SvelteMap([[1, 1]]);
inside.get(1);
inside.set(1, 1);
return inside;
});
let outside_values = $state(false);
let outside_values_map = new SvelteMap([[1, 1]]);
const throw_values = $derived.by(() => {
outside_values_map.values(1);
outside_values_map.set(1, 2);
return outside_values_map;
});
let inside_values = $state(false);
const works_values = $derived.by(() => {
let inside = new SvelteMap([[1, 1]]);
inside.values();
inside.set(1, 1);
return inside;
});
</script>
<button onclick={() => (visibleExternal = true)}>external</button>
{#if visibleExternal}
{throws}
<button onclick={() => (outside_basic = true)}>external</button>
{#if outside_basic}
{throw_basic}
{/if}
<button onclick={() => (inside_basic = true)}>internal</button>
{#if inside_basic}
{works_basic}
{/if}
<button onclick={() => (outside_has = true)}>external</button>
{#if outside_has}
{throw_has}
{/if}
<button onclick={() => (visibleInternal = true)}>internal</button>
{#if visibleInternal}
{works}
<button onclick={() => (inside_has = true)}>internal</button>
{#if inside_has}
{works_has}
{/if}
<button onclick={() => (outside_get = true)}>external</button>
{#if outside_get}
{throw_get}
{/if}
<button onclick={() => (inside_get = true)}>internal</button>
{#if inside_get}
{works_get}
{/if}
<button onclick={() => (outside_values = true)}>external</button>
{#if outside_values}
{throw_values}
{/if}
<button onclick={() => (inside_values = true)}>internal</button>
{#if inside_values}
{works_values}
{/if}

@ -8,7 +8,7 @@ export default test({
},
test({ assert, target }) {
const [button1, button2] = target.querySelectorAll('button');
const [button1, button2, button3, button4] = target.querySelectorAll('button');
assert.throws(() => {
button1?.click();
@ -19,5 +19,15 @@ export default test({
button2?.click();
flushSync();
});
assert.throws(() => {
button3?.click();
flushSync();
}, /state_unsafe_mutation/);
assert.doesNotThrow(() => {
button4?.click();
flushSync();
});
}
});

@ -1,27 +1,52 @@
<script>
import { SvelteSet } from 'svelte/reactivity';
let visibleExternal = $state(false);
let external = new SvelteSet();
const throws = $derived.by(() => {
external.add(1);
return external;
let outside_basic = $state(false);
let outside_basic_set = new SvelteSet();
const throws_basic = $derived.by(() => {
outside_basic_set.add(1);
return outside_basic_set;
})
let visibleInternal = $state(false);
const works = $derived.by(() => {
let inside_basic = $state(false);
const works_basic = $derived.by(() => {
let internal = new SvelteSet();
internal.add(1);
return internal;
})
let outside_has_delete = $state(false);
let outside_has_delete_set = new SvelteSet([1]);
const throws_has_delete = $derived.by(() => {
outside_has_delete_set.has(1);
outside_has_delete_set.delete(1);
return outside_has_delete_set;
})
let inside_has_delete = $state(false);
const works_has_delete = $derived.by(() => {
let internal = new SvelteSet([1]);
internal.has(1);
internal.delete(1);
return internal;
})
</script>
<button onclick={() => (visibleExternal = true)}>external</button>
{#if visibleExternal}
{throws}
<button onclick={() => (outside_basic = true)}>external</button>
{#if outside_basic}
{throws_basic}
{/if}
<button onclick={() => (inside_basic = true)}>internal</button>
{#if inside_basic}
{works_basic}
{/if}
<button onclick={() => (outside_has_delete = true)}>external</button>
{#if outside_has_delete}
{throws_has_delete}
{/if}
<button onclick={() => (visibleInternal = true)}>internal</button>
{#if visibleInternal}
{works}
<button onclick={() => (inside_has_delete = true)}>internal</button>
{#if inside_has_delete}
{works_has_delete}
{/if}

@ -0,0 +1,23 @@
import { flushSync } from 'svelte';
import { test } from '../../test';
export default test({
compileOptions: {
dev: true,
runes: true
},
test({ assert, target }) {
const [button1, button2] = target.querySelectorAll('button');
assert.throws(() => {
button1?.click();
flushSync();
}, /state_unsafe_mutation/);
assert.doesNotThrow(() => {
button2?.click();
flushSync();
});
}
});

@ -0,0 +1,26 @@
<script>
import { Spring } from 'svelte/motion';
let outside_basic = $state(false);
let outside_basic_spring = new Spring(0);
const throws_basic = $derived.by(() => {
outside_basic_spring.set(1);
return outside_basic_spring;
})
let inside_basic = $state(false);
const works_basic = $derived.by(() => {
let internal = new Spring(0);
internal.set(1);
return internal;
})
</script>
<button onclick={() => (outside_basic = true)}>external</button>
{#if outside_basic}
{throws_basic}
{/if}
<button onclick={() => (inside_basic = true)}>internal</button>
{#if inside_basic}
{works_basic}
{/if}

@ -0,0 +1,23 @@
import { flushSync } from 'svelte';
import { test } from '../../test';
export default test({
compileOptions: {
dev: true,
runes: true
},
test({ assert, target }) {
const [button1, button2] = target.querySelectorAll('button');
assert.throws(() => {
button1?.click();
flushSync();
}, /state_unsafe_mutation/);
assert.doesNotThrow(() => {
button2?.click();
flushSync();
});
}
});

@ -0,0 +1,26 @@
<script>
import { Tween } from 'svelte/motion';
let outside_basic = $state(false);
let outside_basic_tween = new Tween(0);
const throws_basic = $derived.by(() => {
outside_basic_tween.set(1);
return outside_basic_tween;
})
let inside_basic = $state(false);
const works_basic = $derived.by(() => {
let internal = new Tween(0);
internal.set(1);
return internal;
})
</script>
<button onclick={() => (outside_basic = true)}>external</button>
{#if outside_basic}
{throws_basic}
{/if}
<button onclick={() => (inside_basic = true)}>internal</button>
{#if inside_basic}
{works_basic}
{/if}
Loading…
Cancel
Save