alleyinteractive/alley-scripts

Block Editor Tools: `Suspense` support

Opened this issue · 8 comments

Description

I just realized that none of the hooks, https://github.com/alleyinteractive/alley-scripts/tree/main/packages/block-editor-tools/src/hooks, have support for getting the status of a selector so that a developer can check if a resolver has finished its job.

A task that's particularly useful in scenarios where you need to know if data has been loaded or not.

This would involve a breaking change release, since the returned value of the hooks would be different.


For example:

const MyBlock = ({
 postID,
}) => {
  const post = usePost(postID, postType);
  
  // has the resolver finished getting the post?
  // What should the component do in the meantime?
  if (post) {
    ...
  }
};

More: https://redux.js.org/usage/deriving-data-selectors

Acceptance Criteria

  • Add "Suspense" versions to hooks that would benefit from them (e.g., ones that use useSelect, like usePost). An example of this would be a hook like useSuspensePost that is a Suspense-enabled version of usePost. Under the hood, it could use useSuspenseSelect instead of useSelect.

@renatonascalves is there a library that uses this pattern that you can reference? We've implemented this in a custom way in several of our repos, but if we are going to implement this in a more robust way, I'd like to follow some established patterns.

Another option here would be to use useSuspenseSelect, which would give the option of leveraging Suspense without threat to being a breaking change.

^ nice find! Had no idea. This would be ideal since it fallbacks to the default behavior.

@kevinfodness Do you have any objection in using the useSuspenseSelect?

As part of the task, I'll add an example with and without suspense, to make it clearer for whoever is copying and pasting.

The docs on this are terrible, so I went digging in the source code to figure out how this actually works. When the docs say that it "throws a Promise" it means that literally: https://github.com/WordPress/gutenberg/blob/3888f1e58528be5a641621ab171eba9f1e816928/packages/data/src/redux-store/index.js#L536

So I think this would be a breaking change, because users would need to wrap these hooks in a try/catch block, otherwise the thrown Promise would yield an uncaught Promise.

For example:

try {
  const post = usePost(postID, postType);
} catch ($promise) {
  // Do something with the unresolved state
}

Yeah, it was too good to be true! Also since the Suspense tag in the parent component is also required to "catch" the promise.

I was reading the original pr that introduced it, WordPress/gutenberg#37261, and the decision was exactly that. Not to fallback to the default behavior, otherwise it would not be truly supporting Suspense.

Having said all that, maybe it'd be better to have a Suspense version. Something like: useSuspensePost.

The useSuspense... is also how some libraries are handing Suspense support.

See https://tanstack.com/query/latest/docs/framework/react/guides/migrating-to-v5#new-hooks-for-suspense

Yeah, let's do that - making a separate version of each hook that would benefit from Suspense makes sense, lets users opt-in, and lets us customize the implementation of those hooks to use Suspense properly for each context (which may be different for different hooks).