Skip to content
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

Make library compatible with Remix v3_singleFetch #2007

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

lizkenyon
Copy link
Contributor

@lizkenyon lizkenyon commented Jan 23, 2025

WHY are these changes introduced?

Fixes #0000

Remix has introduced the v3_singleFetch future flag.

Single Fetch is a new data loading strategy and streaming format. When you enable Single Fetch, Remix will make a single HTTP call to your server on client-side transitions, instead of multiple HTTP calls in parallel (one per loader).

The headers function is now used on both document and data requests when Single Fetch is enabled. You should use that function to merge any headers returned from loaders executed in parallel, or to return any given actionHeaders.

Remix runs the headers function from starting with the parent route and then the sub routes

Remix passes in the parentHeaders to your headers function. So users.tsx headers get passed to users.$userId.tsx, and then users.$userId.tsx's headers are passed to users.$userId.profile.tsx's headers.
We don't want surprise headers in your responses, so it's your job to merge them if you'd like.

Why is this a problem for the Remix package?

With this flag enabled, if there are cases where the package modifies the headers of a response, then a headers function export will need to exist on the route to properly merge the headers.

In the package headers are added/modified in the following scenarios

  1. Redirecting to accept billing

  2. Retrying a request when an ID token is invalid

  3. When using the cors() helper function

  4. When using the liquid() helper function

  5. When loading the bounce page to refetch the id token

  6. Responding to options requests

  7. When forwarding the content type headers of a failed request other than a 401

  8. When throwing the response from revoke scopes. (Does this need to be explicitly set here?)

WHAT is this pull request doing?

This pull request changes from throw a response to throwing a redirect for the cases of 1) and 2). Redirects still cause you to break the call stack, allowing the headers to remain.

Issues 3) and 4)
These could be solved via education through documentation or something like a eslint rule.

Issue 5)
Is there ever a scenario where we would trigger a bounce request in a data/xhr request? If not then providing the headers function in the app.tsx of the template should properly add the headers in these scenarios.

Type of change

  • Patch: Bug (non-breaking change which fixes an issue)
  • Minor: New feature (non-breaking change which adds functionality)
  • Major: Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

  • I have used pnpm changeset to create a draft changelog entry (do NOT update the CHANGELOG.md files manually)
  • I have added/updated tests for this change
  • I have documented new APIs/updated the documentation for modified APIs (for public APIs)

With the remix future flag headers on responses need to be explictly merged

Redirect though will "short circut" and be used as is

When we require that headers need to get added we must now use the redirect
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant