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

Svelte 5: Type problem with async function in $effect rune #9520

Closed
njezersek opened this issue Nov 17, 2023 · 11 comments
Closed

Svelte 5: Type problem with async function in $effect rune #9520

njezersek opened this issue Nov 17, 2023 · 11 comments

Comments

@njezersek
Copy link

njezersek commented Nov 17, 2023

Describe the problem

When using the $effect rune with async function, TypeScript raises an error:

Argument of type '() => Promise<void>' is not assignable to parameter of type '() => void | (() => void)'.
  Type 'Promise<void>' is not assignable to type 'void | (() => void)'.

However Svelte seems to work just fine with async function in `$effect$.

Here is a small example to demonstrate the problem:

<script lang="ts">
	let key = $state("test");
	$effect(async () => {
		let res = await fetch(`http://localhost:8080/api/${key}`);
		let json = await res.json();
		console.log(key, json);
	})
</script>

<input type="text" bind:value={key}>

Describe the proposed solution

Update the type definition to include async functions.

From:

declare function $effect(fn: () => void | (() => void)): void;

to:

declare function $effect(fn: () => void | (() => void) | Promise<void>): void;

Note: I realize that using an async function with $effect might make returning callback hard to handle. Thats why I propose updating the type declaration to only alow void async functions.

Alternatives considered

A possible workaround is to introduce another function:

<script lang="ts">
	let key = $state("test");
	$effect(() => {
		(async () => {
			let res = await fetch(`http://localhost:8080/api/${key}`);
			let json = await res.json();
			console.log(key, json);
		})()
	})
</script>

<input type="text" bind:value={key}>

Importance

would make my life easier

@Conduitry
Copy link
Member

This is functioning as intended. $effect() blocks indeed do not work correctly with async functions - see https://svelte-5-preview.vercel.app/#H4sIAAAAAAAACkWQwW7DIBBEfwWhHIhaJemV2pb6B73HPVCyVKh4sWBJFSH-PYCV9IR2Zt8wkLmxDiKX58xRLcAl_1hX_srptrYhXsER1Dn6FHRThqiDXWmacSYHxLRPSGxku0iKQJz279WpnkmoyXpkFnWABZDEnuXmzLQxLyN7a8tUNmIHxoAmoeINNavb4_QA1J-yxD6DX2yEQ4Do3RVEv6rHYRXg4PyP6NGbUdoxHP_74vCdiGolj1I7q3_H_CxX-oO6GiXLPaY0fEOm-gWLv1hj4cIlhQTlq9wB9puwTz0BAAA=

Any references to variables that happen asynchronously inside the function will not correctly be picked up as dependencies that should re-run the effect. There's nothing that we can do about this currently without additional language features in JS for tracking execution context asynchronously, which is currently impossible.

We can't reasonably tell whether there are any references to variables before vs after the part of the function that executes synchronously, so we prevent code like this. If you really know what you are doing, you can suppress the TS warning or you can wrap in another function, as in your workaround.

@Conduitry Conduitry closed this as not planned Won't fix, can't repro, duplicate, stale Nov 17, 2023
@manzt
Copy link

manzt commented Feb 16, 2024

$effect() blocks indeed do not work correctly with async functions

Makes sense but I'm curious if there is pattern for explicitly declaring deps of an async effect. Probably hoisting the behavior to an async function and calling that?

let href = $state("https://example.com");
let content = $state("");
$effect(() => {
 let current = href; // need to reference here so it is tracked?
 async function fetchData() {
    let response = await fetch(href);
    let data = await response.text();
    content = data;  
  }
  fetchData();
})

Like lifting fetchData to avoid the closure:

let href = $state("https://example.com");
let content = $state("");

async function fetchData(href) {
    let data = await response.text();
    content = data;  
}
$effect(() => {
  fetchData(href)
})

Is this the recommended way?

@niemyjski
Copy link

niemyjski commented Apr 30, 2024

I find myself in this same boat...

I was loading data when Readables change. Now those are state runes and the docs say use effect which runs when any state changes I don't know how the compiler would ever know my intent when only some of the runes change but not all,,, because what if I only want to react to some of my runes? What's the best way to handle this scenario as use effect doesn't support async and this pattern will be in every app (e.g., react to parameter change and go fetch my data).

// There are many other parameters.
    parameters.subscribe(async () => await loadData());
    filter.subscribe(async () => await loadData());
    time.subscribe(async () => await loadData());

How would it also know that I have some parameters in the refactor coming from a svelte.ts file and not defined in the component? I don't want to add log statements to components just to make it get picked up...

@niemyjski
Copy link

niemyjski commented May 3, 2024

