Refactor callback context and handling #328
Conversation
✅ Deploy Preview for kitbag-router ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
@stackoverfloweth incorporated the feedback from our sync review. I think this one is ready to go |
There was a problem hiding this comment.
I get build error locally on this branch, do you see that?
this doesn't quite fully implement my suggestion of a shared instanceof check. Take a look at this PR
| /** | ||
| * Defines the structure of a callback response. | ||
| */ | ||
| export type CallbackResponse = CallbackSuccessResponse | CallbackPushResponse | CallbackRejectResponse | CallbackAbortResponse |
There was a problem hiding this comment.
I see that this type isn't used anywhere, will this be used in props context?
There was a problem hiding this comment.
I use versions of this in my suggestion PR
I don't get a build error when I build locally. What are you seeing?
I went down this route too. But since the two places that use context currently have different context I thought it might be best to allow each callback to pick and choose its context and handle what its expecting. Especially once we add props now there are three different things that have context. Each of which could be different. So it seemed simpler to make the callback context agnostic of where/how it was used. |
Description
Not a change to routing behavior. But abstracts the callback context for hooks into something more generic so we can use it for props. Half of this is copied from #276 which is getting pretty outdated because main has changed quite a bit due to prefetching.
I've removed the generic from hook runners which was used for typing
push. But that wasn't really accomplishing anything. So I replaced it withRegisteredRouterPushfrom the callback context utility.I also moved the hook response types to be more generic callback response types and wrote a generic handler in
createRouter. There are some special cases for before hooks which are handled separately but before and after hooks and in the next PR props can all share the same callback response handler.