Add prerender support to Brouter (#12410)#12421
Open
msynk wants to merge 2 commits into
Open
Conversation
msynk
requested review from
Cyrus-Sushiant,
Copilot,
mhrastegari,
yasmoradi and
zahra-yousefi
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR optimizes BlazorUI JS interop calls to use ChangesBlazorUI JS Interop Synchronous DOM Operations
Brouter Prerendering Support with Blazor Server Demo
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds server prerendering support to Bit.Brouter by moving the initial route match into a prerender-friendly lifecycle phase, and introduces a new Blazor Web App demo head to validate SSR behavior end-to-end.
Changes:
- Run the initial Brouter route match in
OnInitializedAsync(after a single yield) so prerendered HTML includes the matched route output. - Ensure SSR redirect signals (
NavigationException) propagate during prerender instead of being swallowed by Brouter error handling. - Add a new
NewDemointeractive-server project (and solution filter) to showcase and manually verify prerendering behavior; plus minor JS interop perf tweaks viaFastInvoke*.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Brouter/NewDemo/README.md | Documents the new prerendering demo head and how to validate SSR output. |
| src/Brouter/NewDemo/Bit.Brouter.NewDemo.Server/Properties/launchSettings.json | Adds local dev launch settings for the new demo server. |
| src/Brouter/NewDemo/Bit.Brouter.NewDemo.Server/Program.cs | Configures Blazor Web App InteractiveServer pipeline + static assets + core services for the demo. |
| src/Brouter/NewDemo/Bit.Brouter.NewDemo.Server/Components/Pages/Host.razor | Catch-all host page that delegates actual routing to the Core demo’s <AppRouter />. |
| src/Brouter/NewDemo/Bit.Brouter.NewDemo.Server/Components/Pages/Error.razor | Adds a dedicated /Error page for production exception handler re-execution. |
| src/Brouter/NewDemo/Bit.Brouter.NewDemo.Server/Components/BlazorRoutes.razor | Minimal Blazor Router used only to map requests to the catch-all host page. |
| src/Brouter/NewDemo/Bit.Brouter.NewDemo.Server/Components/App.razor | Host document enabling InteractiveServer rendering and loading shared demo styles. |
| src/Brouter/NewDemo/Bit.Brouter.NewDemo.Server/Components/_Imports.razor | Common imports for the new demo components. |
| src/Brouter/NewDemo/Bit.Brouter.NewDemo.Server/Bit.Brouter.NewDemo.Server.csproj | New demo server project referencing the existing demo Core for routes/pages/layout. |
| src/Brouter/NewDemo/Bit.Brouter.NewDemo.Server/appsettings.json | Basic logging/hosting config for the new demo server. |
| src/Brouter/Bit.Brouter/Brouter.cs | Core change: initial match moved to OnInitializedAsync + SSR redirect propagation. |
| src/Brouter/Bit.Brouter.slnx | Adds the new demo server project to the solution structure. |
| src/Brouter/Bit.Brouter.NewDemo.slnf | Adds a solution filter focused on the new demo scenario. |
| src/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/UtilsJsRuntimeExtensions.cs | Switches Utils JS calls to FastInvoke* paths (sync in-process, async fallback otherwise). |
| src/BlazorUI/Bit.BlazorUI/Components/Surfaces/ScrollablePane/BitScrollablePaneJsRuntimeExtensions.cs | Switches scroll-to-end interop to FastInvokeVoid. |
Comment on lines
+8
to
+9
| MainLayout is supplied by the Blazor router (Routes.razor -> RouteView DefaultLayout), so we | ||
| only render AppRouter here and do not wrap it in a layout again. |
Comment on lines
+3
to
+7
| // The Utils JS functions are pure, synchronous DOM operations and every one of them | ||
| // internally null-guards its target element (see Scripts/Utils.ts). That makes them safe | ||
| // to run through FastInvoke/FastInvokeVoid: on an in-process runtime (Blazor WASM/Hybrid) | ||
| // they execute synchronously without the async interop round-trip, and on other runtimes | ||
| // (Blazor Server/prerender) FastInvoke transparently falls back to the async path. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/UtilsJsRuntimeExtensions.cs`:
- Around line 28-31: BitUtilsGetBoundingClientRect can return
default(BoundingClientRect) when FastInvoke<TValue> swallows JsonException for
IJSInProcessRuntime (see IJSRuntimeFastExtensions.FastInvoke<TValue>) or
propagate deserialization errors for other runtimes; update all callers of
BitUtilsGetBoundingClientRect to explicitly handle a default/empty
BoundingClientRect (or null-equivalent) rather than assuming valid values—e.g.,
check for width/height==0 or a sentinel and bail out or provide fallback layout
logic during prerendering/non-JS contexts (also consider whether
BitUtilsGetBoundingClientRect itself should validate the result and throw/return
a nullable to make handling explicit).
- Around line 3-7: Update the comment in UtilsJsRuntimeExtensions.cs to remove
the blanket claim that all Utils functions are "pure" and "internally
null-guard" their target element; instead state that most functions in
Scripts/Utils.ts (e.g., setProperty, getProperty, getBoundingClientRect,
scrollElementIntoView, selectText, setStyle, toggleOverflow) guard their inputs
but some do not (notably getBodyWidth reads document.body.offsetWidth without a
null check) and several intentionally cause side effects (scrollElementIntoView,
selectText, setStyle, toggleOverflow), and therefore FastInvoke/FastInvokeVoid
should be used with that nuance in mind (it still falls back to async when
needed).
In `@src/Brouter/NewDemo/README.md`:
- Around line 23-24: Update the README command so the path for dotnet run is
explicit for both common working directories: show one example that runs from
the repository root (e.g., dotnet run --project
src/Brouter/NewDemo/Bit.Brouter.NewDemo.Server) and another that runs from the
src/Brouter folder (e.g., dotnet run --project
NewDemo/Bit.Brouter.NewDemo.Server), and apply the same change to the other
occurrence at the later lines; reference the NewDemo/Bit.Brouter.NewDemo.Server
project name in the text so copy/pasted commands work regardless of current
directory.
- Around line 27-28: The README's example curl commands for verifying
prerendering use plain http and may hit the service's HTTPS redirect; update the
two example commands (the curl lines referencing /counter/1234 and
/profile/saleh) to either use the correct HTTPS URL (https://localhost:7180/...)
or include curl -L to follow redirects so the verification returns the
prerendered HTML as intended.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 19157446-3a6b-4b87-817f-1826701737e8
📒 Files selected for processing (15)
src/BlazorUI/Bit.BlazorUI/Components/Surfaces/ScrollablePane/BitScrollablePaneJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/UtilsJsRuntimeExtensions.cssrc/Brouter/Bit.Brouter.NewDemo.slnfsrc/Brouter/Bit.Brouter.slnxsrc/Brouter/Bit.Brouter/Brouter.cssrc/Brouter/NewDemo/Bit.Brouter.NewDemo.Server/Bit.Brouter.NewDemo.Server.csprojsrc/Brouter/NewDemo/Bit.Brouter.NewDemo.Server/Components/App.razorsrc/Brouter/NewDemo/Bit.Brouter.NewDemo.Server/Components/BlazorRoutes.razorsrc/Brouter/NewDemo/Bit.Brouter.NewDemo.Server/Components/Pages/Error.razorsrc/Brouter/NewDemo/Bit.Brouter.NewDemo.Server/Components/Pages/Host.razorsrc/Brouter/NewDemo/Bit.Brouter.NewDemo.Server/Components/_Imports.razorsrc/Brouter/NewDemo/Bit.Brouter.NewDemo.Server/Program.cssrc/Brouter/NewDemo/Bit.Brouter.NewDemo.Server/Properties/launchSettings.jsonsrc/Brouter/NewDemo/Bit.Brouter.NewDemo.Server/appsettings.jsonsrc/Brouter/NewDemo/README.md
Comment on lines
+3
to
+7
| // The Utils JS functions are pure, synchronous DOM operations and every one of them | ||
| // internally null-guards its target element (see Scripts/Utils.ts). That makes them safe | ||
| // to run through FastInvoke/FastInvokeVoid: on an in-process runtime (Blazor WASM/Hybrid) | ||
| // they execute synchronously without the async interop round-trip, and on other runtimes | ||
| // (Blazor Server/prerender) FastInvoke transparently falls back to the async path. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify that all Utils JS functions are synchronous and null-guarded
# Find and display the Utils TypeScript/JavaScript implementation
fd -e ts -e js --full-path "Scripts/Utils" | xargs cat
# Also search for the Utils module definition if not in Scripts/ directory
ast-grep --pattern 'namespace BitBlazorUI {
$$$
export namespace Utils {
$$$
}
$$$
}'Repository: bitfoundation/bitplatform
Length of output: 4548
Fix JS-runtime rationale: Utils helpers aren’t all null-guarded/pure (src/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/UtilsJsRuntimeExtensions.cs, Lines 3-7)
Scripts/Utils.tsimplementations for the listed helpers are synchronous (noasync/await/Promisereturns), and most listed functions guard their element inputs (setProperty,getProperty,getBoundingClientRect,scrollElementIntoView,selectText,setStyle,toggleOverflow).- The blanket claim “every one … internally null-guards its target element” is inaccurate:
getBodyWidth()directly readsdocument.body.offsetWidthwith no guard fordocument.body. - Calling them “pure” is also inaccurate: several functions intentionally cause side effects (e.g.,
scrollIntoView,select, and style/overflow mutation).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/UtilsJsRuntimeExtensions.cs`
around lines 3 - 7, Update the comment in UtilsJsRuntimeExtensions.cs to remove
the blanket claim that all Utils functions are "pure" and "internally
null-guard" their target element; instead state that most functions in
Scripts/Utils.ts (e.g., setProperty, getProperty, getBoundingClientRect,
scrollElementIntoView, selectText, setStyle, toggleOverflow) guard their inputs
but some do not (notably getBodyWidth reads document.body.offsetWidth without a
null check) and several intentionally cause side effects (scrollElementIntoView,
selectText, setStyle, toggleOverflow), and therefore FastInvoke/FastInvokeVoid
should be used with that nuance in mind (it still falls back to async when
needed).
Comment on lines
28
to
31
| internal static ValueTask<BoundingClientRect> BitUtilsGetBoundingClientRect(this IJSRuntime jsRuntime, ElementReference element) | ||
| { | ||
| return jsRuntime.Invoke<BoundingClientRect>("BitBlazorUI.Utils.getBoundingClientRect", element); | ||
| return jsRuntime.FastInvoke<BoundingClientRect>("BitBlazorUI.Utils.getBoundingClientRect", element); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Examine FastInvoke<T> implementation to verify error handling
# Search for FastInvoke<T> implementation in Butil
rg -n -A 30 "public static ValueTask<TValue> FastInvoke" src/Butil/
# Also check for any error handling or JsonException handling
rg -n -C 5 "FastInvoke.*JsonException" src/Butil/Repository: bitfoundation/bitplatform
Length of output: 51
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find any FastInvoke implementations anywhere in src
rg -n --hidden -S "FastInvoke<|FastInvokeVoid|FastInvoke\s*\(" src || true
# Find the file(s) containing "FastInvokeVoid" specifically
rg -n --hidden -S "FastInvokeVoid" src || true
# Find any JSON exception handling near FastInvoke
rg -n --hidden -S "FastInvoke.*JsonException|JsonException.*FastInvoke" src || true
# Print candidate FastInvoke implementation blocks (limited context)
rg -n -S -A 40 "FastInvoke<" src || trueRepository: bitfoundation/bitplatform
Length of output: 28739
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --hidden -S "FastInvoke<|FastInvokeVoid|FastInvoke\s*\(" src || true
rg -n --hidden -S "FastInvokeVoid" src || true
rg -n --hidden -S "FastInvoke.*JsonException|JsonException.*FastInvoke" src || true
rg -n -S -A 40 "FastInvoke<" src || trueRepository: bitfoundation/bitplatform
Length of output: 28739
Confirm FastInvoke<T> deserialization error handling for BoundingClientRect.
jsRuntime.FastInvoke<BoundingClientRect> ultimately uses FastInvoke<TValue> in src/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/IJSRuntimeFastExtensions.cs: when jsRuntime is an IJSInProcessRuntime, it catches JsonException, logs to Console.Error, and returns default(TValue) (so a bad JSON payload yields a default BoundingClientRect instead of throwing). When jsRuntime is not an IJSInProcessRuntime, FastInvoke<TValue> delegates to jsRuntime.Invoke<TValue>(...) without catching, so deserialization failures will follow Invoke behavior.
- Ensure callers of
BitUtilsGetBoundingClientRecthandle the possibledefault(BoundingClientRect)result (and consider whether prerendering uses a non-IJSInProcessRuntimepath).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/UtilsJsRuntimeExtensions.cs`
around lines 28 - 31, BitUtilsGetBoundingClientRect can return
default(BoundingClientRect) when FastInvoke<TValue> swallows JsonException for
IJSInProcessRuntime (see IJSRuntimeFastExtensions.FastInvoke<TValue>) or
propagate deserialization errors for other runtimes; update all callers of
BitUtilsGetBoundingClientRect to explicitly handle a default/empty
BoundingClientRect (or null-equivalent) rather than assuming valid values—e.g.,
check for width/height==0 or a sentinel and bail out or provide fallback layout
logic during prerendering/non-JS contexts (also consider whether
BitUtilsGetBoundingClientRect itself should validate the result and throw/return
a nullable to make handling explicit).
Comment on lines
+23
to
+24
| dotnet run --project NewDemo/Bit.Brouter.NewDemo.Server | ||
|
|
There was a problem hiding this comment.
Make dotnet run --project path unambiguous from this README location.
The command path assumes a specific working directory. Add explicit alternatives (repo root vs src/Brouter) to prevent copy/paste failures from src/Brouter/NewDemo context.
✍️ Suggested doc tweak
- dotnet run --project NewDemo/Bit.Brouter.NewDemo.Server
+ # From src/Brouter
+ dotnet run --project NewDemo/Bit.Brouter.NewDemo.Server
+ # From repo root
+ dotnet run --project src/Brouter/NewDemo/Bit.Brouter.NewDemo.ServerAlso applies to: 48-49
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Brouter/NewDemo/README.md` around lines 23 - 24, Update the README
command so the path for dotnet run is explicit for both common working
directories: show one example that runs from the repository root (e.g., dotnet
run --project src/Brouter/NewDemo/Bit.Brouter.NewDemo.Server) and another that
runs from the src/Brouter folder (e.g., dotnet run --project
NewDemo/Bit.Brouter.NewDemo.Server), and apply the same change to the other
occurrence at the later lines; reference the NewDemo/Bit.Brouter.NewDemo.Server
project name in the text so copy/pasted commands work regardless of current
directory.
Comment on lines
+27
to
+28
| curl http://localhost:5180/counter/1234 | ||
| curl http://localhost:5180/profile/saleh |
There was a problem hiding this comment.
Use HTTPS (or follow redirects) in prerender verification commands.
Given HTTPS redirection in startup, Line 27 and Line 28 may return a redirect instead of raw prerendered HTML. Update examples to https://localhost:7180/... or add curl -L so the verification works as written.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Brouter/NewDemo/README.md` around lines 27 - 28, The README's example
curl commands for verifying prerendering use plain http and may hit the
service's HTTPS redirect; update the two example commands (the curl lines
referencing /counter/1234 and /profile/saleh) to either use the correct HTTPS
URL (https://localhost:7180/...) or include curl -L to follow redirects so the
verification returns the prerendered HTML as intended.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #12410
Summary by CodeRabbit
New Features
Bug Fixes
Performance Improvements