@Rich-Harris I'm seeing a trend in SPA mode development as well as on the discord channel of lots of questions and issues around dealing with runes / $effect and async. Is there a general recommended approach that you'd recommend? I feel like this is a super common scenario and not everyone loads their data or async work in a page.ts esp for SPA.

@khromov
Copy link
Contributor

khromov commented May 9, 2024

@niemyjski Even if you are building a SPA the recommendation is to use SvelteKit which provide load functions for loading async data:
https://kit.svelte.dev/docs/single-page-apps

@niemyjski
Copy link

There are a ton of scenarios where I don't know what data to load until runtime, what about posting form data etc..

@khromov
Copy link
Contributor

khromov commented May 9, 2024

You asked for a "general recommended" approach and SvelteKit is that. SPA is in fact in a heavy downwards trend - React is moving away from it with RSC, other meta frameworks like Remix and SvelteKit are very server focused. There are use cases for SPA, and SvelteKit supports it fully. You can post form data from a SPA but naturally you can't submit an actual <form> because that would require a serverful application, but there are workarounds like customizing use:enhance to send form data to a JSON API for example.

@niemyjski
Copy link

niemyjski commented May 9, 2024 via email

@ejsmith
Copy link

ejsmith commented May 9, 2024

You asked for a "general recommended" approach and SvelteKit is that. SPA is in fact in a heavy downwards trend - React is moving away from it with RSC, other meta frameworks like Remix and SvelteKit are very server focused. There are use cases for SPA, and SvelteKit supports it fully. You can post form data from a SPA but naturally you can't submit an actual <form> because that would require a serverful application, but there are workarounds like customizing use:enhance to send form data to a JSON API for example.

SPA is a downward trend because it was overused for normal marketing type sites and SSR is the right choice for those sites, but it is not the right choice for business apps with backends in non-JavaScript technology.

@lolmaus
Copy link

lolmaus commented Jun 10, 2024

My intented attempt was like this:

let foo = $state();
let bar = $state();

$effect(async () => {
  const baz = await someAsyncStuff();

  // Svelte compiler cannot "see" below `await`, so the usage is not triggered.
  baz.use(foo, bar);
})

My workaround:

let foo = $state();
let bar = $state();

$effect(async () => {
  // Above the async part, obtain local references to necessary $state vars.
  // This lets the Svelte compiler know these are used for this $effect.
  const sync = { foo, bar };
  
  // Use the good old `.then` instead of `await` to keep TypeScript happy.
  someAsyncStuff().then(baz => {
    // Use local references to $state vars
    baz.use(sync.foo, sync.bar);
  });
});

@Rich-Harris Please consider adding this to docs! 🙏

@bdmackie
Copy link

bdmackie commented Jun 24, 2024

Writing new code and have an 'auto update entity name' use case:

  1. Page level declares name via $state and $effect to detect changes.
let name = $state(document.name)
  1. Binds to name property on svelte components 2 levels deep that displays and edits the name.
<DocumentName bind:name />
  1. using superForms for the input in that bottom level component. Using a dedicated superforms instance for encapsulation and set to SPA mode to avoid knowledge of page-level forms. Valid updates set the bindable name prop.

  2. back up at the page level on the name-change detected by $effect, i'm calling fetch to do the update.

$effect(() => {
    // Auto-save name edits.
    if (!$state.is(document.name, name)) {
      console.log('name: ' + name)
      void fetch(
       // fetch args here...
      ).then(({ updated }) => {
        if (updated > 0) {
          document.name = name
          console.log('Document name updated: ' + name)
        } else {
          console.log('Document name NOT updated!')
        }
      })
    }
  })

Similar to the OP I got confused as to the recommended path at this point.

So based on my limited understanding of Svelte 5 thus far, I believe I have two options architecturally:

  1. Use the async code in $effect and treat it as a 'gotcha' which I think validates the OP's point (or doc update request), but expands it to say it's more common than "an SPA requirement".
  2. Tighten the connection between page-level forms and nested components that update data, avoiding the need for $state, $bindable and $effect altogether.

Option 2 seems to agree with "SPA is on a downtrend" but it feels like it's also saying "$state is on a downtrend", at least for this use case. Assuming the point of runes is a happy medium between client and server centricity, it'd be great to hear what approach is recommended, or if it's known runes has some dissonance baked in here.


UPDATE: For any other lost souls who find themselves here, my take on this having travelled a little farther:

While it's easy to forgive a newcomer to see the $state rune as a powerful encapsulation of both data and events, bubbling up to $effect has limitations and if you are hitting this use case, you are probably better following another maintainer recommendation and wrap state in a helper class/closure that also handles change actions. The context API can be used to reduce prop drilling across parent/child hierarchies if applicable. The result has a bit more boilerplate but not a lot, and leaves the codebase in a more extensible state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants