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

Properly handle lazy errors during router initialization #10201

Merged
merged 1 commit into from
Mar 14, 2023

Conversation

brophdawg11
Copy link
Contributor

Run lazy() via startNavigation in router.inititialize() so we can properly bubble lazy() errors to boundaries.

Closes #10194

@changeset-bot
Copy link

changeset-bot bot commented Mar 14, 2023

🦋 Changeset detected

Latest commit: 647dd85

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

element: <h1>Action</h1>,
})}
/>
</Route>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Altered this test so it's not loading into a lazy route, and instead is submitting to one (which is what the test is for)

element: <h1>Path</h1>,
})}
/>
</Route>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same here - load into a non-lazy route, submit form to a lazy route

@@ -4434,72 +4475,6 @@ function testDomRouter(
`);
});

it("renders hydration errors on lazy leaf elements", async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not a use-case we need to support (for now) since we landed on SSR needing to pre-resolve lazy for initial matches

// in the normal navigation flow. For SSR it's expected that lazy modules are
// resolved prior to router creation since we can't go into a fallbackElement
// UI for SSR'd apps
if (!state.initialized) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This goes back to exactly what we did prior to lazy. There's no "I'm half iniitalized" concept. Either both lazy and data is resolved for initial matches (SSR), or neither is (SPA).

Copy link
Member

@markdalgleish markdalgleish left a comment

Choose a reason for hiding this comment

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

Nice one 👍

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.

[Bug]: Errors fetching lazy routes aren't caught by errorElement on refresh
3 participants