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

useCatch in root CatchBoundary returns nested route's data if root and nested loader throw #4791

Closed
nikosdouvlis opened this issue Dec 7, 2022 · 8 comments · Fixed by remix-run/react-router#9702
Labels
bug Something isn't working feat:routing

Comments

@nikosdouvlis
Copy link

nikosdouvlis commented Dec 7, 2022

What version of Remix are you using?

1.8.1

Steps to Reproduce

  • Create a new Remix app by running npx create-remix@latest. The stack does not matter, so feel free to choose the default configuration (TS + Remix App Server).
  • Export a loader from root.tsx that throws a json response
  • Export a CatchBoundary from root.tsx that renders the data returned by calling useCatch
  • Create a new route, routes/withBoundary.tsx. This route exports a loader that throws a json response. It also exports a CatchBoundary.
  • Create a new route, routes/withoutBoundary.tsx. This route exports a loader that throws a json response. It does not export a CatchBoundary.
  • Run npm run dev to start the app

Expected Behavior

  • Navigating to /: the root CatchBoundary should render and useCatch will return the data of the root loader response.
  • Navigating to /withBoundary: the root CatchBoundary should render and useCatch will return the data of the root loader response.
  • Navigating to /withoutBoundary: the root CatchBoundary should render and useCatch will return the data of the root loader response.

Actual Behavior

  • Navigating to /: the root CatchBoundary renders, and useCatch returns the data of the root loader response.
  • Navigating to /withBoundary: the root CatchBoundary renders and useCatch returns of the root loader response.
  • Navigating to /withoutBoundary: the root CatchBoundary renders but useCatch returns the data of the /withoutBoundary loader response instead.

More context

Hello everyone! First things first... Thanks for the fantastic work you've been doing on Remix and RR; both are a joy to use.

Clerk added support for Remix apps last February when we released the @clerk/remix package, receiving great feedback from our community. Our auth mechanism is based on auto-refreshing short-lived JWTs. We use the root loader and root CatchBoundary to get a fresh token if we detect a stale one.

We've been investigating an issue our users reported following the Remix 1.8 release. It looks like the behavior of the useCatch/CatchBoundary changed between Remix versions 1.7.6 and 1.8, breaking our integration for some specific edge cases.

Maintaining OS projects is hard, so we hope the "Debugging" and "Reproducible examples" section below are helpful. Would you let us know if this is a breaking change or just a bug that slipped in the 1.8 release?

Debugging

As described above, starting with Remix 1.8, calling useCatch inside the root CatchBoundary will not return the data thrown by the root loader when these conditions are true for a given document request:

  • Root exports a loader and a CatchBoundary. The loader throws a json response.
  • A nested route exports a loader, but neither it nor one of its parent routes exports a CatchBoundary. Its loader also throws a json response.

In this scenario, the root CatchBoundary will be rendered but calling useCatch will return:

  • the status code of the root loader response
  • the data of the nested route loader response

Stepping through the Remix codebase after the latest release, we found out that:

errors = {} 
errors = { ...errors, root: results[0].error } // the error of the root loader is saved
errors = { ...errors, root: results[1].error } // the error of the nested route loader overwrites the above, because boundaryMatch.route.id falls back to 'root'

Reproducible examples

To demonstrate the above better, I created a sample repo that contains a main branch that uses Remix v1.8 and displays the issue I described above. It also includes a remix_1_7_6 branch that uses Remix v1.7.6 and works as expected. The only difference between the two is the Remix version used. Please find the repo here:

To make sure that we can reproduce the above outside of our local dev env, we also deployed both versions of the app on Vercel as well:

If you prefer stackblitz demos, please use these instead:

Also added a bug-report-test as requested by the "Submit Issue" template in this PR: #4792

nikosdouvlis added a commit to clerk/javascript that referenced this issue Dec 7, 2022
We temporarily throw the interstitial page from within getAuth in addition to the rootAuthLoader.ts

See remix-run/remix#4791 for details
nikosdouvlis added a commit to clerk/javascript that referenced this issue Dec 7, 2022
We temporarily throw the interstitial page from within getAuth in addition to the rootAuthLoader.ts

See remix-run/remix#4791 for details
nikosdouvlis added a commit to clerk/javascript that referenced this issue Dec 7, 2022
We temporarily throw the interstitial page from within getAuth in addition to the rootAuthLoader.ts

See remix-run/remix#4791 for details
@gsong
Copy link

gsong commented Dec 7, 2022

Off topic, but shoutout to @nikosdouvlis for such an amazing bug report.

@brophdawg11
Copy link
Contributor

@nikosdouvlis 🙌 They oughta give out awards for "best bug report" - this is fantastic! Just pushed a fix for this over in the @remix-run/router (remix-run/react-router#9702) so we'll get that out in an upcoming Remix release 👍

@nikosdouvlis
Copy link
Author

Awesome! I'm glad that the report helped :) And many thanks for pushing out a fix so fast, it's highly appreciated!

@brophdawg11
Copy link
Contributor

Hey @nikosdouvlis - just FYI this fix is available in a 1.9.0-pre.0 release if you wanted to give it a test! We're hoping to get the stable 1.9.0 out later today or tomorrow.

@brophdawg11 brophdawg11 added the awaiting release This issue has been fixed and will be released soon label Dec 15, 2022
@xHomu
Copy link
Contributor

xHomu commented Dec 25, 2022

Remix v1.9 with Clerk works fine!
But the problem seemed to have resurfaced with Remix v1.10.0pre5, would the v2_ErrorBoundary flag help?

image

React 17.

@brophdawg11
Copy link
Contributor

@xHomu Can you elaborate on your issue? Using 1.10.0-pre.5 against the stack blitz reproduction above, I am correctly getting the root loader thrown data from useCatch when accessing /example-without-catchboundary.

I think whatever you're reporting is potentially a separate issue?

@brophdawg11
Copy link
Contributor

@nikosdouvlis Just wanted to check back in to see if you've been able to confirm this fix on your end? There is a 1.10.0-pre.6 out currently and a 1.10.0-pre.7 coming in the next few hours which we think could be the final prerelease. We're aiming for a stable as early as Monday so would love to confirm your issue is fixed!

@xHomu Please let me know if you're still seeing anything wrong - and if it turns out to be a different cause let's open a new issue for that 👍

Thanks folks!

@xHomu
Copy link
Contributor

xHomu commented Jan 6, 2023

Will open a separate issue if I can isolate the situation! Thanks for the hard work, @brophdawg11 !

@brophdawg11 brophdawg11 removed the awaiting release This issue has been fixed and will be released soon label Apr 21, 2023
@brophdawg11 brophdawg11 removed their assignment Apr 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working feat:routing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants