Add Brouter (#9923)#12361
Open
msynk wants to merge 3 commits into
Open
Conversation
msynk
requested review from
Cyrus-Sushiant,
Copilot,
mhrastegari,
yasmoradi and
zahra-yousefi
WalkthroughThis PR introduces Bit.Brouter, a comprehensive async-aware Blazor router supporting declarative nested routes, typed parameters with constraints, async guards and loaders, route specificity matching, and programmatic navigation. The implementation includes service infrastructure, multi-platform demo applications (MAUI and WebAssembly), and extensive test coverage. ChangesBit.Brouter Library, Demos, and Tests
🎯 4 (Complex) | ⏱️ ~60 minutes
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a new library, Bit.Brouter, a declarative Blazor router (closes #9923). It ships the library, a demo project (Web + MAUI + Core), tests, and integrates the new projects into the top-level solution files.
Changes:
- New
Bit.Brouterlibrary:Brouter/Route/Outletcomponents, template parsing with constraints/optional/catch-all, guards/loaders,BrouterLink,IBrouterservice with programmatic navigation and hooks. - New demo apps (
Bit.Brouter.Demo.Core/Web/Maui) and a bUnit test project covering template parsing, route matching, guards, and loaders. - Solution wiring (
Bit.Brouter.slnx,Bit.Brouter.Web.slnf) and registration in rootBit.slnx/Bit.CI.Release.slnx.
Reviewed changes
Copilot reviewed 85 out of 88 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Brouter/Bit.Brouter/* | Core router library: components, services, options, template parsing, constraints, link, attributes |
| src/Brouter/Bit.Brouter.csproj, slnx, slnf | Library project + solution files (multi-target net8/9/10) |
| src/Brouter/README.md | API documentation for the new library |
| src/Brouter/Demo/Bit.Brouter.Demo.Core/* | Shared demo pages, routes, layouts, and DI extensions |
| src/Brouter/Demo/Bit.Brouter.Demo.Web/* | Blazor WebAssembly host for the demo |
| src/Brouter/Demo/Bit.Brouter.Demo.Maui/* | MAUI Blazor Hybrid host for the demo |
| src/Brouter/Tests/Bit.Brouter.Tests/* | bUnit + xUnit v3 tests for parser, matching, guards, loaders |
| src/Brouter/settings.VisualStudio.json | VS workspace defaults |
| src/Bit.slnx, src/Bit.CI.Release.slnx | Wire new Brouter projects into root solutions |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Nitpick comments (4)
src/Brouter/Tests/Bit.Brouter.Tests/BrouterTests.cs (2)
33-47: ⚡ Quick winStrengthen specificity assertion by proving wildcard is not rendered.
Current assertion only proves
/aboutappears; it won’t fail if both routes render.Proposed test tightening
- cut.WaitForAssertion(() => Assert.NotNull(cut.Find("[data-testid=about]"))); + cut.WaitForAssertion(() => + { + Assert.NotNull(cut.Find("[data-testid=about]")); + Assert.Empty(cut.FindAll("[data-testid=star]")); + });🤖 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/Tests/Bit.Brouter.Tests/BrouterTests.cs` around lines 33 - 47, The test Selects_most_specific_route_when_wildcard_is_declared_first currently only asserts that the about route is present but doesn't verify the wildcard route is not rendered; update the assertion after rendering BrouterComp (variable cut) to explicitly assert that the wildcard element (data-testid="star") is not present—use the component's Find/FindAll or query API on cut to assert absence (e.g., Assert.Throws or Assert.Empty/Assert.Null) so the test fails if both routes render.
50-58: ⚡ Quick winCover both optional-parameter paths (with and without value).
The test name says “with_or_without_value”, but only the “without” path is validated.
Proposed additional assertion
public void Optional_parameter_matches_with_or_without_value() { var nav = Services.GetRequiredService<FakeNavigationManager>(); nav.NavigateTo("http://localhost/users"); var cut = RenderComponent<OptionalParamHost>(); cut.WaitForAssertion(() => Assert.Equal("(none)", cut.Find("[data-testid=out]").TextContent)); + nav.NavigateTo("http://localhost/users/42"); + cut.WaitForAssertion(() => Assert.Equal("42", cut.Find("[data-testid=out]").TextContent)); }🤖 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/Tests/Bit.Brouter.Tests/BrouterTests.cs` around lines 50 - 58, The test Optional_parameter_matches_with_or_without_value only exercises the "without value" path; add the missing "with value" path by navigating the FakeNavigationManager to a URL that includes the optional parameter (e.g., "/users/42" or whatever valid value your OptionalParamHost expects), re-render or update the component as needed (using RenderComponent<OptionalParamHost>() or calling cut.SetParametersAndRender/trigger navigation), and add a corresponding assertion that cut.Find("[data-testid=out]").TextContent equals the expected output for the provided value to verify both cases.src/Brouter/Tests/Bit.Brouter.Tests/GuardAndLoaderTests.cs (1)
18-26: ⚡ Quick winAssert guard blocks protected content in addition to redirecting.
Checking
nav.Urialone doesn’t prove/secretcontent never rendered.Proposed assertion expansion
- cut.WaitForAssertion(() => Assert.EndsWith("/login", nav.Uri)); + cut.WaitForAssertion(() => + { + Assert.EndsWith("/login", nav.Uri); + Assert.Single(cut.FindAll("[data-testid=login]")); + Assert.Empty(cut.FindAll("[data-testid=ok]")); + });🤖 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/Tests/Bit.Brouter.Tests/GuardAndLoaderTests.cs` around lines 18 - 26, The test Guard_can_redirect currently only asserts nav.Uri changed, but must also verify that the protected "/secret" content was not rendered and that the login content is present; after RenderComponent<GuardHost>() use the rendered component (cut) to assert the protected fragment is absent (e.g., Assert.DoesNotContain or equivalent on cut.Markup/Find output for the "/secret" text or element) and assert the login view is rendered (e.g., Assert.Contains login-related text or element in cut.Markup). Ensure you reference the test method Guard_can_redirect, the FakeNavigationManager instance nav, and the rendered component cut when adding these assertions.src/Brouter/Demo/Bit.Brouter.Demo.Core/Shared/MainLayout.razor.css (1)
1-2: 💤 Low valueConsider removing the empty CSS block.
The
.pageclass has an empty rule block that serves no purpose. You can remove it or add the intended styles.♻️ Proposed cleanup
-.page { -} - main { padding: 1rem; }🤖 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/Demo/Bit.Brouter.Demo.Core/Shared/MainLayout.razor.css` around lines 1 - 2, Remove the empty CSS rule for the .page selector in MainLayout.razor.css (the ".page { }" block) since it serves no purpose; either delete that rule entirely or replace it with the intended styles if some layout/visual rules were omitted, ensuring you update any related markup that relied on a placeholder rule.
🤖 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/Brouter/Bit.Brouter/Brouter.cs`:
- Around line 149-151: The previous CancellationTokenSource (_navCts) is being
Cancelled but not disposed, causing resource leakage; update the navigation
creation logic so that if _navCts is non-null you call _navCts.Cancel() and then
_navCts.Dispose() (or _navCts?.Dispose()) before assigning a new
CancellationTokenSource; ensure the new token is created from the fresh _navCts
and that any references to _navCts are thread-safe if accessed concurrently.
- Around line 307-310: The test for a trailing single-star is wrong: replace the
impossible expression "templateSegments.Length - segments.Length == -1 + 1" with
a check that the URL has exactly one extra segment (e.g. "segments.Length -
templateSegments.Length == 1") so lastSingleStar becomes "last.IsSingleWildcard
&& segments.Length - templateSegments.Length == 1"; keep lastCatchAll as-is and
the subsequent early return logic unchanged.
In `@src/Brouter/Bit.Brouter/BrouterConstraints.cs`:
- Around line 41-45: Unregister currently uses a case-sensitive switch on the
incoming name which lets callers bypass built-in protection (e.g., "INT") while
_factories.Remove uses a case-insensitive comparer; update Unregister (in
BrouterConstraints.cs) to perform the same case-insensitive check as
Register/Create by normalizing the name (e.g.,
ToLowerInvariant/ToUpperInvariant) or by doing a case-insensitive comparison
against the built-in names before calling _factories.Remove(name), ensuring the
built-in types
("int","bool","guid","long","float","double","decimal","datetime") cannot be
unregistered regardless of input casing and keeping behavior consistent with
Register and Create.
- Around line 11-49: The dictionary _factories is mutated without
synchronization in Register and Unregister, causing race conditions; replace the
Dictionary with a ConcurrentDictionary<string, Func<RouteConstraint>> (keeping
StringComparer.OrdinalIgnoreCase), then update Register to use TryAdd(name,
factory) and throw InvalidOperationException if TryAdd returns false, and update
Unregister to use TryRemove(name, out _) for non-built-in names; Create can
continue to use TryGetValue on the ConcurrentDictionary.
In `@src/Brouter/Bit.Brouter/BrouterLink.cs`:
- Around line 47-51: The component currently only recalculates `_isActive` in
response to navigation events (in the OnInitialized override), so when Href or
Match parameter values change the link can stay stale; add an override of
OnParametersSet or OnParametersSetAsync in the same class (and the other variant
around lines 61–72) and call UpdateActiveState() (and then
base.OnParametersSet/OnParametersSetAsync()) so the active state is recomputed
whenever component parameters change; keep the existing Brouter.OnNavigated
subscription logic intact.
In `@src/Brouter/Bit.Brouter/BrouterLocation.cs`:
- Around line 16-20: The BrouterLocation constructor stores the caller's mutable
string[] and exposes it via the Segments property, breaking immutability; fix by
making Segments an immutable/read-only view (e.g. IReadOnlyList<string> or
ReadOnlyCollection<string>), and in both constructors (the one shown and the
other overload referenced) defensively copy the incoming array
(Array.Copy/ToList().AsReadOnly() or Array.Empty() when null) and assign the
copied read-only collection to a get-only Segments property so external callers
cannot mutate internal state.
In `@src/Brouter/Bit.Brouter/NavigationContext.cs`:
- Line 43: The Redirect method currently assigns url directly to RedirectUrl,
allowing null/empty/whitespace values to flow downstream; update the public
Redirect(string url) method to validate its argument at the API boundary and
throw an appropriate exception (ArgumentNullException for null, or
ArgumentException for empty/whitespace) when url is invalid, ensuring
RedirectUrl is only set when the input passes validation and include a clear
parameter name in the exception.
In `@src/Brouter/Bit.Brouter/TemplateParser.cs`:
- Around line 69-77: The current loop only throws when a parameter appears after
an optional (it checks sawOptional && s.IsParameter), but it should reject any
non-optional segment after the first optional (including literals and
wildcards); update the condition to throw when sawOptional is true and the
current segment is not optional (e.g., change the check to sawOptional &&
!s.IsOptional) so that once sawOptional is set no subsequent segments except
optional ones are allowed, using the existing s.IsOptional, s.IsParameter,
sawOptional and template references to locate the logic in TemplateParser.cs.
In `@src/Brouter/Bit.Brouter/TemplateSegment.cs`:
- Around line 73-75: The Constraints assignment silently drops empty constraint
tokens because rest.Split uses StringSplitOptions.RemoveEmptyEntries; change the
split to include empty entries (use StringSplitOptions.None or default) and then
explicitly validate each token before calling RouteConstraint.Parse so malformed
tokens like "" from "{id::int}" or trailing ":" are detected; locate the
Constraints assignment in TemplateSegment.cs (the rest variable and Constraints
property) and call RouteConstraint.Parse(template, segment, token) only after
checking token is non-empty (or let RouteConstraint.Parse handle empties by
passing the raw token) so parsing raises on malformed constraint lists instead
of skipping them.
In `@src/Brouter/Demo/Bit.Brouter.Demo.Core/AppRouter.razor`:
- Line 48: The Razor comment in AppRouter.razor contains an extra trailing '>'
("@* ... *@>") which is being parsed as stray markup; edit the file (look for
the comment near the catch-all parameter note in AppRouter.razor) and remove the
extraneous '>' so the comment ends with "*@". Ensure no other surrounding markup
is changed.
- Around line 12-21: The OnNavigating handler is added as an inline lambda in
OnInitialized (brouter.OnNavigating) without being removed, causing handler
accumulation on component recreation; capture the handler delegate in a field
(e.g., a Func/Action or EventHandler-like variable), subscribe that stored
handler in OnInitialized and unsubscribe it in Dispose() (implement IDisposable
or override Dispose/OnDispose method to call brouter.OnNavigating -=
storedHandler), and also remove the stray '>' in the Razor comment (change '*@>'
to '*@') to fix the syntax error.
In `@src/Brouter/Demo/Bit.Brouter.Demo.Core/Pages/CounterPage.razor`:
- Line 5: The button currently calls ChangeCount?.Invoke(CurrentCount + 1) which
never triggers the two-way binding EventCallback CurrentCountChanged; change the
increment handler to update CurrentCount and invoke CurrentCountChanged (e.g.
await CurrentCountChanged.InvokeAsync(CurrentCount + 1) or equivalent) so the
bind contract is satisfied. Also remove or disable the one-time gate in the
parameter-reading code (the block that prevents re-reading the route parameter
on subsequent navigations) and move route parsing into
OnParametersSet/OnParametersSetAsync so the component always parses the route
param and assigns CurrentCount on each parameter change (optionally only when
the parsed value differs), keeping CurrentCount and CurrentCountChanged
consistent.
In `@src/Brouter/Demo/Bit.Brouter.Demo.Maui/wwwroot/index.html`:
- Line 7: Update the viewport meta tag to allow user zooming: remove the
user-scalable=no attribute (and ideally maximum-scale=1.0) from the content
string of the <meta name="viewport"> element so the tag no longer blocks
pinch-to-zoom; you can either omit those attributes entirely or set
user-scalable=yes/maximum-scale to a larger value to permit scaling.
In `@src/Brouter/Demo/Bit.Brouter.Demo.Web/wwwroot/index.html`:
- Line 7: Update the viewport meta tag (the <meta name="viewport" ...> element)
so it no longer disables user scaling: remove the user-scalable=no token from
the content attribute (and optionally the maximum-scale=1.0 token if you want to
fully allow zooming), leaving width=device-width, initial-scale=1.0 (and any
other desired tokens) so browsers can honor user zoom for accessibility.
In `@src/Brouter/README.md`:
- Around line 12-14: The fenced code block that contains the install command
`dotnet add package Bit.Brouter` is missing a language hint; update that fenced
block to include a language identifier (e.g., add `bash` after the opening
triple backticks) so the block becomes a bash code block and satisfies markdown
linting checks.
In `@src/Brouter/settings.VisualStudio.json`:
- Around line 1-2: Remove the invalid JavaScript-style comment at the top of the
JSON file so the file becomes strict JSON; locate the string "/* Visual Studio
Settings File */" in src/Brouter/settings.VisualStudio.json and delete that
entire comment line so the file starts with a JSON object token "{" and will
parse with strict JSON parsers.
---
Nitpick comments:
In `@src/Brouter/Demo/Bit.Brouter.Demo.Core/Shared/MainLayout.razor.css`:
- Around line 1-2: Remove the empty CSS rule for the .page selector in
MainLayout.razor.css (the ".page { }" block) since it serves no purpose; either
delete that rule entirely or replace it with the intended styles if some
layout/visual rules were omitted, ensuring you update any related markup that
relied on a placeholder rule.
In `@src/Brouter/Tests/Bit.Brouter.Tests/BrouterTests.cs`:
- Around line 33-47: The test
Selects_most_specific_route_when_wildcard_is_declared_first currently only
asserts that the about route is present but doesn't verify the wildcard route is
not rendered; update the assertion after rendering BrouterComp (variable cut) to
explicitly assert that the wildcard element (data-testid="star") is not
present—use the component's Find/FindAll or query API on cut to assert absence
(e.g., Assert.Throws or Assert.Empty/Assert.Null) so the test fails if both
routes render.
- Around line 50-58: The test Optional_parameter_matches_with_or_without_value
only exercises the "without value" path; add the missing "with value" path by
navigating the FakeNavigationManager to a URL that includes the optional
parameter (e.g., "/users/42" or whatever valid value your OptionalParamHost
expects), re-render or update the component as needed (using
RenderComponent<OptionalParamHost>() or calling
cut.SetParametersAndRender/trigger navigation), and add a corresponding
assertion that cut.Find("[data-testid=out]").TextContent equals the expected
output for the provided value to verify both cases.
In `@src/Brouter/Tests/Bit.Brouter.Tests/GuardAndLoaderTests.cs`:
- Around line 18-26: The test Guard_can_redirect currently only asserts nav.Uri
changed, but must also verify that the protected "/secret" content was not
rendered and that the login content is present; after
RenderComponent<GuardHost>() use the rendered component (cut) to assert the
protected fragment is absent (e.g., Assert.DoesNotContain or equivalent on
cut.Markup/Find output for the "/secret" text or element) and assert the login
view is rendered (e.g., Assert.Contains login-related text or element in
cut.Markup). Ensure you reference the test method Guard_can_redirect, the
FakeNavigationManager instance nav, and the rendered component cut when adding
these assertions.
🪄 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: 813db709-f992-4195-b563-74c08acab2a6
⛔ Files ignored due to path filters (3)
src/Brouter/Demo/Bit.Brouter.Demo.Maui/Resources/AppIcon/appicon.svgis excluded by!**/*.svgsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Resources/AppIcon/appiconfg.svgis excluded by!**/*.svgsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Resources/Splash/splash.svgis excluded by!**/*.svg
📒 Files selected for processing (85)
src/Bit.CI.Release.slnxsrc/Bit.slnxsrc/Brouter/Bit.Brouter.Web.slnfsrc/Brouter/Bit.Brouter.slnxsrc/Brouter/Bit.Brouter/Bit.Brouter.csprojsrc/Brouter/Bit.Brouter/BitBrouter.cssrc/Brouter/Bit.Brouter/Brouter.cssrc/Brouter/Bit.Brouter/BrouterConstraints.cssrc/Brouter/Bit.Brouter/BrouterLink.cssrc/Brouter/Bit.Brouter/BrouterLocation.cssrc/Brouter/Bit.Brouter/BrouterMatchMode.cssrc/Brouter/Bit.Brouter/BrouterOptions.cssrc/Brouter/Bit.Brouter/BrouterParameterAttribute.cssrc/Brouter/Bit.Brouter/BrouterScrollMode.cssrc/Brouter/Bit.Brouter/BrouterService.cssrc/Brouter/Bit.Brouter/IBrouter.cssrc/Brouter/Bit.Brouter/NavigationContext.cssrc/Brouter/Bit.Brouter/Outlet.cssrc/Brouter/Bit.Brouter/Route.cssrc/Brouter/Bit.Brouter/RouteConstraint.cssrc/Brouter/Bit.Brouter/RouteParameters.cssrc/Brouter/Bit.Brouter/RouteRenderer.cssrc/Brouter/Bit.Brouter/RouteTemplate.cssrc/Brouter/Bit.Brouter/TemplateParser.cssrc/Brouter/Bit.Brouter/TemplateSegment.cssrc/Brouter/Bit.Brouter/TypeRouteConstraint.cssrc/Brouter/Demo/Bit.Brouter.Demo.Core/AppRouter.razorsrc/Brouter/Demo/Bit.Brouter.Demo.Core/Bit.Brouter.Demo.Core.csprojsrc/Brouter/Demo/Bit.Brouter.Demo.Core/Extentions/IServiceCollectionExtensions.cssrc/Brouter/Demo/Bit.Brouter.Demo.Core/Pages/CounterPage.razorsrc/Brouter/Demo/Bit.Brouter.Demo.Core/Pages/FetchDataPage.razorsrc/Brouter/Demo/Bit.Brouter.Demo.Core/Pages/HomePage.razorsrc/Brouter/Demo/Bit.Brouter.Demo.Core/Pages/Nested.razorsrc/Brouter/Demo/Bit.Brouter.Demo.Core/Pages/Nested2.razorsrc/Brouter/Demo/Bit.Brouter.Demo.Core/Pages/PostPage.razorsrc/Brouter/Demo/Bit.Brouter.Demo.Core/Pages/ProfilePage.razorsrc/Brouter/Demo/Bit.Brouter.Demo.Core/Routes.razorsrc/Brouter/Demo/Bit.Brouter.Demo.Core/Shared/Header.razorsrc/Brouter/Demo/Bit.Brouter.Demo.Core/Shared/Header.razor.csssrc/Brouter/Demo/Bit.Brouter.Demo.Core/Shared/MainLayout.razorsrc/Brouter/Demo/Bit.Brouter.Demo.Core/Shared/MainLayout.razor.csssrc/Brouter/Demo/Bit.Brouter.Demo.Core/_Imports.razorsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/App.xamlsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/App.xaml.cssrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Bit.Brouter.Demo.Maui.csprojsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Extensions/IServiceCollectionExtensions.cssrc/Brouter/Demo/Bit.Brouter.Demo.Maui/MainPage.xamlsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/MainPage.xaml.cssrc/Brouter/Demo/Bit.Brouter.Demo.Maui/MauiProgram.cssrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/Android/AndroidManifest.xmlsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/Android/MainActivity.cssrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/Android/MainApplication.cssrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/Android/Resources/values/colors.xmlsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/MacCatalyst/AppDelegate.cssrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/MacCatalyst/Entitlements.plistsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/MacCatalyst/Info.plistsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/MacCatalyst/Program.cssrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/Tizen/Main.cssrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/Tizen/tizen-manifest.xmlsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/Windows/App.xamlsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/Windows/App.xaml.cssrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/Windows/Package.appxmanifestsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/Windows/app.manifestsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/iOS/AppDelegate.cssrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/iOS/Info.plistsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/iOS/Program.cssrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Properties/launchSettings.jsonsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/wwwroot/index.htmlsrc/Brouter/Demo/Bit.Brouter.Demo.Web/Bit.Brouter.Demo.Web.csprojsrc/Brouter/Demo/Bit.Brouter.Demo.Web/Extensions/IServiceCollectionExtensions.cssrc/Brouter/Demo/Bit.Brouter.Demo.Web/Program.cssrc/Brouter/Demo/Bit.Brouter.Demo.Web/Properties/launchSettings.jsonsrc/Brouter/Demo/Bit.Brouter.Demo.Web/wwwroot/index.htmlsrc/Brouter/Demo/Bit.Brouter.Demo.Web/wwwroot/sw.jssrc/Brouter/README.mdsrc/Brouter/Tests/Bit.Brouter.Tests/Bit.Brouter.Tests.csprojsrc/Brouter/Tests/Bit.Brouter.Tests/BrouterTests.cssrc/Brouter/Tests/Bit.Brouter.Tests/GuardAndLoaderTests.cssrc/Brouter/Tests/Bit.Brouter.Tests/GuardHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/LoaderHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/OptionalParamHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/TemplateParserTests.cssrc/Brouter/Tests/Bit.Brouter.Tests/ValueReader.razorsrc/Brouter/Tests/Bit.Brouter.Tests/_Imports.razorsrc/Brouter/settings.VisualStudio.json
Member
Author
|
@coderabbitai re-review |
|
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (1)
src/Brouter/settings.VisualStudio.json (1)
1-2:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove JSON comment to restore strict JSON validity.
Line 1 uses a JS-style comment, so strict parsers fail on this file.
Suggested patch
-/* Visual Studio Settings File */ {#!/bin/bash # Verify strict JSON parsing for this file. jq . src/Brouter/settings.VisualStudio.json >/dev/null🤖 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/settings.VisualStudio.json` around lines 1 - 2, The file contains a JavaScript-style comment "/* Visual Studio Settings File */" at the top of src/Brouter/settings.VisualStudio.json which breaks strict JSON parsing; remove that comment so the file starts with a valid JSON object (i.e., delete the "/* Visual Studio Settings File */" line and any trailing comment syntax), then validate the file with a strict parser (e.g., run jq . on settings.VisualStudio.json) to ensure the JSON is well-formed.
🧹 Nitpick comments (6)
src/Brouter/Tests/Bit.Brouter.Tests/GuardAndLoaderTests.cs (1)
18-26: ⚡ Quick winAssert rendered post-redirect state, not just URL.
Guard_can_redirectshould also verify login content is shown and protected content is not rendered.Proposed assertion hardening
- cut.WaitForAssertion(() => Assert.EndsWith("/login", nav.Uri)); + cut.WaitForAssertion(() => + { + Assert.EndsWith("/login", nav.Uri); + Assert.Equal("login", cut.Find("[data-testid=login]").TextContent); + Assert.Empty(cut.FindAll("[data-testid=ok]")); + });🤖 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/Tests/Bit.Brouter.Tests/GuardAndLoaderTests.cs` around lines 18 - 26, Improve the Guard_can_redirect test to assert the rendered page state after the redirect: after using FakeNavigationManager (nav) to NavigateTo and rendering GuardHost, wait/assert that the login UI is visible (e.g., check for the login text/element) and that the protected content is not present (e.g., assert absence of protected-page text/component). Update the test method Guard_can_redirect to include these post-redirect assertions in addition to the existing Assert.EndsWith check on nav.Uri so it verifies both URL and rendered content.src/Brouter/Tests/Bit.Brouter.Tests/BrouterTests.cs (2)
33-47: ⚡ Quick winStrengthen specificity test to reject wildcard fallback rendering.
This test currently proves
aboutrendered, but not that wildcard lost. Add a negative assertion forstarto prevent false positives.Proposed test hardening
- cut.WaitForAssertion(() => Assert.NotNull(cut.Find("[data-testid=about]"))); + cut.WaitForAssertion(() => + { + Assert.Equal("about", cut.Find("[data-testid=about]").TextContent); + Assert.Empty(cut.FindAll("[data-testid=star]")); + });🤖 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/Tests/Bit.Brouter.Tests/BrouterTests.cs` around lines 33 - 47, The test Selects_most_specific_route_when_wildcard_is_declared_first currently only asserts that the about route rendered; add a negative assertion to ensure the wildcard route did not render by asserting that the element with data-testid="star" is absent (e.g., use cut.FindAll("[data-testid=star]") and Assert.Empty(...) or assert that cut.Find(...) throws ElementNotFoundException) immediately after the existing positive assertion so the test rejects false positives where both star and about are present; update the test body around the RenderComponent<BrouterComp> usage and the cut variable to include this negative assertion.
50-58: ⚡ Quick winCover both optional-parameter paths in this test.
Method name says “with or without value”, but only the no-value case is asserted.
Proposed coverage completion
public void Optional_parameter_matches_with_or_without_value() { var nav = Services.GetRequiredService<FakeNavigationManager>(); nav.NavigateTo("http://localhost/users"); var cut = RenderComponent<OptionalParamHost>(); cut.WaitForAssertion(() => Assert.Equal("(none)", cut.Find("[data-testid=out]").TextContent)); + + nav.NavigateTo("http://localhost/users/42"); + cut.WaitForAssertion(() => Assert.Equal("42", cut.Find("[data-testid=out]").TextContent)); }🤖 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/Tests/Bit.Brouter.Tests/BrouterTests.cs` around lines 50 - 58, The test Optional_parameter_matches_with_or_without_value currently only exercises the no-value route; after the existing assertion use the same FakeNavigationManager (Services.GetRequiredService<FakeNavigationManager>()/nav) to NavigateTo a URL that includes the optional parameter (e.g. "/users/42"), then either re-render or use the existing RenderComponent<OptionalParamHost>() instance and call cut.WaitForAssertion(() => Assert.Equal("42", cut.Find("[data-testid=out]").TextContent)) to verify the with-value path; ensure you navigate to the value URL and assert the expected output from cut.Find("[data-testid=out]") so both paths are covered.src/Brouter/Bit.Brouter/Brouter.cs (1)
82-94: 💤 Low valueConsider using a separate sequence counter for the inner RenderFragment.
The
seqvariable is captured by reference in the lambda (line 86), and the same value is used for bothAddContentcalls (lines 88 and 91). While these render paths are mutually exclusive, reusing sequence numbers across renders can cause subtle Blazor diffing issues if content structure changes.Suggested improvement
builder.AddAttribute(seq++, "ChildContent", (RenderFragment)(b => { - b.AddContent(seq, ChildContent); + var innerSeq = 0; + b.AddContent(innerSeq++, ChildContent); if (_noRouteMatched && NotFoundContent is not null && string.IsNullOrEmpty(NotFound)) { - b.AddContent(seq, NotFoundContent(CurrentLocation)); + b.AddContent(innerSeq++, NotFoundContent(CurrentLocation)); } }));🤖 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/Bit.Brouter/Brouter.cs` around lines 82 - 94, The outer seq variable is captured by the ChildContent RenderFragment lambda causing the same sequence number to be reused for inner AddContent calls; introduce a separate local sequence counter inside the lambda (e.g., var inner = 0) and use inner++ for the b.AddContent calls (and any other builder calls inside the lambda) so the CascadingValue/Cascading content uses distinct sequence numbers; update the lambda in the OpenComponent<CascadingValue<Brouter>> ChildContent to reference the new inner counter while leaving the outer seq for the component attributes and Open/Close calls, preserving existing symbols like seq, ChildContent, _noRouteMatched, NotFoundContent, NotFound, and CurrentLocation.src/Brouter/Bit.Brouter/Outlet.cs (1)
41-72: 💤 Low valueConsider isolating sequence counters in nested RenderFragments.
The
seqvariable is captured and mutated across multiple nested lambdas. The sequence number forChildContent(line 66) depends on whetherContentorComponentwas rendered—it will be 8 or 9 respectively. When the matched child changes between havingContentvsComponent, Blazor may see different sequence numbers for the same logical position, potentially causing unnecessary DOM recreation.Suggested improvement
protected override void BuildRenderTree(RenderTreeBuilder builder) { base.BuildRenderTree(builder); if (_matchedChild is null) return; - var seq = 0; - builder.OpenComponent<CascadingValue<Outlet>>(seq++); - builder.AddAttribute(seq++, "Name", "Outlet"); - builder.AddAttribute(seq++, "Value", this); + builder.OpenComponent<CascadingValue<Outlet>>(0); + builder.AddAttribute(1, "Name", "Outlet"); + builder.AddAttribute(2, "Value", this); - builder.AddAttribute(seq++, "ChildContent", (RenderFragment)(b => + builder.AddAttribute(3, "ChildContent", (RenderFragment)(b => { - // Re-establish ParentRoute for any nested routes declared inside the matched child's content, - // so they can register themselves and recurse correctly. - b.OpenComponent<CascadingValue<Route>>(seq++); - b.AddAttribute(seq++, "Name", "ParentRoute"); - b.AddAttribute(seq++, "Value", _matchedChild); - b.AddAttribute(seq++, "ChildContent", (RenderFragment)(b2 => + b.OpenComponent<CascadingValue<Route>>(0); + b.AddAttribute(1, "Name", "ParentRoute"); + b.AddAttribute(2, "Value", _matchedChild); + b.AddAttribute(3, "ChildContent", (RenderFragment)(b2 => { if (_matchedChild.Content is not null) { - b2.AddContent(seq, _matchedChild.Content(_parameters)); + b2.AddContent(0, _matchedChild.Content(_parameters)); } else if (_matchedChild.Component is not null) { - b2.OpenComponent(seq++, _matchedChild.Component); + b2.OpenComponent(1, _matchedChild.Component); b2.CloseComponent(); } - // Render any descendant routes declared as ChildContent. - b2.AddContent(seq, _matchedChild.ChildContent); + b2.AddContent(2, _matchedChild.ChildContent); })); b.CloseComponent(); })); builder.CloseComponent(); }🤖 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/Bit.Brouter/Outlet.cs` around lines 41 - 72, The shared seq counter is captured by nested RenderFragment lambdas (the ChildContent and its inner fragment), causing inconsistent sequence numbers when switching between _matchedChild.Content and _matchedChild.Component; fix by giving each RenderFragment its own local sequence counter rather than capturing the outer seq: create a local int (e.g., seqOuter = 0) for the builder.OpenComponent<CascadingValue<Outlet>> lambda, and inside its ChildContent lambda use a new local counter (e.g., seqInner = 0) for the cascading Route component, and another local counter (e.g., seqInner2 = 0) for the inner RenderFragment that renders _matchedChild.Content/_matchedChild.Component and _matchedChild.ChildContent; update calls that currently use seq/seq++ to use the appropriate local counters so sequence numbers for each logical position remain stable.src/Brouter/Bit.Brouter/RouteTemplate.cs (1)
8-8: ⚡ Quick winDefensively copy
segmentsto avoid alias-driven template mutation.The constructor keeps the caller array reference, so later mutations can change routing behavior unexpectedly.
Proposed fix
public RouteTemplate(string template, TemplateSegment[] segments) { Template = template; - TemplateSegments = segments; + TemplateSegments = segments is { Length: > 0 } + ? (TemplateSegment[])segments.Clone() + : Array.Empty<TemplateSegment>(); }Also applies to: 10-13
🤖 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/Bit.Brouter/RouteTemplate.cs` at line 8, The TemplateSegments property currently exposes the same array reference passed into the RouteTemplate constructor which allows external code to mutate the template; update the RouteTemplate constructor (where TemplateSegments is assigned) to defensively copy the incoming segments array (e.g., create a new array or call ToArray()) and assign that copy to the TemplateSegments backing field/property, and likewise ensure any other assignments to TemplateSegments (lines around the same constructor/initialization) also perform a copy so external mutations cannot change internal routing state.
🤖 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/Brouter/Bit.Brouter/RouteConstraint.cs`:
- Around line 24-33: Cached RouteConstraint instances are returned without
checking for re-registration/unregistration, causing stale behavior; update the
Parse method (the block using _cache and calling BrouterConstraints.Create) to
consult BrouterConstraints.Create(constraint) first and then update or replace
the _cache entry based on the freshly created instance (or its type/identity)
instead of returning the cached value immediately, and ensure that
Register/Unregister operations (methods named Register and Unregister)
invalidate or remove the corresponding key from _cache so re-registration or
unregistration affects subsequent Parse calls.
In `@src/Brouter/Bit.Brouter/RouteParameters.cs`:
- Line 17: The constructor RouteParameters(IDictionary<string, object?> values)
currently aliases the incoming mutable dictionary; change it to snapshot the
data (e.g., create a new Dictionary<string, object?>(values) or otherwise copy
entries) and store that copy in the private _values field, and expose Values as
a read-only view (IReadOnlyDictionary<string, object?> or ReadOnlyDictionary
wrapper) so external callers cannot mutate internal state; apply the same
snapshot+read-only exposure changes to the other constructors/properties
referenced around the Values usage (lines ~20-21) so no external reference to
the original mutable dictionary is kept.
In `@src/Brouter/Bit.Brouter/RouteRenderer.cs`:
- Around line 29-73: RenderRoute currently captures and mutates the outer seq
across nested RenderFragment lambdas (b1, b2, b3), causing sequence collisions;
fix by either converting the nested RenderFragment delegates to accept and
propagate a ref int seq parameter (thread ref int seq through each lambda and
into ApplyTypedParameters) or by creating a local int localSeq = seq inside each
lambda and using/incrementing localSeq when calling builder methods, then assign
seq = localSeq at the end of the lambda—apply this change to the lambdas that
declare b1, b2, b3 and to calls to ApplyTypedParameters and any inner
OpenComponent/AddAttribute/OpenComponent calls to ensure unique sequence values.
- Around line 15-27: The seq counter is reused inside the child RenderFragment
causing duplicate sequence numbers; fix by copying seq into a fresh local (e.g.,
innerSeq = seq) before the lambda, use innerSeq for b.AddContent and pass
innerSeq to RenderRoute so nested OpenComponent/seq++ calls inside RenderRoute
consume innerSeq, then after the lambda finishes assign seq = innerSeq (so the
outer method continues with the advanced counter); update BuildRenderTree to use
innerSeq for all operations inside the lambda and then propagate its final value
back to seq.
In `@src/Brouter/Bit.Brouter/TemplateParser.cs`:
- Around line 101-107: In ValidateParameterName (TemplateParser.cs) the
normalization currently trims the constraint suffix (colon) before removing a
trailing '?' which allows embedded '?' to persist; to fix, change the
normalization order on the local variable name (derived from inner) so you first
strip the leading "**" prefix, then remove a trailing '?' (if present) to detect
optional marker correctly, and only after that find and trim the constraint
suffix via IndexOf(':'); ensure you operate on the same local variable ('name')
and preserve the existing slices so that '?' inside the middle of the raw inner
string is rejected rather than treated as the optional marker.
In `@src/Brouter/Demo/Bit.Brouter.Demo.Core/Shared/MainLayout.razor.css`:
- Around line 1-2: The CSS contains an empty rule `.page {}` which violates
block-no-empty; remove the empty `.page` block (or replace it with actual styles
if intended) from MainLayout.razor.css to satisfy the linter and prevent the
block-no-empty error.
In `@src/Brouter/README.md`:
- Line 58: Replace the mojibake sequence "—" in the optional-parameters bullet
in README.md with a proper dash character; locate the line containing the text
"**Optional parameters**: `{id?}` — must be trailing" and change the encoding
artifact to a proper em-dash (—) or hyphen-minus (-) so the rendered docs show a
correct dash.
In `@src/Brouter/Tests/Bit.Brouter.Tests/TemplateParserTests.cs`:
- Around line 64-76: The tests currently only assert a TargetInvocationException
from calling Parse, which is too broad; change each
Assert.Throws<TargetInvocationException>(() => Parse(...)) to capture the
exception (e.g., var ex = Assert.Throws<TargetInvocationException>(() =>
Parse(...))) and then assert the InnerException is the specific parser exception
type (e.g., Assert.IsType<TemplateParseException>(ex.InnerException)) so the
tests validate the parser error rather than any reflection wrapper; update
occurrences in Optionals_must_be_trailing, Duplicate_parameter_names_throw (and
similar invalid-template tests) and replace TemplateParseException with the
actual parser exception type used by Parse.
---
Duplicate comments:
In `@src/Brouter/settings.VisualStudio.json`:
- Around line 1-2: The file contains a JavaScript-style comment "/* Visual
Studio Settings File */" at the top of src/Brouter/settings.VisualStudio.json
which breaks strict JSON parsing; remove that comment so the file starts with a
valid JSON object (i.e., delete the "/* Visual Studio Settings File */" line and
any trailing comment syntax), then validate the file with a strict parser (e.g.,
run jq . on settings.VisualStudio.json) to ensure the JSON is well-formed.
---
Nitpick comments:
In `@src/Brouter/Bit.Brouter/Brouter.cs`:
- Around line 82-94: The outer seq variable is captured by the ChildContent
RenderFragment lambda causing the same sequence number to be reused for inner
AddContent calls; introduce a separate local sequence counter inside the lambda
(e.g., var inner = 0) and use inner++ for the b.AddContent calls (and any other
builder calls inside the lambda) so the CascadingValue/Cascading content uses
distinct sequence numbers; update the lambda in the
OpenComponent<CascadingValue<Brouter>> ChildContent to reference the new inner
counter while leaving the outer seq for the component attributes and Open/Close
calls, preserving existing symbols like seq, ChildContent, _noRouteMatched,
NotFoundContent, NotFound, and CurrentLocation.
In `@src/Brouter/Bit.Brouter/Outlet.cs`:
- Around line 41-72: The shared seq counter is captured by nested RenderFragment
lambdas (the ChildContent and its inner fragment), causing inconsistent sequence
numbers when switching between _matchedChild.Content and
_matchedChild.Component; fix by giving each RenderFragment its own local
sequence counter rather than capturing the outer seq: create a local int (e.g.,
seqOuter = 0) for the builder.OpenComponent<CascadingValue<Outlet>> lambda, and
inside its ChildContent lambda use a new local counter (e.g., seqInner = 0) for
the cascading Route component, and another local counter (e.g., seqInner2 = 0)
for the inner RenderFragment that renders
_matchedChild.Content/_matchedChild.Component and _matchedChild.ChildContent;
update calls that currently use seq/seq++ to use the appropriate local counters
so sequence numbers for each logical position remain stable.
In `@src/Brouter/Bit.Brouter/RouteTemplate.cs`:
- Line 8: The TemplateSegments property currently exposes the same array
reference passed into the RouteTemplate constructor which allows external code
to mutate the template; update the RouteTemplate constructor (where
TemplateSegments is assigned) to defensively copy the incoming segments array
(e.g., create a new array or call ToArray()) and assign that copy to the
TemplateSegments backing field/property, and likewise ensure any other
assignments to TemplateSegments (lines around the same
constructor/initialization) also perform a copy so external mutations cannot
change internal routing state.
In `@src/Brouter/Tests/Bit.Brouter.Tests/BrouterTests.cs`:
- Around line 33-47: The test
Selects_most_specific_route_when_wildcard_is_declared_first currently only
asserts that the about route rendered; add a negative assertion to ensure the
wildcard route did not render by asserting that the element with
data-testid="star" is absent (e.g., use cut.FindAll("[data-testid=star]") and
Assert.Empty(...) or assert that cut.Find(...) throws ElementNotFoundException)
immediately after the existing positive assertion so the test rejects false
positives where both star and about are present; update the test body around the
RenderComponent<BrouterComp> usage and the cut variable to include this negative
assertion.
- Around line 50-58: The test Optional_parameter_matches_with_or_without_value
currently only exercises the no-value route; after the existing assertion use
the same FakeNavigationManager
(Services.GetRequiredService<FakeNavigationManager>()/nav) to NavigateTo a URL
that includes the optional parameter (e.g. "/users/42"), then either re-render
or use the existing RenderComponent<OptionalParamHost>() instance and call
cut.WaitForAssertion(() => Assert.Equal("42",
cut.Find("[data-testid=out]").TextContent)) to verify the with-value path;
ensure you navigate to the value URL and assert the expected output from
cut.Find("[data-testid=out]") so both paths are covered.
In `@src/Brouter/Tests/Bit.Brouter.Tests/GuardAndLoaderTests.cs`:
- Around line 18-26: Improve the Guard_can_redirect test to assert the rendered
page state after the redirect: after using FakeNavigationManager (nav) to
NavigateTo and rendering GuardHost, wait/assert that the login UI is visible
(e.g., check for the login text/element) and that the protected content is not
present (e.g., assert absence of protected-page text/component). Update the test
method Guard_can_redirect to include these post-redirect assertions in addition
to the existing Assert.EndsWith check on nav.Uri so it verifies both URL and
rendered content.
🪄 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: 2fec6a8a-43b0-407c-a1dc-d60a0931a90a
⛔ Files ignored due to path filters (3)
src/Brouter/Demo/Bit.Brouter.Demo.Maui/Resources/AppIcon/appicon.svgis excluded by!**/*.svgsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Resources/AppIcon/appiconfg.svgis excluded by!**/*.svgsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Resources/Splash/splash.svgis excluded by!**/*.svg
📒 Files selected for processing (85)
src/Bit.CI.Release.slnxsrc/Bit.slnxsrc/Brouter/Bit.Brouter.Web.slnfsrc/Brouter/Bit.Brouter.slnxsrc/Brouter/Bit.Brouter/Bit.Brouter.csprojsrc/Brouter/Bit.Brouter/BitBrouter.cssrc/Brouter/Bit.Brouter/Brouter.cssrc/Brouter/Bit.Brouter/BrouterConstraints.cssrc/Brouter/Bit.Brouter/BrouterLink.cssrc/Brouter/Bit.Brouter/BrouterLocation.cssrc/Brouter/Bit.Brouter/BrouterMatchMode.cssrc/Brouter/Bit.Brouter/BrouterOptions.cssrc/Brouter/Bit.Brouter/BrouterParameterAttribute.cssrc/Brouter/Bit.Brouter/BrouterScrollMode.cssrc/Brouter/Bit.Brouter/BrouterService.cssrc/Brouter/Bit.Brouter/IBrouter.cssrc/Brouter/Bit.Brouter/NavigationContext.cssrc/Brouter/Bit.Brouter/Outlet.cssrc/Brouter/Bit.Brouter/Route.cssrc/Brouter/Bit.Brouter/RouteConstraint.cssrc/Brouter/Bit.Brouter/RouteParameters.cssrc/Brouter/Bit.Brouter/RouteRenderer.cssrc/Brouter/Bit.Brouter/RouteTemplate.cssrc/Brouter/Bit.Brouter/TemplateParser.cssrc/Brouter/Bit.Brouter/TemplateSegment.cssrc/Brouter/Bit.Brouter/TypeRouteConstraint.cssrc/Brouter/Demo/Bit.Brouter.Demo.Core/AppRouter.razorsrc/Brouter/Demo/Bit.Brouter.Demo.Core/Bit.Brouter.Demo.Core.csprojsrc/Brouter/Demo/Bit.Brouter.Demo.Core/Extensions/IServiceCollectionExtensions.cssrc/Brouter/Demo/Bit.Brouter.Demo.Core/Pages/CounterPage.razorsrc/Brouter/Demo/Bit.Brouter.Demo.Core/Pages/FetchDataPage.razorsrc/Brouter/Demo/Bit.Brouter.Demo.Core/Pages/HomePage.razorsrc/Brouter/Demo/Bit.Brouter.Demo.Core/Pages/Nested.razorsrc/Brouter/Demo/Bit.Brouter.Demo.Core/Pages/Nested2.razorsrc/Brouter/Demo/Bit.Brouter.Demo.Core/Pages/PostPage.razorsrc/Brouter/Demo/Bit.Brouter.Demo.Core/Pages/ProfilePage.razorsrc/Brouter/Demo/Bit.Brouter.Demo.Core/Routes.razorsrc/Brouter/Demo/Bit.Brouter.Demo.Core/Shared/Header.razorsrc/Brouter/Demo/Bit.Brouter.Demo.Core/Shared/Header.razor.csssrc/Brouter/Demo/Bit.Brouter.Demo.Core/Shared/MainLayout.razorsrc/Brouter/Demo/Bit.Brouter.Demo.Core/Shared/MainLayout.razor.csssrc/Brouter/Demo/Bit.Brouter.Demo.Core/_Imports.razorsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/App.xamlsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/App.xaml.cssrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Bit.Brouter.Demo.Maui.csprojsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Extensions/IServiceCollectionExtensions.cssrc/Brouter/Demo/Bit.Brouter.Demo.Maui/MainPage.xamlsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/MainPage.xaml.cssrc/Brouter/Demo/Bit.Brouter.Demo.Maui/MauiProgram.cssrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/Android/AndroidManifest.xmlsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/Android/MainActivity.cssrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/Android/MainApplication.cssrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/Android/Resources/values/colors.xmlsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/MacCatalyst/AppDelegate.cssrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/MacCatalyst/Entitlements.plistsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/MacCatalyst/Info.plistsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/MacCatalyst/Program.cssrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/Tizen/Main.cssrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/Tizen/tizen-manifest.xmlsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/Windows/App.xamlsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/Windows/App.xaml.cssrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/Windows/Package.appxmanifestsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/Windows/app.manifestsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/iOS/AppDelegate.cssrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/iOS/Info.plistsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/iOS/Program.cssrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Properties/launchSettings.jsonsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/wwwroot/index.htmlsrc/Brouter/Demo/Bit.Brouter.Demo.Web/Bit.Brouter.Demo.Web.csprojsrc/Brouter/Demo/Bit.Brouter.Demo.Web/Extensions/IServiceCollectionExtensions.cssrc/Brouter/Demo/Bit.Brouter.Demo.Web/Program.cssrc/Brouter/Demo/Bit.Brouter.Demo.Web/Properties/launchSettings.jsonsrc/Brouter/Demo/Bit.Brouter.Demo.Web/wwwroot/index.htmlsrc/Brouter/Demo/Bit.Brouter.Demo.Web/wwwroot/sw.jssrc/Brouter/README.mdsrc/Brouter/Tests/Bit.Brouter.Tests/Bit.Brouter.Tests.csprojsrc/Brouter/Tests/Bit.Brouter.Tests/BrouterTests.cssrc/Brouter/Tests/Bit.Brouter.Tests/GuardAndLoaderTests.cssrc/Brouter/Tests/Bit.Brouter.Tests/GuardHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/LoaderHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/OptionalParamHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/TemplateParserTests.cssrc/Brouter/Tests/Bit.Brouter.Tests/ValueReader.razorsrc/Brouter/Tests/Bit.Brouter.Tests/_Imports.razorsrc/Brouter/settings.VisualStudio.json
Member
Author
|
@coderabbitai re-review |
|
✅ Actions performedFull review triggered. |
| : StringComparison.OrdinalIgnoreCase; | ||
|
|
||
| var templateSegments = routeTemplate.TemplateSegments; | ||
| if (templateSegments.Length == 0) return false; |
Comment on lines
+33
to
+37
| builder.OpenComponent<CascadingValue<RouteParameters>>(1); | ||
| builder.AddAttribute(0, "Name", "RouteParameters"); | ||
| builder.AddAttribute(1, "Value", routeParams); | ||
| builder.AddAttribute(2, "IsFixed", false); | ||
| builder.AddAttribute(3, "ChildContent", (RenderFragment)(b1 => |
Comment on lines
+58
to
+62
| else if (_matchedChild.Component is not null) | ||
| { | ||
| b2.OpenComponent(0, _matchedChild.Component); | ||
| b2.CloseComponent(); | ||
| } |
Comment on lines
+108
to
+132
| // handler with preventDefault only in that case. We use a mousedown handler to detect | ||
| // modifier keys/non-primary buttons and conditionally disable preventDefault so that | ||
| // Ctrl+click, middle-click, etc. still trigger the browser's native "open in new tab" behavior. | ||
| if (Replace) | ||
| { | ||
| builder.AddAttribute(5, "onmousedown", Microsoft.AspNetCore.Components.EventCallback.Factory.Create<Microsoft.AspNetCore.Components.Web.MouseEventArgs>(this, OnMouseDown)); | ||
| builder.AddAttribute(6, "onclick", Microsoft.AspNetCore.Components.EventCallback.Factory.Create<Microsoft.AspNetCore.Components.Web.MouseEventArgs>(this, OnClick)); | ||
| builder.AddAttribute(7, "onclick:preventDefault", _preventDefaultClick); | ||
| } | ||
| builder.AddContent(8, ChildContent); | ||
| builder.CloseElement(); | ||
| } | ||
|
|
||
| private void OnMouseDown(Microsoft.AspNetCore.Components.Web.MouseEventArgs e) | ||
| { | ||
| // Runs before the click event. Update _preventDefaultClick so the rendered attribute | ||
| // reflects whether the upcoming click should be intercepted or left to the browser. | ||
| _preventDefaultClick = IsUnmodifiedPrimaryClick(e); | ||
| } | ||
|
|
||
| private void OnClick(Microsoft.AspNetCore.Components.Web.MouseEventArgs e) | ||
| { | ||
| // Only invoked when Replace=true (see BuildRenderTree). | ||
| // Let the browser handle modified clicks (Ctrl/Cmd+click, Shift+click, middle-click) | ||
| // so that "open in new tab" and similar native behaviors work as expected. |
Comment on lines
+1
to
+5
| /* Visual Studio Settings File */ | ||
| { | ||
| "languages.defaults.tabs.tabSize": 4, | ||
| "languages.defaults.general.lineNumbers": true, | ||
| "environment.documents.saveWithSpecificEncoding": true, |
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (2)
src/Brouter/Bit.Brouter/RouteParameters.cs (1)
15-22:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSeal
Valuesfrom external mutation by returning a read-only dictionary wrapper.Line 21 currently exposes the internal mutable dictionary instance. Consumers can cast and mutate it, breaking route parameter immutability guarantees.
Suggested patch
+using System.Collections.ObjectModel; using System.Diagnostics.CodeAnalysis; using System.Globalization; @@ - private readonly Dictionary<string, object?> _values; + private readonly IReadOnlyDictionary<string, object?> _values; @@ - internal RouteParameters(IDictionary<string, object?> values) => - _values = new Dictionary<string, object?>(values, StringComparer.OrdinalIgnoreCase); + internal RouteParameters(IDictionary<string, object?> values) + { + var snapshot = new Dictionary<string, object?>(values, StringComparer.OrdinalIgnoreCase); + _values = new ReadOnlyDictionary<string, object?>(snapshot); + } @@ - public IReadOnlyDictionary<string, object?> Values => _values; + public IReadOnlyDictionary<string, object?> Values => _values;🤖 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/Bit.Brouter/RouteParameters.cs` around lines 15 - 22, RouteParameters currently exposes its internal mutable _values via the public Values property which allows external mutation; change the Values getter to return a read-only wrapper instead of the raw _values (e.g., return a ReadOnlyDictionary over _values) so consumers cannot cast and mutate the underlying Dictionary; update the Values property implementation in the RouteParameters class to create and return an IReadOnlyDictionary wrapper around the _values field.src/Brouter/Bit.Brouter/TemplateParser.cs (1)
101-107:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNormalize
?before:to reject malformed parameter names like{id?:int}.Line 104 currently strips constraints before optional marker handling, so invalid embedded
?can be accepted. This should be reordered to preserve validation semantics.Suggested patch
var name = inner; if (name.StartsWith("**", StringComparison.Ordinal)) name = name[2..]; - var colon = name.IndexOf(':'); - if (colon >= 0) name = name[..colon]; if (name.EndsWith('?')) name = name[..^1]; + var colon = name.IndexOf(':'); + if (colon >= 0) name = name[..colon];🤖 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/Bit.Brouter/TemplateParser.cs` around lines 101 - 107, The code in TemplateParser.cs currently strips the constraint suffix (colon and following) before handling the optional marker '?' which allows malformed names like "{id?:int}" to pass; update the logic in the method that parses parameter names (the block that sets var name = inner) to normalize the trailing '?' before removing any ':...' constraint: first strip the leading "**" as now, then check and remove a trailing '?' (handle EndWith('?') -> name = name[..^1]), then locate and remove the constraint suffix by finding ':' and slicing; adjust any validation that expects '?' position accordingly so embedded '?' (e.g. in the middle of the name) are still rejected.
🧹 Nitpick comments (3)
src/Brouter/Demo/Bit.Brouter.Demo.Core/Pages/Nested.razor (1)
9-9: ⚡ Quick winPrefer router link component over raw
<a>for in-app navigation.Using a plain anchor here may bypass SPA-style navigation and router hooks in some hosts. Prefer
BrouterLink/router-native link to keep navigation in the Brouter pipeline.Suggested change
-<a href="/nested/n1/@count">"/nested/n1/@count"</a> +<BrouterLink To="@($"/nested/n1/{count}")">"/nested/n1/@count"</BrouterLink>🤖 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/Demo/Bit.Brouter.Demo.Core/Pages/Nested.razor` at line 9, The raw anchor in Nested.razor (the <a href="/nested/n1/@count">"/nested/n1/@count"</a> link) should be converted to the BrouterLink/router-native link component so navigation goes through the SPA router: replace the anchor with the BrouterLink (or your host's router link) component, bind the href/To parameter to the same route expression (keeping the "/nested/n1/@count" route generation logic), preserve the link text and any CSS/classes, and add the necessary using/import for the Brouter link component if missing so the page compiles and in-app navigation uses the Brouter pipeline.src/Brouter/Bit.Brouter/RouteTemplate.cs (1)
5-5: ⚡ Quick winAvoid mutable shared separator state.
Line 5 exposes a mutable static
char[]; accidental writes can change route parsing behavior globally. Prefer an immutable representation (e.g.,const charor a private array with no external exposure).🤖 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/Bit.Brouter/RouteTemplate.cs` at line 5, The public static mutable char[] Separators on the RouteTemplate class can be accidentally modified; change the API to expose an immutable separator instead (e.g., replace the public static readonly char[] Separators with an immutable representation such as a public const char Separator = '/' or a private static readonly char[] plus a public ReadOnlySpan<char> or IReadOnlyList<char> accessor) and update any callers that reference RouteTemplate.Separators to use the new immutable member; ensure the array is no longer directly writable from outside the class.src/Brouter/Tests/Bit.Brouter.Tests/BrouterTests.cs (1)
46-47: ⚡ Quick winTighten the specificity test to assert exclusivity.
This test should also assert the wildcard output is not rendered; otherwise it may pass even if route selection regresses to rendering multiple matches.
🤖 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/Tests/Bit.Brouter.Tests/BrouterTests.cs` around lines 46 - 47, The test in BrouterTests.cs currently only asserts that the "[data-testid=about]" element is present; tighten it to assert exclusivity by also verifying the wildcard output is not rendered (e.g., assert that the element with the wildcard test id is null/not found or that only a single matching route node exists). Update the WaitForAssertion block that calls cut.Find("[data-testid=about]") to include an additional assertion such as Assert.Null(cut.Find("[data-testid=wildcard]")) or an assertion that exactly one route element is present so multiple matches cannot pass the test.
🤖 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/Brouter/Bit.Brouter/Brouter.cs`:
- Around line 285-287: The early return that rejects templates with zero
segments prevents the root path from matching; instead of unconditionally
returning false when routeTemplate.TemplateSegments.Length == 0, check if the
incoming request path represents the root (e.g., request path is "/" or has zero
parsed segments) and return true for a match in that case. Locate the code that
reads templateSegments from routeTemplate.TemplateSegments and replace the
unconditional "if (templateSegments.Length == 0) return false;" with logic that
verifies whether the current request's segment count (or raw path) indicates
root and only returns false when the template is empty but the request is not
root.
In `@src/Brouter/Bit.Brouter/BrouterLink.cs`:
- Around line 81-90: NormalisePath is treating absolute URLs like simple strings
(producing "/https://..."), so update it to detect and handle absolute URLs:
inside NormalisePath(string href) use Uri.TryCreate(href, UriKind.Absolute, out
var uri) and, when true, take uri.AbsolutePath (or uri.LocalPath) as the path to
normalise instead of the raw href; then apply the existing trimming of trailing
'/' (unless root) and ensure a leading '/' as before. Keep the existing
fragment/query stripping logic for non-absolute inputs and fall back to the
original behaviour when Uri.TryCreate returns false.
In `@src/Brouter/Bit.Brouter/BrouterService.cs`:
- Around line 52-61: BackAsync currently catches JSDisconnectedException,
InvalidOperationException, and TaskCanceledException but not JSException, so
background calls to _js.InvokeVoidAsync("history.back") can leave unobserved
faults; add a catch (JSException) { /* Circuit disconnected; nothing to do. */ }
(or same no-op policy used for the other JS-related catches) to the BackAsync
method to swallow interop errors and prevent unobserved task exceptions.
In `@src/Brouter/Bit.Brouter/Outlet.cs`:
- Around line 58-61: The component-based child route rendering omits passing
route parameters: when _matchedChild.Component is not null the call sequence
uses b2.OpenComponent(0, _matchedChild.Component); b2.CloseComponent(); — update
this to pass the existing _parameters into the OpenComponent call (same as
content routes use) so nested component routes receive parameter bindings;
locate the OpenComponent invocation around _matchedChild.Component and add the
_parameters argument.
In `@src/Brouter/Demo/Bit.Brouter.Demo.Core/Shared/Header.razor.css`:
- Line 5: Add a localized stylelint suppression immediately above the rule that
uses the Blazor-specific pseudo-element to silence
selector-pseudo-element-no-unknown; for the selector ".container ::deep
a.active" in Header.razor.css, insert a comment like "/*
stylelint-disable-next-line selector-pseudo-element-no-unknown */" directly
above the ".container ::deep a.active { ... }" rule so only that selector is
ignored by stylelint.
In `@src/Brouter/Demo/Bit.Brouter.Demo.Web/wwwroot/index.html`:
- Around line 13-15: navigator.serviceWorker.register('/sw.js') is invoked
unguarded and may throw if service workers are unsupported or registration is
blocked; update the <script> to first check 'navigator' and
'navigator.serviceWorker' presence, then call
navigator.serviceWorker.register('/sw.js') inside an async block and handle
rejection via .catch or try/catch to log or silently ignore failures. Reference
the navigator.serviceWorker.register call and the '/sw.js' registration target
when making the change so the registration is only attempted on supported
browsers and any errors are handled gracefully.
---
Duplicate comments:
In `@src/Brouter/Bit.Brouter/RouteParameters.cs`:
- Around line 15-22: RouteParameters currently exposes its internal mutable
_values via the public Values property which allows external mutation; change
the Values getter to return a read-only wrapper instead of the raw _values
(e.g., return a ReadOnlyDictionary over _values) so consumers cannot cast and
mutate the underlying Dictionary; update the Values property implementation in
the RouteParameters class to create and return an IReadOnlyDictionary wrapper
around the _values field.
In `@src/Brouter/Bit.Brouter/TemplateParser.cs`:
- Around line 101-107: The code in TemplateParser.cs currently strips the
constraint suffix (colon and following) before handling the optional marker '?'
which allows malformed names like "{id?:int}" to pass; update the logic in the
method that parses parameter names (the block that sets var name = inner) to
normalize the trailing '?' before removing any ':...' constraint: first strip
the leading "**" as now, then check and remove a trailing '?' (handle
EndWith('?') -> name = name[..^1]), then locate and remove the constraint suffix
by finding ':' and slicing; adjust any validation that expects '?' position
accordingly so embedded '?' (e.g. in the middle of the name) are still rejected.
---
Nitpick comments:
In `@src/Brouter/Bit.Brouter/RouteTemplate.cs`:
- Line 5: The public static mutable char[] Separators on the RouteTemplate class
can be accidentally modified; change the API to expose an immutable separator
instead (e.g., replace the public static readonly char[] Separators with an
immutable representation such as a public const char Separator = '/' or a
private static readonly char[] plus a public ReadOnlySpan<char> or
IReadOnlyList<char> accessor) and update any callers that reference
RouteTemplate.Separators to use the new immutable member; ensure the array is no
longer directly writable from outside the class.
In `@src/Brouter/Demo/Bit.Brouter.Demo.Core/Pages/Nested.razor`:
- Line 9: The raw anchor in Nested.razor (the <a
href="/nested/n1/@count">"/nested/n1/@count"</a> link) should be converted to
the BrouterLink/router-native link component so navigation goes through the SPA
router: replace the anchor with the BrouterLink (or your host's router link)
component, bind the href/To parameter to the same route expression (keeping the
"/nested/n1/@count" route generation logic), preserve the link text and any
CSS/classes, and add the necessary using/import for the Brouter link component
if missing so the page compiles and in-app navigation uses the Brouter pipeline.
In `@src/Brouter/Tests/Bit.Brouter.Tests/BrouterTests.cs`:
- Around line 46-47: The test in BrouterTests.cs currently only asserts that the
"[data-testid=about]" element is present; tighten it to assert exclusivity by
also verifying the wildcard output is not rendered (e.g., assert that the
element with the wildcard test id is null/not found or that only a single
matching route node exists). Update the WaitForAssertion block that calls
cut.Find("[data-testid=about]") to include an additional assertion such as
Assert.Null(cut.Find("[data-testid=wildcard]")) or an assertion that exactly one
route element is present so multiple matches cannot pass the test.
🪄 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: ff4cc4f9-9c19-4393-97b1-2fe4a86595f5
⛔ Files ignored due to path filters (3)
src/Brouter/Demo/Bit.Brouter.Demo.Maui/Resources/AppIcon/appicon.svgis excluded by!**/*.svgsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Resources/AppIcon/appiconfg.svgis excluded by!**/*.svgsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Resources/Splash/splash.svgis excluded by!**/*.svg
📒 Files selected for processing (85)
src/Bit.CI.Release.slnxsrc/Bit.slnxsrc/Brouter/Bit.Brouter.Web.slnfsrc/Brouter/Bit.Brouter.slnxsrc/Brouter/Bit.Brouter/Bit.Brouter.csprojsrc/Brouter/Bit.Brouter/BitBrouter.cssrc/Brouter/Bit.Brouter/Brouter.cssrc/Brouter/Bit.Brouter/BrouterConstraints.cssrc/Brouter/Bit.Brouter/BrouterLink.cssrc/Brouter/Bit.Brouter/BrouterLocation.cssrc/Brouter/Bit.Brouter/BrouterMatchMode.cssrc/Brouter/Bit.Brouter/BrouterOptions.cssrc/Brouter/Bit.Brouter/BrouterParameterAttribute.cssrc/Brouter/Bit.Brouter/BrouterScrollMode.cssrc/Brouter/Bit.Brouter/BrouterService.cssrc/Brouter/Bit.Brouter/IBrouter.cssrc/Brouter/Bit.Brouter/NavigationContext.cssrc/Brouter/Bit.Brouter/Outlet.cssrc/Brouter/Bit.Brouter/Route.cssrc/Brouter/Bit.Brouter/RouteConstraint.cssrc/Brouter/Bit.Brouter/RouteParameters.cssrc/Brouter/Bit.Brouter/RouteRenderer.cssrc/Brouter/Bit.Brouter/RouteTemplate.cssrc/Brouter/Bit.Brouter/TemplateParser.cssrc/Brouter/Bit.Brouter/TemplateSegment.cssrc/Brouter/Bit.Brouter/TypeRouteConstraint.cssrc/Brouter/Demo/Bit.Brouter.Demo.Core/AppRouter.razorsrc/Brouter/Demo/Bit.Brouter.Demo.Core/Bit.Brouter.Demo.Core.csprojsrc/Brouter/Demo/Bit.Brouter.Demo.Core/Extensions/IServiceCollectionExtensions.cssrc/Brouter/Demo/Bit.Brouter.Demo.Core/Pages/CounterPage.razorsrc/Brouter/Demo/Bit.Brouter.Demo.Core/Pages/FetchDataPage.razorsrc/Brouter/Demo/Bit.Brouter.Demo.Core/Pages/HomePage.razorsrc/Brouter/Demo/Bit.Brouter.Demo.Core/Pages/Nested.razorsrc/Brouter/Demo/Bit.Brouter.Demo.Core/Pages/Nested2.razorsrc/Brouter/Demo/Bit.Brouter.Demo.Core/Pages/PostPage.razorsrc/Brouter/Demo/Bit.Brouter.Demo.Core/Pages/ProfilePage.razorsrc/Brouter/Demo/Bit.Brouter.Demo.Core/Routes.razorsrc/Brouter/Demo/Bit.Brouter.Demo.Core/Shared/Header.razorsrc/Brouter/Demo/Bit.Brouter.Demo.Core/Shared/Header.razor.csssrc/Brouter/Demo/Bit.Brouter.Demo.Core/Shared/MainLayout.razorsrc/Brouter/Demo/Bit.Brouter.Demo.Core/Shared/MainLayout.razor.csssrc/Brouter/Demo/Bit.Brouter.Demo.Core/_Imports.razorsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/App.xamlsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/App.xaml.cssrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Bit.Brouter.Demo.Maui.csprojsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Extensions/IServiceCollectionExtensions.cssrc/Brouter/Demo/Bit.Brouter.Demo.Maui/MainPage.xamlsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/MainPage.xaml.cssrc/Brouter/Demo/Bit.Brouter.Demo.Maui/MauiProgram.cssrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/Android/AndroidManifest.xmlsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/Android/MainActivity.cssrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/Android/MainApplication.cssrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/Android/Resources/values/colors.xmlsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/MacCatalyst/AppDelegate.cssrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/MacCatalyst/Entitlements.plistsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/MacCatalyst/Info.plistsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/MacCatalyst/Program.cssrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/Tizen/Main.cssrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/Tizen/tizen-manifest.xmlsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/Windows/App.xamlsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/Windows/App.xaml.cssrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/Windows/Package.appxmanifestsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/Windows/app.manifestsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/iOS/AppDelegate.cssrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/iOS/Info.plistsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/iOS/Program.cssrc/Brouter/Demo/Bit.Brouter.Demo.Maui/Properties/launchSettings.jsonsrc/Brouter/Demo/Bit.Brouter.Demo.Maui/wwwroot/index.htmlsrc/Brouter/Demo/Bit.Brouter.Demo.Web/Bit.Brouter.Demo.Web.csprojsrc/Brouter/Demo/Bit.Brouter.Demo.Web/Extensions/IServiceCollectionExtensions.cssrc/Brouter/Demo/Bit.Brouter.Demo.Web/Program.cssrc/Brouter/Demo/Bit.Brouter.Demo.Web/Properties/launchSettings.jsonsrc/Brouter/Demo/Bit.Brouter.Demo.Web/wwwroot/index.htmlsrc/Brouter/Demo/Bit.Brouter.Demo.Web/wwwroot/sw.jssrc/Brouter/README.mdsrc/Brouter/Tests/Bit.Brouter.Tests/Bit.Brouter.Tests.csprojsrc/Brouter/Tests/Bit.Brouter.Tests/BrouterTests.cssrc/Brouter/Tests/Bit.Brouter.Tests/GuardAndLoaderTests.cssrc/Brouter/Tests/Bit.Brouter.Tests/GuardHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/LoaderHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/OptionalParamHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/TemplateParserTests.cssrc/Brouter/Tests/Bit.Brouter.Tests/ValueReader.razorsrc/Brouter/Tests/Bit.Brouter.Tests/_Imports.razorsrc/Brouter/settings.VisualStudio.json
Comment on lines
+285
to
+287
| var templateSegments = routeTemplate.TemplateSegments; | ||
| if (templateSegments.Length == 0) return false; | ||
|
|
There was a problem hiding this comment.
Root path ("/") cannot match due to empty-segment early return.
Line 286 unconditionally rejects templates with zero segments. Since "/" parses to zero segments, root routes (e.g., redirect from /) never match.
Suggested fix
- if (templateSegments.Length == 0) return false;
+ if (templateSegments.Length == 0) return segments.Length == 0;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| var templateSegments = routeTemplate.TemplateSegments; | |
| if (templateSegments.Length == 0) return false; | |
| var templateSegments = routeTemplate.TemplateSegments; | |
| if (templateSegments.Length == 0) return segments.Length == 0; |
🤖 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/Bit.Brouter/Brouter.cs` around lines 285 - 287, The early return
that rejects templates with zero segments prevents the root path from matching;
instead of unconditionally returning false when
routeTemplate.TemplateSegments.Length == 0, check if the incoming request path
represents the root (e.g., request path is "/" or has zero parsed segments) and
return true for a match in that case. Locate the code that reads
templateSegments from routeTemplate.TemplateSegments and replace the
unconditional "if (templateSegments.Length == 0) return false;" with logic that
verifies whether the current request's segment count (or raw path) indicates
root and only returns false when the template is empty but the request is not
root.
Comment on lines
+81
to
+90
| private static string NormalisePath(string href) | ||
| { | ||
| var path = href; | ||
| var hashIdx = path.IndexOf('#'); | ||
| if (hashIdx >= 0) path = path[..hashIdx]; | ||
| var qIdx = path.IndexOf('?'); | ||
| if (qIdx >= 0) path = path[..qIdx]; | ||
| if (path.Length > 1 && path.EndsWith('/')) path = path[..^1]; | ||
| if (path.Length == 0 || path[0] != '/') path = "/" + path; | ||
| return path; |
There was a problem hiding this comment.
Handle absolute URLs correctly in NormalisePath.
At Line 81–90, absolute URLs are treated as plain strings and converted to invalid targets (e.g., /https://...), so active matching breaks despite the API allowing “URL or path”.
Proposed fix
private static string NormalisePath(string href)
{
- var path = href;
+ var path = href;
+ if (Uri.TryCreate(href, UriKind.Absolute, out var absolute))
+ {
+ path = absolute.PathAndQuery + absolute.Fragment;
+ }
+
var hashIdx = path.IndexOf('#');
if (hashIdx >= 0) path = path[..hashIdx];
var qIdx = path.IndexOf('?');
if (qIdx >= 0) path = path[..qIdx];
if (path.Length > 1 && path.EndsWith('/')) path = path[..^1];
if (path.Length == 0 || path[0] != '/') path = "/" + path;
return path;
}🤖 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/Bit.Brouter/BrouterLink.cs` around lines 81 - 90, NormalisePath
is treating absolute URLs like simple strings (producing "/https://..."), so
update it to detect and handle absolute URLs: inside NormalisePath(string href)
use Uri.TryCreate(href, UriKind.Absolute, out var uri) and, when true, take
uri.AbsolutePath (or uri.LocalPath) as the path to normalise instead of the raw
href; then apply the existing trimming of trailing '/' (unless root) and ensure
a leading '/' as before. Keep the existing fragment/query stripping logic for
non-absolute inputs and fall back to the original behaviour when Uri.TryCreate
returns false.
Comment on lines
+52
to
+61
| private async Task BackAsync() | ||
| { | ||
| try | ||
| { | ||
| await _js.InvokeVoidAsync("history.back").ConfigureAwait(false); | ||
| } | ||
| catch (JSDisconnectedException) { /* Circuit disconnected; nothing to do. */ } | ||
| catch (InvalidOperationException) { /* JS interop not available during pre-render. */ } | ||
| catch (TaskCanceledException) { /* Component disposed mid-call. */ } | ||
| } |
There was a problem hiding this comment.
Handle JSException in BackAsync to avoid unobserved task faults.
Line 49 fires BackAsync without awaiting, but Lines 58–60 don’t catch JSException. A browser/interop failure can leave a faulted background task. Add a catch (JSException) branch with the same no-op policy.
Suggested patch
private async Task BackAsync()
{
try
{
await _js.InvokeVoidAsync("history.back").ConfigureAwait(false);
}
+ catch (JSException) { /* JS interop runtime/browser error; best-effort back. */ }
catch (JSDisconnectedException) { /* Circuit disconnected; nothing to do. */ }
catch (InvalidOperationException) { /* JS interop not available during pre-render. */ }
catch (TaskCanceledException) { /* Component disposed mid-call. */ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private async Task BackAsync() | |
| { | |
| try | |
| { | |
| await _js.InvokeVoidAsync("history.back").ConfigureAwait(false); | |
| } | |
| catch (JSDisconnectedException) { /* Circuit disconnected; nothing to do. */ } | |
| catch (InvalidOperationException) { /* JS interop not available during pre-render. */ } | |
| catch (TaskCanceledException) { /* Component disposed mid-call. */ } | |
| } | |
| private async Task BackAsync() | |
| { | |
| try | |
| { | |
| await _js.InvokeVoidAsync("history.back").ConfigureAwait(false); | |
| } | |
| catch (JSException) { /* JS interop runtime/browser error; best-effort back. */ } | |
| catch (JSDisconnectedException) { /* Circuit disconnected; nothing to do. */ } | |
| catch (InvalidOperationException) { /* JS interop not available during pre-render. */ } | |
| catch (TaskCanceledException) { /* Component disposed mid-call. */ } | |
| } |
🤖 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/Bit.Brouter/BrouterService.cs` around lines 52 - 61, BackAsync
currently catches JSDisconnectedException, InvalidOperationException, and
TaskCanceledException but not JSException, so background calls to
_js.InvokeVoidAsync("history.back") can leave unobserved faults; add a catch
(JSException) { /* Circuit disconnected; nothing to do. */ } (or same no-op
policy used for the other JS-related catches) to the BackAsync method to swallow
interop errors and prevent unobserved task exceptions.
Comment on lines
+58
to
+61
| else if (_matchedChild.Component is not null) | ||
| { | ||
| b2.OpenComponent(0, _matchedChild.Component); | ||
| b2.CloseComponent(); |
There was a problem hiding this comment.
Pass route parameters when rendering component-based child routes.
At Line 58–61, component routes are rendered without _parameters, while content routes receive them (Line 56). This can break parameter binding for nested component routes.
🤖 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/Bit.Brouter/Outlet.cs` around lines 58 - 61, The component-based
child route rendering omits passing route parameters: when
_matchedChild.Component is not null the call sequence uses b2.OpenComponent(0,
_matchedChild.Component); b2.CloseComponent(); — update this to pass the
existing _parameters into the OpenComponent call (same as content routes use) so
nested component routes receive parameter bindings; locate the OpenComponent
invocation around _matchedChild.Component and add the _parameters argument.
| padding: 0.5rem 1rem; | ||
| } | ||
|
|
||
| .container ::deep a.active { |
There was a problem hiding this comment.
Resolve the stylelint failure for ::deep.
Line 5 currently violates selector-pseudo-element-no-unknown, which can fail lint/CI. Add a local suppression for this Blazor-specific selector (or update stylelint config globally).
Suggested localized fix
+/* stylelint-disable-next-line selector-pseudo-element-no-unknown */
.container ::deep a.active {
font-weight: bold;
text-decoration: underline;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| .container ::deep a.active { | |
| /* stylelint-disable-next-line selector-pseudo-element-no-unknown */ | |
| .container ::deep a.active { | |
| font-weight: bold; | |
| text-decoration: underline; | |
| } |
🧰 Tools
🪛 Stylelint (17.11.1)
[error] 5-5: Unknown pseudo-element selector "::deep" (selector-pseudo-element-no-unknown)
(selector-pseudo-element-no-unknown)
🤖 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/Demo/Bit.Brouter.Demo.Core/Shared/Header.razor.css` at line 5,
Add a localized stylelint suppression immediately above the rule that uses the
Blazor-specific pseudo-element to silence selector-pseudo-element-no-unknown;
for the selector ".container ::deep a.active" in Header.razor.css, insert a
comment like "/* stylelint-disable-next-line selector-pseudo-element-no-unknown
*/" directly above the ".container ::deep a.active { ... }" rule so only that
selector is ignored by stylelint.
Comment on lines
+13
to
+15
| <script> | ||
| navigator.serviceWorker.register('/sw.js'); | ||
| </script> |
There was a problem hiding this comment.
Guard service worker registration and handle rejected registration.
Directly calling navigator.serviceWorker.register can throw where service workers are unavailable/blocked. Add capability check + error handling to avoid noisy runtime failures.
Proposed fix
- <script>
- navigator.serviceWorker.register('/sw.js');
- </script>
+ <script>
+ if ('serviceWorker' in navigator) {
+ navigator.serviceWorker.register('/sw.js').catch(console.error);
+ }
+ </script>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <script> | |
| navigator.serviceWorker.register('/sw.js'); | |
| </script> | |
| <script> | |
| if ('serviceWorker' in navigator) { | |
| navigator.serviceWorker.register('/sw.js').catch(console.error); | |
| } | |
| </script> |
🤖 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/Demo/Bit.Brouter.Demo.Web/wwwroot/index.html` around lines 13 -
15, navigator.serviceWorker.register('/sw.js') is invoked unguarded and may
throw if service workers are unsupported or registration is blocked; update the
<script> to first check 'navigator' and 'navigator.serviceWorker' presence, then
call navigator.serviceWorker.register('/sw.js') inside an async block and handle
rejection via .catch or try/catch to log or silently ignore failures. Reference
the navigator.serviceWorker.register call and the '/sw.js' registration target
when making the change so the registration is only attempted on supported
browsers and any errors are handled gracefully.
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 #9923
Summary by CodeRabbit
New Features
Documentation
Demos
Tests