New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a new optional AbortController parameter to defer #10792
base: dev
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 94973bb The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
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 |
return new DeferredData( | ||
data, | ||
responseInit, | ||
controller || new AbortController() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use user controller
if provided
// controller also aborts. | ||
let result = await handler(args); | ||
if (args.request.signal.aborted && isDeferredData(result)) { | ||
result.cancel(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cancel defer()
instances created after the request
has already been aborted
// Abort the defer() instance if we abort while waiting on other loaders | ||
// from this navigation | ||
let deferResult = result; | ||
request.signal.addEventListener("abort", () => deferResult.cancel()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cancel defer()
instanced created before the request
is aborted
🦋 Changeset detectedLatest commit: 2d7450b The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
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 |
Talked to @jacob-ebey and we think we may want to align React Router with the Remix behavior here. In Remix because it's using an actual HTTP streaming request, the This should just require attaching the navigation |
POC for #10786 which would let users provide an
AbortController
todefer
Note, the second parameter to
defer
is aResponseInit
(just likejson
), so I made this a standalone 3rd param to be explicit/simple/clear. We could override the second param and also allowdefer({ data: promise }, controller)
if we wanted though.