-
-
Notifications
You must be signed in to change notification settings - Fork 5
Show error if js assets fail to load #2019
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
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughImplements conditional asset loading with try-catch in SvelteLayout.razor, adds a nullable string field for errors, renders the error in the UI, and differentiates import behavior for development vs. prebuilt assets. On failure, sets a contextual error message, triggers StateHasChanged, and rethrows. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
UI unit Tests 1 files 45 suites 29s ⏱️ Results for commit 2e00b08. ♻️ This comment has been updated with latest results. |
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/FwLite/FwLiteShared/Layout/SvelteLayout.razor(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: frontend-component-unit-tests
- GitHub Check: frontend
- GitHub Check: Build FW Lite and run tests
🔇 Additional comments (2)
backend/FwLite/FwLiteShared/Layout/SvelteLayout.razor (2)
39-39: LGTM! Error field and rendering setup looks good.The nullable
errorfield and its rendering in the UI are appropriate for displaying asset loading failures.Also applies to: 46-46
76-84: LGTM! Conditional asset loading logic is correct.The branching between development and production asset imports is properly implemented with appropriate paths for each environment.
f62831e to
2e00b08
Compare
If assets fail to load then there's just a white page.
Initially I just added this in dev mode, but why not make errors more visible in prod too?
This improvement has been in my stashes for a long time.