Skip to content

Conversation

@trueadm
Copy link
Contributor

@trueadm trueadm commented Dec 18, 2024

WIP

@changeset-bot
Copy link

changeset-bot bot commented Dec 18, 2024

🦋 Changeset detected

Latest commit: d2d1b20

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
svelte Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions
Copy link
Contributor

Playground

pnpm add https://pkg.pr.new/svelte@14753

@trueadm trueadm self-assigned this Dec 18, 2024
@trueadm
Copy link
Contributor Author

trueadm commented Jan 23, 2025

It occurred to me that rather than passing in the abort signal as an argument, we could instead capture it using a utility function const signal = getAbortSignal() that you can call inside reactions and then you can pass this to your promises and have the reaction clean it up for you. This also means we can treeshake out this logic if it's not used anywhere.

We should also supply a way of making it easy to compose in other contexts too:

const abort = withAbortSignal(() => {
  ///
});

@briancray
Copy link

briancray commented Jan 23, 2025

Besides allowing the async signature (YES PLEASE), how is this different than the following? (I've been doing this pattern myself)

$effect(() => {
  const aborter = new AbortController()
  fetch('/foo', { signal: aborter.signal })
  return () => aborter.abort()
})

@JonathonRP
Copy link

Just wanted to add this pattern for event handlers -

$effect(() => {
    const controller = new AbortController()

    window.addEventListener('resize', handleResize, {

        signal: controller.signal,

    })

    window.addEventListener('hashchange', handleHashChange, {

         signal: controller.signal,

     })

     window.addEventListener('storage', handleStorageChange, {

         signal: controller.signal,

      })



     return () => {

         // Calling `.abort()` removes ALL event listeners

        // associated with `controller.signal`. Gone!

        controller.abort()

     }
}

@trueadm
Copy link
Contributor Author

trueadm commented Jan 29, 2025

Besides allowing the async signature (YES PLEASE), how is this different than the following? (I've been doing this pattern myself)

$effect(() => {
  const aborter = new AbortController()
  fetch('/foo', { signal: aborter.signal })
  return () => aborter.abort()
})

We might want to use this signal in other places other than effects.

@epavanello
Copy link

What happens to variables that are not caught by the synchronous execution of $effect?
Currently, without declaring dependencies explicitly (react style, or runed watch https://runed.dev/docs/utilities/watch), async code would make it less clear what is tracked by $effect and what is not.

@trueadm trueadm closed this Feb 27, 2025
@dummdidumm dummdidumm deleted the effect-abort branch February 27, 2025 10:58
@JonathonRP
Copy link

I'm curious why this is closed and if there is a good or better place to track async primitives?

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

Successfully merging this pull request may close these issues.

5 participants