-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Remove single fetch response stub, add headers #9769
Conversation
🦋 Changeset detectedLatest commit: 698ce13 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 |
My 2 cents: |
The response stub is very useful when you must always set some headers and you don't want to forget passing the headers manually, for example: export async function action({ request, response }: ActionFunctionArgs) {
const { userId } = await getSupabaseSession({
request,
response,
})
// ...
} Here if you don't set the headers returned by Supabase the user can end up with stale refresh token. Please reconsider this change |
In the future, session management will happen in // Pseudo-code - future API not yet determined
async function middleware({ request, context }, next) {
const { userId, headers } = await getSupabaseSession(request);
context.set('userId', userId);
let response = await next();
headers.entries().forEach(([k, v]) => response.headers.set(k, v));
return response;
} |
I agree - we're still bike shedding this a bit internally |
🤖 Hello there, We just published version Thanks! |
I saw that this was about to be released. I also saw the original roadmap planning video. I had two questions:
|
🤖 Hello there, We just published version Thanks! |
I agree with this. I use a lot of This breaks those use cases. I ended up aliasing all I now have a file in my project that aliases all in the following way: import {
type HeadersFunction,
unstable_data,
unstable_defineAction,
unstable_defineLoader,
} from '@remix-run/node';
export const defineAction = unstable_defineAction;
export const defineLoader = unstable_defineLoader;
export const defineHeaders = (headers: HeadersFunction) => headers;
export const defineData = unstable_data; |
is there any downside to wrapping all turbo-stream responses with |
Nope - no downside! |
Sibling React Router PR: remix-run/react-router#11836
TODO:
See below for details but the tl;dr; here is that for Single Fetch (unstable) we're removing
ResponseStub
and bringing backheaders()
which will be the place to merge headers for both document and data requests when single fetch is enabled. See the roadmap planning for details.Background
ResponseStub
so users could mutate status/headers in middleware before/after handlers and during handlersResponseStub
and removed the usage ofheaders
in single fetchawait next()
directly.headers
and apply that to single fetch and avoid introducing a totally new thing (that always felt a bit awkward to work with anyway).With this change:
return { data: whatever };
status
/headers
turbo-stream
, you can keep returning the responses you return today (i.e.,json()
)Response.json
in the next version thoughturbo-stream
- we'll be adding a newunstable_data({...}, responseInit)
utility that will essentially be a drop in replacement fordefer
and will let you send back status/headers alongside your data without having to encod eit into aResponse
headers()
function will let you control header merging for document and data requests