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

Updates to static handler for Remix integration #9511

Merged
merged 7 commits into from
Nov 1, 2022

Conversation

brophdawg11
Copy link
Contributor

@brophdawg11 brophdawg11 commented Oct 27, 2022

Updates to createStaticHandler to streamline and make internal router-thrown ErrorResponse's consistent for easier integration into Remix. This also fixes a bug where a loader/action could previously return undefined which causes subsequent issues with hydration and revalidation, so we now throw an error to the errorElement if this happens. Users can return null if they don't need to return any data.

Todo:

  • Repoint to dev
  • ensure changeset is updated for any non-static fixes

@changeset-bot
Copy link

changeset-bot bot commented Oct 27, 2022

🦋 Changeset detected

Latest commit: 51f453a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages
Name Type
@remix-run/router Patch
react-router Patch
react-router-dom Patch
react-router-dom-v5-compat Patch
react-router-native Patch

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

@@ -458,7 +458,7 @@ describe("NavLink using a data router", () => {
fireEvent.click(screen.getByText("Link to Bar"));
expect(screen.getByText("Link to Bar").className).toBe("pending");

dfd.resolve();
dfd.resolve(null);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These tests used to return undefined, so make them return null now

@@ -11214,7 +11302,7 @@ describe("a router", () => {
);
});

it("should handle not found routes with a 404 Response", async () => {
describe("Errors with Status Codes", () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

View this section with whitespace hidden, but now instead of returning responses we return a new ErrorWithStatus instance and we align the error messages with what Remix uses

loaderHeaders: {},
actionHeaders: {},
};
throw result.error;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No need to put this into a context object, just throw it directly now

@brophdawg11 brophdawg11 self-assigned this Oct 28, 2022
@brophdawg11 brophdawg11 changed the base branch from release-6.4.3 to dev November 1, 2022 15:24
@brophdawg11 brophdawg11 merged commit 67f16e7 into dev Nov 1, 2022
@brophdawg11 brophdawg11 deleted the brophdawg11/rrr-updates branch November 1, 2022 16:27
GOI17 pushed a commit to GOI17/react-router that referenced this pull request Nov 15, 2022
* Updates to unstable staticHandler logic for Remix integration

* Add changeset

* bump bundle

* Remove ErrorWithStatus in favor of extended ErrorResponse

* update

* Update changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants