make effects depend on state created inside them

pull/16198/head
Simon Holthausen 3 months ago
parent d137ecafc3
commit 45ecae4ea0

@ -57,7 +57,6 @@ import {
import * as w from './warnings.js';
import { current_batch, Batch, batch_deriveds } from './reactivity/batch.js';
import { handle_error, invoke_error_boundary } from './error-handling.js';
import { snapshot } from '../shared/clone.js';
/** @type {Effect | null} */
let last_scheduled_effect = null;
@ -259,7 +258,12 @@ function schedule_possible_effect_self_invalidation(signal, effect, root = true)
for (var i = 0; i < reactions.length; i++) {
var reaction = reactions[i];
if (reaction_sources?.[1].includes(signal) && reaction_sources[0] === active_reaction) continue;
if (
!async_mode_flag &&
reaction_sources?.[1].includes(signal) &&
reaction_sources[0] === active_reaction
)
continue;
if ((reaction.f & DERIVED) !== 0) {
schedule_possible_effect_self_invalidation(/** @type {Derived} */ (reaction), effect, false);
@ -299,7 +303,9 @@ export function update_reaction(reaction) {
untracking = false;
read_version++;
reaction.f |= EFFECT_IS_UPDATING;
if (!async_mode_flag || (reaction.f & DERIVED) !== 0) {
reaction.f |= EFFECT_IS_UPDATING;
}
if (reaction.ac !== null) {
reaction.ac?.abort(STALE_REACTION);
@ -383,7 +389,9 @@ export function update_reaction(reaction) {
set_component_context(previous_component_context);
untracking = previous_untracking;
reaction.f ^= EFFECT_IS_UPDATING;
if (!async_mode_flag || (reaction.f & DERIVED) !== 0) {
reaction.f ^= EFFECT_IS_UPDATING;
}
}
}
@ -776,7 +784,9 @@ export function get(signal) {
if (
!destroyed &&
(!reaction_sources?.[1].includes(signal) || reaction_sources[0] !== active_reaction)
((async_mode_flag && (active_reaction.f & DERIVED) === 0) ||
!reaction_sources?.[1].includes(signal) ||
reaction_sources[0] !== active_reaction)
) {
var deps = active_reaction.deps;

@ -6,6 +6,11 @@ export function enable_async_mode_flag() {
async_mode_flag = true;
}
/** ONLY USE THIS DURING TESTING */
export function disable_async_mode_flag() {
async_mode_flag = false;
}
export function enable_legacy_mode_flag() {
legacy_mode_flag = true;
}

@ -194,6 +194,8 @@ if (typeof window !== 'undefined') {
export const fragments = /** @type {'html' | 'tree'} */ (process.env.FRAGMENTS) ?? 'html';
export const async_mode = false; // process.env.SVELTE_NO_ASYNC !== 'true';
/**
* @param {any[]} logs
*/

@ -3,10 +3,10 @@ import { setImmediate } from 'node:timers/promises';
import { globSync } from 'tinyglobby';
import { createClassComponent } from 'svelte/legacy';
import { proxy } from 'svelte/internal/client';
import { flushSync, hydrate, mount, unmount, untrack } from 'svelte';
import { flushSync, hydrate, mount, unmount } from 'svelte';
import { render } from 'svelte/server';
import { afterAll, assert, beforeAll } from 'vitest';
import { compile_directory, fragments } from '../helpers.js';
import { async_mode, compile_directory, fragments } from '../helpers.js';
import { assert_html_equal, assert_html_equal_with_options } from '../html_equal.js';
import { raf } from '../animation-helpers.js';
import type { CompileOptions } from '#compiler';
@ -45,6 +45,8 @@ export interface RuntimeTest<Props extends Record<string, any> = Record<string,
mode?: Array<'server' | 'client' | 'hydrate'>;
/** Temporarily skip specific modes, without skipping the entire test */
skip_mode?: Array<'server' | 'client' | 'hydrate'>;
/** Skip if running with process.env.NO_ASYNC */
skip_no_async?: boolean;
html?: string;
ssrHtml?: string;
compileOptions?: Partial<CompileOptions>;
@ -121,7 +123,11 @@ let console_error = console.error;
export function runtime_suite(runes: boolean) {
return suite_with_variants<RuntimeTest, 'hydrate' | 'ssr' | 'dom', CompileOptions>(
['dom', 'hydrate', 'ssr'],
(variant, config) => {
(variant, config, test_name) => {
if (!async_mode && (config.skip_no_async || test_name.startsWith('async-'))) {
return true;
}
if (variant === 'hydrate') {
if (config.mode && !config.mode.includes('hydrate')) return 'no-test';
if (config.skip_mode?.includes('hydrate')) return true;
@ -169,7 +175,7 @@ async function common_setup(cwd: string, runes: boolean | undefined, config: Run
dev: force_hmr ? true : undefined,
hmr: force_hmr ? true : undefined,
experimental: {
async: runes
async: runes && async_mode
},
fragments,
...config.compileOptions,

@ -1,3 +1,4 @@
import { async_mode } from '../../../helpers';
import { test } from '../../test';
import { flushSync } from 'svelte';
@ -10,6 +11,12 @@ export default test({
flushSync(() => {
b1.click();
});
assert.deepEqual(logs, ['init 0']);
// With async mode (which is on by default for runtime-runes) this works as expected, without it
// it works differently: https://github.com/sveltejs/svelte/pull/15564
assert.deepEqual(
logs,
async_mode ? ['init 0', 'cleanup 2', null, 'init 2', 'cleanup 4', null, 'init 4'] : ['init 0']
);
}
});

@ -14,4 +14,4 @@
})
</script>
<button on:click={() => count++ }>Click</button>
<button onclick={() => count++ }>Click</button>

@ -1,6 +1,7 @@
import { test } from '../../test';
export default test({
skip_no_async: true,
async test({ assert, logs }) {
await Promise.resolve();
await Promise.resolve();

@ -1,11 +1,16 @@
import { flushSync } from 'svelte';
import { test } from '../../test';
import { async_mode } from '../../../helpers';
export default test({
async test({ target, assert, logs }) {
const button = target.querySelector('button');
flushSync(() => button?.click());
assert.ok(logs[0].startsWith('set_context_after_init'));
assert.ok(
async_mode
? logs[0].startsWith('set_context_after_init')
: logs[0] === 'works without experimental async but really shouldnt'
);
}
});

@ -7,6 +7,7 @@
if (condition) {
try {
setContext('potato', {});
console.log('works without experimental async but really shouldnt')
} catch (e) {
console.log(e.message);
}

@ -2,6 +2,10 @@ import { flushSync } from 'svelte';
import { test } from '../../test';
export default test({
// In async mode we _do_ want to run effects that react to their own state changing, so we need to disable async mode here
compileOptions: {
experimental: { async: false }
},
test({ assert, target, logs }) {
const button = target.querySelector('button');

@ -16,6 +16,7 @@ import { snapshot } from '../../src/internal/shared/clone.js';
import { SvelteSet } from '../../src/reactivity/set';
import { DESTROYED } from '../../src/internal/client/constants';
import { noop } from 'svelte/internal/client';
import { disable_async_mode_flag, enable_async_mode_flag } from '../../src/internal/flags';
/**
* @param runes runes mode
@ -1010,14 +1011,39 @@ describe('signals', () => {
};
});
test('effects do not depend on state they own', () => {
user_effect(() => {
const value = state(0);
set(value, $.get(value) + 1);
test('effects do depend on state they own', (runes) => {
// This behavior is important for use cases like a Resource class
// which shares its instance between multiple effects and triggers
// rerenders by self-invalidating its state.
const log: number[] = [];
let count: any;
if (runes) {
// We will make this the new default behavior once it's stable but until then
// we need to keep the old behavior to not break existing code.
enable_async_mode_flag();
}
effect(() => {
if (!count || $.get<number>(count) < 2) {
count ||= state(0);
log.push($.get(count));
set(count, $.get<number>(count) + 1);
}
});
return () => {
flushSync();
try {
flushSync();
if (runes) {
assert.deepEqual(log, [0, 1]);
} else {
assert.deepEqual(log, [0]);
}
} finally {
disable_async_mode_flag();
}
};
});

@ -35,7 +35,7 @@ export function suite<Test extends BaseTest>(fn: (config: Test, test_dir: string
export function suite_with_variants<Test extends BaseTest, Variants extends string, Common>(
variants: Variants[],
should_skip_variant: (variant: Variants, config: Test) => boolean | 'no-test',
should_skip_variant: (variant: Variants, config: Test, test_name: string) => boolean | 'no-test',
common_setup: (config: Test, test_dir: string) => Promise<Common> | Common,
fn: (config: Test, test_dir: string, variant: Variants, common: Common) => void
) {
@ -46,11 +46,11 @@ export function suite_with_variants<Test extends BaseTest, Variants extends stri
let called_common = false;
let common: any = undefined;
for (const variant of variants) {
if (should_skip_variant(variant, config) === 'no-test') {
if (should_skip_variant(variant, config, dir) === 'no-test') {
continue;
}
// TODO unify test interfaces
const skip = config.skip || should_skip_variant(variant, config);
const skip = config.skip || should_skip_variant(variant, config, dir);
const solo = config.solo;
let it_fn = skip ? it.skip : solo ? it.only : it;

Loading…
Cancel
Save