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

Add support for X-Remix-Reload-Document header #6842

Merged
merged 4 commits into from
Aug 22, 2023
Merged

Add support for X-Remix-Reload-Document header #6842

merged 4 commits into from
Aug 22, 2023

Conversation

robbtraister
Copy link
Contributor

This PR is paired with remix-run/react-router#10705 to add support for passing along the X-Remix-Reload-Document header.

Open Development discussion

@changeset-bot
Copy link

changeset-bot bot commented Jul 14, 2023

⚠️ No Changeset found

Latest commit: 19e9291

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes changesets to release 16 packages
Name Type
@remix-run/cloudflare Minor
@remix-run/deno Minor
@remix-run/node Minor
@remix-run/server-runtime Minor
@remix-run/cloudflare-pages Minor
@remix-run/cloudflare-workers Minor
@remix-run/architect Minor
@remix-run/express Minor
@remix-run/serve Minor
@remix-run/testing Minor
@remix-run/dev Minor
@remix-run/react Minor
create-remix Minor
remix Minor
@remix-run/css-bundle Minor
@remix-run/eslint-config Minor

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@robbtraister robbtraister marked this pull request as ready for review July 14, 2023 20:22
@zackerydev
Copy link

Any idea when this is going to land? Need any help getting this in

@gerbyzation
Copy link

@zackeryg see remix-run/react-router#10705 (comment)

@brophdawg11
Copy link
Contributor

Thanks @robbtraister! We updated to an experimental react router version in #7040 so this is good to merge to dev and ship in Remix v2. I added an integration test as well so I'll get this merge once CI passes.

Comment on lines 258 to +264
if (revalidate) {
headers["X-Remix-Revalidate"] = revalidate;
}
let reloadDocument = response.headers.get("X-Remix-Reload-Document");
if (reloadDocument) {
headers["X-Remix-Reload-Document"] = reloadDocument;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brophdawg11 do you think it's worth refactoring this into an array + for-loop (or .forEach or .reduce)?

something like:

let HEADERS_TO_PRESERVE = ["X-Remix-Revalidate", "X-Remix-Reload-Document"];

...

  for (let headerName of HEADERS_TO_PRESERVE) {
    let headerValue = response.headers.get(headerName);
    if (headerValue) {
      headers[headerName] = headerValue;
    }
  }

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nah not yet for the 2nd header - something something kent c dodds avoid hasty abstractions :)

@brophdawg11 brophdawg11 merged commit 50cdfec into remix-run:dev Aug 22, 2023
@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version v0.0.0-nightly-49e8da1-20230823 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@robbtraister robbtraister deleted the feature/redirect-with-reload branch August 23, 2023 16:55
@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version 2.0.0-pre.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants