From 118e28445df1e8ec3480b2825ab886d283167dab Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 23 Jan 2025 15:04:52 -0500 Subject: [PATCH] experimental waterfall detection --- .../.generated/client-warnings.md | 8 ++++ .../messages/client-warnings/warnings.md | 6 +++ .../internal/client/reactivity/deriveds.js | 45 ++++++++++++++++--- .../svelte/src/internal/client/runtime.js | 1 + .../svelte/src/internal/client/warnings.js | 11 +++++ 5 files changed, 66 insertions(+), 5 deletions(-) diff --git a/documentation/docs/98-reference/.generated/client-warnings.md b/documentation/docs/98-reference/.generated/client-warnings.md index 284e9a7c3e..dcce04bcb8 100644 --- a/documentation/docs/98-reference/.generated/client-warnings.md +++ b/documentation/docs/98-reference/.generated/client-warnings.md @@ -34,6 +34,14 @@ function add() { } ``` +### await_waterfall + +``` +Detected an unnecessary async waterfall +``` + +TODO + ### binding_property_non_reactive ``` diff --git a/packages/svelte/messages/client-warnings/warnings.md b/packages/svelte/messages/client-warnings/warnings.md index 943cf6f01f..cb0645367b 100644 --- a/packages/svelte/messages/client-warnings/warnings.md +++ b/packages/svelte/messages/client-warnings/warnings.md @@ -30,6 +30,12 @@ function add() { } ``` +## await_waterfall + +> Detected an unnecessary async waterfall + +TODO + ## binding_property_non_reactive > `%binding%` is binding to a non-reactive property diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index f211903ab6..66e98b0666 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -23,6 +23,7 @@ import { } from '../runtime.js'; import { equals, safe_equals } from './equality.js'; import * as e from '../errors.js'; +import * as w from '../warnings.js'; import { block, destroy_effect } from './effects.js'; import { inspect_effects, internal_set, set_inspect_effects, source } from './sources.js'; import { get_stack } from '../dev/tracing.js'; @@ -77,6 +78,9 @@ export function derived(fn) { return signal; } +// Used for waterfall detection +var async_deps = new Set(); + /** * @template V * @param {() => Promise} fn @@ -84,16 +88,18 @@ export function derived(fn) { */ /*#__NO_SIDE_EFFECTS__*/ export function async_derived(fn) { - let effect = /** @type {Effect | null} */ (active_effect); + let parent = /** @type {Effect | null} */ (active_effect); - if (effect === null) { + if (parent === null) { throw new Error('TODO cannot create unowned async derived'); } var promise = /** @type {Promise} */ (/** @type {unknown} */ (undefined)); var value = source(/** @type {V} */ (undefined)); - block(async () => { + var current_deps = new Set(async_deps); + + var effect = block(async () => { var current = (promise = fn()); var restore = capture(); @@ -102,16 +108,45 @@ export function async_derived(fn) { try { var v = await promise; - if ((effect.f & DESTROYED) !== 0) { + // check to see if we just created an unnecessary waterfall + if (current_deps.size > 0) { + var justified = false; + + if (effect.deps !== null) { + for (const dep of effect.deps) { + if (current_deps.has(dep)) { + justified = true; + break; + } + } + } + + if (!justified) { + w.await_waterfall(); + } + } + + if ((parent.f & DESTROYED) !== 0) { return; } if (promise === current) { restore(); internal_set(value, v); + + // make a note that we're updating this derived, + // so that we can detect waterfalls + async_deps.add(value); + + // TODO we want to clear this after we've updated effects. + // `queue_post_micro_task` appears to run too early. + // for now, as a POC, use setTimeout + setTimeout(() => { + async_deps.delete(value); + }); } } catch (e) { - handle_error(e, effect, null, effect.ctx); + handle_error(e, parent, null, parent.ctx); } finally { unsuspend(); diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index c937f7fb9c..9ed1731522 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -99,6 +99,7 @@ export function set_active_effect(effect) { } // TODO remove this, once we're satisfied that we're not leaking context +/* @__PURE__ */ setInterval(() => { if (active_effect !== null || active_reaction !== null) { debugger; diff --git a/packages/svelte/src/internal/client/warnings.js b/packages/svelte/src/internal/client/warnings.js index 250c6eca2f..f4dcfdd650 100644 --- a/packages/svelte/src/internal/client/warnings.js +++ b/packages/svelte/src/internal/client/warnings.js @@ -18,6 +18,17 @@ export function assignment_value_stale(property, location) { } } +/** + * Detected an unnecessary async waterfall + */ +export function await_waterfall() { + if (DEV) { + console.warn(`%c[svelte] await_waterfall\n%cDetected an unnecessary async waterfall\nhttps://svelte.dev/e/await_waterfall`, bold, normal); + } else { + console.warn(`https://svelte.dev/e/await_waterfall`); + } +} + /** * `%binding%` (%location%) is binding to a non-reactive property * @param {string} binding