⚠️ No Changeset found

Latest commit: 637446e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

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

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 637446e:

I really like this idea, but I think it's going to be very easy to forget to use step() somewhere, and that would have really unpleasant consequences when trying to resume a function. TypeScript can't help either, as there's no way to constrain what can be awaited. If this used generators instead it would be possible to ensure that step() is always used, both at runtime and when type checking.

could we remove first parameter of step? how do you think about using reference check of promise as React's useThenable?

I really like this idea, but I think it's going to be very easy to forget to use step() somewhere, and that would have really unpleasant consequences when trying to resume a function. TypeScript can't help either, as there's no way to constrain what can be awaited. If this used generators instead it would be possible to ensure that step() is always used, both at runtime and when type checking.

You have the same problem with generators, since it's possible to make a promise.then(...) call without wrapping it or execute some unwrapped side-effect.

The important thing to remember here is that this is an opt-in feature. The default behavior is that the promise (or parts of the promise) will re-run. So it will still work, but this may be undesirable. However, I'd argue that it's impractical to prevent every possible way that a developer may forget to do something or do things wrongly, outside of providing lint rules.

IMO if strict guardrails are desired, using a state machine would be the way to go.

could we remove first parameter of step? how do you think about using reference check of promise as React's useThenable?

If we remove the first parameter, then 1) we don't have explicit names for steps, which makes it harder to visualize, and 2) similar to React hooks, we're now dependent on the order in which these steps get called, which limits the ability to call these within e.g. loops or if-statements.

Reference checking wouldn't work when restoring state (e.g. from localstorage or a DB).

If we remove the first parameter, then 1) we don't have explicit names for steps

Maybe name could be inferred from named functions, and overridden by name attribute?

function step(promiseCreator: () => Promise<any>, nameOverride?: string) {
  let name = promiseCreator.name;
  if (nameOverride) name = nameOverride;
  
  if(!name) {
    if (isDevelopment) {
      console.warn('...')
    }
    return promiseCreator();
  }
        
  if (state.steps?.[name]) {
    return state.steps[name][1];
  }

  ...
}

Usage:

const fetchUser = () => fetch(url);
step(fetchUser); // => name = fetchUser
step(fetchUser, 'user'); // => name = user
step(() => fetch(url), 'user'); // => name = user
step(() => fetch(url)); // => no name, just returns the promise and warns in dev mode

I guess depending 'name' property of constructor or instance could make confusion for production as names are mingled formally?

I guess depending 'name' property of constructor or instance could make confusion for production as names are mingled formally?

That might indeed be problematic for visualization.