-
Notifications
You must be signed in to change notification settings - Fork 4
Refactor loader functions to improve error handling and simplify response structure in package source tab #1617
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
base: 11-19-_ts-api-react-actions_enhance_error_handling_in_api_actions_and_hooks_with_user-facing_error_mapping
Are you sure you want to change the base?
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
| message: "Failed to load source", | ||
| source: undefined, | ||
| }; | ||
| throwUserFacingPayloadResponse({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Nitpick: this could be defined as variable outside the loader and then used by both loaders.
- Offtopic:
packageListing.tsxseems to always render link to this tab even if the source doesn't exists. Not ideal UX and not up to feature parity - On the legacy site accessing source tab for a mod that doesn't have source available doesn't show 404. I personally think showing 404 here would be ok if we didn't link to the non-existent page ourselves. If Add ESLint #2 above isn't fixed, I think this page should show non-404 page that tells the user the source isn't available. I don't have a strong opinion on which one we do, but we should do one.
| ); | ||
| } | ||
|
|
||
| export function ErrorBoundary() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I didn't get any response the last time I suggested using export { NimbusDefaultRouteErrorBoundary as ErrorBoundary } from "cyberstorm/utils/errors/NimbusErrorBoundary";. Does that not work or do you think it's a bad idea otherwise?
cd40580 to
5ac0d36
Compare
aeff7bd to
643d845
Compare
…onse structure in package source tab
643d845 to
1f8b5db
Compare
5ac0d36 to
de2572f
Compare

No description provided.