Update single fetch headers approach to use response stub API #9142
Conversation
🦋 Changeset detectedLatest commit: 6bfd9f3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 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 |
| @@ -16,10 +15,6 @@ export function getDocumentHeaders( | |||
| ? context.matches.slice(0, boundaryIdx + 1) | |||
| : context.matches; | |||
|
|
|||
| if (loadRouteIds) { | |||
| matches = matches.filter((m) => loadRouteIds.includes(m.route.id)); | |||
| } | |||
There was a problem hiding this comment.
single fetch no longer uses getDocumentHeaders
| loadContext, | ||
| action, | ||
| params, | ||
| request, | ||
| routeId, | ||
| singleFetch, | ||
| response, |
There was a problem hiding this comment.
ResponseStub is sent down to all loader/action calls (single fetch only)
| @@ -27,6 +27,22 @@ export type DataFunctionArgs = RRActionFunctionArgs<AppLoadContext> & | |||
| context: AppLoadContext; | |||
| }; | |||
|
|
|||
| export const ResponseStubOperationsSymbol = Symbol("ResponseStubOperations"); | |||
| export type ResponseStubOperation = [ | |||
| "set" | "append" | "delete", | |||
There was a problem hiding this comment.
Only track "write" operations
| @@ -406,60 +410,79 @@ async function singleFetchAction( | |||
| signal: request.signal, | |||
| ...(request.body ? { duplex: "half" } : undefined), | |||
| }); | |||
| let result = await staticHandler.queryRoute(handlerRequest, { | |||
There was a problem hiding this comment.
action's use query now as well and pass skipLoaders to just call the action. I think this should let us combine the singleFetchAction/singleFetchLoaders functions eventually too - but didn't look into that yet as part of this work
| requestContext: loadContext, | ||
| skipLoaderErrorBubbling: true, | ||
| skipLoaders: true, | ||
| unstable_dataStrategy: getSingleFetchDataStrategy(responseStubs), |
There was a problem hiding this comment.
We pass a factory-created dataStrategy to query now so we can have a set of responseStub's scoped to this request
| let headersProxy = new Proxy(headers, { | ||
| get(target, prop, receiver) { | ||
| if (prop === "set" || prop === "append" || prop === "delete") { | ||
| return (name: string, value: string) => { | ||
| operations.push([prop, name, value]); | ||
| Reflect.apply(target[prop], target, [name, value]); | ||
| }; | ||
| } | ||
| return Reflect.get(target, prop, receiver); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Using a Proxy around Headers to track write operations
|
|
||
| // Unsure why this is complaining? It's fine in VSCode but fails with tsc... | ||
| // @ts-expect-error | ||
| for (let v of result.result.headers.getSetCookie()) { |
There was a problem hiding this comment.
Confirming this is fine now that we use undici? I know this was one of the areas we deviated from the spec and didn't yet implement this method
github-actions
bot
added
the
awaiting release
| - `headers.set` on any child handler will overwrite values from parent handlers | ||
| - `headers.append` can be used to set the same header from both a parent and child handler | ||
| - `headers.delete` can be used to delete a value set by a parent handler, but not a value set from a child handler | ||
| - Because single fetch supports naked object returns, and you no longer need to return a `Response` instance to set status/headers, the `json`/`redirect`/`redirectDocument`/`defer` utilities are considered deprecated when using Single Fetch |
There was a problem hiding this comment.
@brophdawg11 , I totally understand that json and defer can be considered deprecated but I'm surprised for redirect and redirectDocument, especially redirectDocument which relies on a Remix custom header, ie an implementation that should be ignored in user land I think. So what will be the recommended way to redirect?
There was a problem hiding this comment.
I agree, specially because I usually throw redirect responses, to stop execution, will that stop to work?
No description provided.