manzt / anywidget

reusable widgets made easy
https://anywidget.dev
MIT License
490 stars 39 forks source link

feat(experimental): Replace invoke timeout with `AbortSignal` #540

Closed manzt closed 6 months ago

manzt commented 6 months ago

This allows more flexible control over aborting the invoke request, including delegating to third-party libraries that manage cancellation.

export default {
  async render({ model, el }) {
    const controller = new AbortController();

    // Randomly abort the request after 1 second
    setTimeout(() => Math.random() < 0.5 && controller.abort(), 1000);

    const signal = controller.signal;
    model
      .invoke("echo", "Hello, world", { signal })
      .then((result) => {
        el.innerHTML = result;
      })
      .catch((err) => {
        el.innerHTML = `Error: ${err.message}`;
      });
  },
};
changeset-bot[bot] commented 6 months ago

🦋 Changeset detected

Latest commit: f0aa55e14126698a8271d2094cc193d75c6c39d7

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

This PR includes changesets to release 4 packages | Name | Type | | ----------------- | ----- | | anywidget | Patch | | @anywidget/types | Patch | | @anywidget/react | Patch | | @anywidget/svelte | Patch |

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

manzt commented 6 months ago

cc: @keller-mark @kylebarron

This means you could get pass down the signal from Deck.gl tile fetcher to abort tile requests.

kylebarron commented 6 months ago

😍

I think it might be worth having both a timeout and a signal. Like set timeout to 10 seconds but stop sooner if the signal was aborted?

manzt commented 6 months ago

I think it might be worth having both a timeout and a signal. Like set timeout to 10 seconds but stop sooner if the signal was aborted?

I guess I'd defer this to the caller with AbortSignal.any. What do you think?


let res = await invoke("_get_tile", tileId, {
  signal: AbortSignal.any([deckSignal, AbortSignal.timeout(10_000)]),
});

I guess we could do this internally to have some final fallback.... but deciding on what the timeout should be generally is challenging.

kylebarron commented 6 months ago

I guess I'd defer this to the caller with AbortSignal.any. What do you think?

Oh that's cool! I didn't know that existed. That seems fine!