Add Brouter (#9923)#12361
Open
msynk wants to merge 36 commits into
Open
Conversation
msynk
requested review from
Cyrus-Sushiant,
Copilot,
mhrastegari,
yasmoradi and
zahra-yousefi
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds Bit.Brouter core library, DI registration, parsing/matching/renderer, IBrouter service, BrouterLink and JS helper, demo apps (WASM + MAUI), solution filters, extensive bUnit/MSTest suites, and README documentation. ChangesBit.Brouter end-to-end introduction
Sequence Diagram(s)sequenceDiagram
participant Browser
participant NavigationManager
participant Brouter
participant BrouterService
participant JSRuntime
Browser->>NavigationManager: User clicks / address change
NavigationManager->>Brouter: LocationChanged (Uri)
Brouter->>BrouterService: InvokeOnNavigating(ctx)
Brouter->>Brouter: Match routes, run guards, run loaders
Brouter->>BrouterService: InvokeOnNavigated(ctx)
Brouter->>JSRuntime: ApplyScrollAsync / history.back (if needed)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
✨ 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. |
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
Member
Author
|
@coderabbitai re-review |
|
✅ Actions performedFull review triggered. |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 86 out of 89 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (1)
src/Brouter/.editorconfig:18
- This file is marked
root = true, which prevents the repository-widesrc/.editorconfigfrom applying undersrc/Brouter(e.g., charset and C#/.NET formatting rules). If the intent is to extend repo conventions, removeroot = trueand only override the handful of settings that need to differ; otherwise, consider duplicating the necessary .NET rules here to avoid inconsistent formatting within the repo.
root = true
# Defaults for every file
[*]
charset = utf-8
end_of_line = crlf
insert_final_newline = true
trim_trailing_whitespace = true
indent_style = space
indent_size = 4
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/Brouter/Demo/Bit.Brouter.Demo.Maui/Bit.Brouter.Demo.Maui.csproj (1)
55-55:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUpdate unavailable NuGet dependency: Microsoft.Extensions.Logging.Debug 10.0.8
The referenced version 10.0.8 of
Microsoft.Extensions.Logging.Debugis not available on NuGet as of May 2026. Update to an available version (e.g., 10.0.7) or align all related packages to a consistent set of available versions.🤖 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.Maui/Bit.Brouter.Demo.Maui.csproj` at line 55, The PackageReference for Microsoft.Extensions.Logging.Debug points to an unavailable version 10.0.8; update the PackageReference for Microsoft.Extensions.Logging.Debug in the project file to an available version (for example 10.0.7) or adjust it to match the other Microsoft.Extensions.* package versions used in the repo so dependencies stay consistent; edit the <PackageReference Include="Microsoft.Extensions.Logging.Debug" Version="..."/> entry to the chosen available version and restore packages to verify resolution.
🧹 Nitpick comments (1)
src/Brouter/Demo/Bit.Brouter.Demo.Core/Pages/Nested2.razor (1)
9-11: 💤 Low valueConsider using BrouterLink for consistency.
The links use plain HTML
<a>tags, whereas the similarNested.razordemo page uses<BrouterLink>components. For consistency across demo pages and to showcase the framework's link component, consider switching toBrouterLink.♻️ Proposed refactor
-<a href="/nested2/n1/@count">/nested2/n1/@count</a> +<BrouterLink Href="@($"/nested2/n1/{count}")">/nested2/n1/@count</BrouterLink> <br /> -<a href="/nested2/n2/@count">/nested2/n2/@count</a> +<BrouterLink Href="@($"/nested2/n2/{count}")">/nested2/n2/@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/Nested2.razor` around lines 9 - 11, Replace the plain anchor tags in Nested2.razor with the framework link component to match Nested.razor: change the two <a href="/nested2/n1/@count"> and <a href="/nested2/n2/@count"> usages to <BrouterLink> usages (e.g., use the To parameter with the same route string that includes the count token and put the link text as ChildContent) so the demo uses BrouterLink consistently; ensure you import or reference the BrouterLink component if required by the page.
🤖 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/Tests/Bit.Brouter.Tests/ValueReader.razor`:
- Line 4: The cascading parameter property Data on ValueReader.razor is missing
an access modifier so Blazor cannot set it; update the property declaration for
Data (the [CascadingParameter(Name = "RouteData")] object? Data { get; set; })
to be public so the framework can inject the cascading value—locate the Data
property in ValueReader.razor and add the public access modifier to the
automatic property.
---
Duplicate comments:
In `@src/Brouter/Demo/Bit.Brouter.Demo.Maui/Bit.Brouter.Demo.Maui.csproj`:
- Line 55: The PackageReference for Microsoft.Extensions.Logging.Debug points to
an unavailable version 10.0.8; update the PackageReference for
Microsoft.Extensions.Logging.Debug in the project file to an available version
(for example 10.0.7) or adjust it to match the other Microsoft.Extensions.*
package versions used in the repo so dependencies stay consistent; edit the
<PackageReference Include="Microsoft.Extensions.Logging.Debug" Version="..."/>
entry to the chosen available version and restore packages to verify resolution.
---
Nitpick comments:
In `@src/Brouter/Demo/Bit.Brouter.Demo.Core/Pages/Nested2.razor`:
- Around line 9-11: Replace the plain anchor tags in Nested2.razor with the
framework link component to match Nested.razor: change the two <a
href="/nested2/n1/@count"> and <a href="/nested2/n2/@count"> usages to
<BrouterLink> usages (e.g., use the To parameter with the same route string that
includes the count token and put the link text as ChildContent) so the demo uses
BrouterLink consistently; ensure you import or reference the BrouterLink
component if required by the page.
🪄 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: 28c1a461-5db9-4fd3-8594-f4809d6802d0
⛔ 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 (94)
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/BrouterNavigationContext.cssrc/Brouter/Bit.Brouter/BrouterOptions.cssrc/Brouter/Bit.Brouter/BrouterOutlet.cssrc/Brouter/Bit.Brouter/BrouterParameterAttribute.cssrc/Brouter/Bit.Brouter/BrouterRoute.cssrc/Brouter/Bit.Brouter/BrouterRouteConstraint.cssrc/Brouter/Bit.Brouter/BrouterRouteParameters.cssrc/Brouter/Bit.Brouter/BrouterRouteRenderer.cssrc/Brouter/Bit.Brouter/BrouterRouteTemplate.cssrc/Brouter/Bit.Brouter/BrouterScrollMode.cssrc/Brouter/Bit.Brouter/BrouterService.cssrc/Brouter/Bit.Brouter/BrouterTemplateParser.cssrc/Brouter/Bit.Brouter/BrouterTemplateSegment.cssrc/Brouter/Bit.Brouter/BrouterTypeRouteConstraint.cssrc/Brouter/Bit.Brouter/IBrouter.cssrc/Brouter/Bit.Brouter/wwwroot/BitBrouter.jssrc/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/BrouterLinkTests.cssrc/Brouter/Tests/Bit.Brouter.Tests/BrouterTests.cssrc/Brouter/Tests/Bit.Brouter.Tests/BunitTestContext.cssrc/Brouter/Tests/Bit.Brouter.Tests/GuardAndLoaderTests.cssrc/Brouter/Tests/Bit.Brouter.Tests/GuardHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/LinkHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/LoaderHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/NamedRouteHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/OptionalParamHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/QueryBindingTests.cssrc/Brouter/Tests/Bit.Brouter.Tests/QueryConsumer.razorsrc/Brouter/Tests/Bit.Brouter.Tests/QueryHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/ResolveUrlTests.cssrc/Brouter/Tests/Bit.Brouter.Tests/SimpleHomeHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/SpecificityHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/TemplateParserTests.cssrc/Brouter/Tests/Bit.Brouter.Tests/ValueReader.razorsrc/Brouter/Tests/Bit.Brouter.Tests/_Imports.razor
Member
Author
|
@coderabbitai re-review |
|
✅ Actions performedFull review triggered. |
Member
Author
|
@coderabbitai re-review |
|
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/Brouter/Bit.Brouter/BrouterOutlet.cs (1)
66-72:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftAvoid potential render-tree sequence reuse after typed-parameter emission.
b2.AddContent(1, ...)can collide with sequences emitted insideBrouterRouteRenderer.ApplyTypedParameters(...)in the same render region. Please advance sequence allocation consistently (e.g., haveApplyTypedParametersaccept/returnseq, then use the returned value forAddContent).#!/bin/bash # Verify whether ApplyTypedParameters emits attribute/content frames starting from 1 # and whether BrouterOutlet reuses sequence 1 afterward. rg -n -C4 'ApplyTypedParameters\s*\(' src/Brouter/Bit.Brouter/BrouterRouteRenderer.cs rg -n -C4 'OpenComponent\(0, _matchedChild.Component\)|ApplyTypedParameters|AddContent\(1, _matchedChild.ChildContent\)' src/Brouter/Bit.Brouter/BrouterOutlet.cs🤖 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/BrouterOutlet.cs` around lines 66 - 72, The render sequence 1 used by b2.AddContent(1, _matchedChild.ChildContent) may collide with sequence values emitted inside BrouterRouteRenderer.ApplyTypedParameters; change ApplyTypedParameters (BrouterRouteRenderer.ApplyTypedParameters) to accept a starting sequence integer and return the next available sequence, call it from BrouterOutlet passing the current seq after OpenComponent/CloseComponent, capture the returned seq, and use that returned value when calling AddContent for _matchedChild.ChildContent so sequence allocation is advanced consistently.
🧹 Nitpick comments (1)
src/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/MacCatalyst/Info.plist (1)
9-14: ⚡ Quick winRemove unused iPhone orientation settings.
UIDeviceFamilyis set to iPad-only (value2), so the iPhone-specificUISupportedInterfaceOrientationskey (lines 9-14) is unused. Only the~ipadvariant applies.♻️ Proposed fix to remove unused configuration
- <key>UISupportedInterfaceOrientations</key> - <array> - <string>UIInterfaceOrientationPortrait</string> - <string>UIInterfaceOrientationLandscapeLeft</string> - <string>UIInterfaceOrientationLandscapeRight</string> - </array> <key>UISupportedInterfaceOrientations~ipad</key>🤖 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.Maui/Platforms/MacCatalyst/Info.plist` around lines 9 - 14, Remove the unused iPhone orientation settings by deleting the UISupportedInterfaceOrientations array block (the <key>UISupportedInterfaceOrientations</key> ... </array>) from the MacCatalyst Info.plist since UIDeviceFamily is set to iPad-only (value 2) and only the ~ipad variant applies; ensure no other code or plist entries reference UISupportedInterfaceOrientations so the file remains valid after removal.
🤖 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/BrouterNavigationContext.cs`:
- Around line 39-49: Cancel() and Redirect(string url) can currently both be
set, creating ambiguous navigation state; make them mutually exclusive by
updating Cancel() to clear any existing RedirectUrl (set RedirectUrl = null) and
updating Redirect(string url) to first validate url then throw an
InvalidOperationException if IsCancelled is true (or alternatively clear
IsCancelled if you prefer state override) so you cannot set a redirect on a
cancelled context; reference Cancel(), Redirect(string url), IsCancelled and
RedirectUrl when making the change.
---
Duplicate comments:
In `@src/Brouter/Bit.Brouter/BrouterOutlet.cs`:
- Around line 66-72: The render sequence 1 used by b2.AddContent(1,
_matchedChild.ChildContent) may collide with sequence values emitted inside
BrouterRouteRenderer.ApplyTypedParameters; change ApplyTypedParameters
(BrouterRouteRenderer.ApplyTypedParameters) to accept a starting sequence
integer and return the next available sequence, call it from BrouterOutlet
passing the current seq after OpenComponent/CloseComponent, capture the returned
seq, and use that returned value when calling AddContent for
_matchedChild.ChildContent so sequence allocation is advanced consistently.
---
Nitpick comments:
In `@src/Brouter/Demo/Bit.Brouter.Demo.Maui/Platforms/MacCatalyst/Info.plist`:
- Around line 9-14: Remove the unused iPhone orientation settings by deleting
the UISupportedInterfaceOrientations array block (the
<key>UISupportedInterfaceOrientations</key> ... </array>) from the MacCatalyst
Info.plist since UIDeviceFamily is set to iPad-only (value 2) and only the ~ipad
variant applies; ensure no other code or plist entries reference
UISupportedInterfaceOrientations so the file remains valid after removal.
🪄 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: a8acf999-0180-4976-8783-77220bea6a29
⛔ 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 (94)
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/BrouterNavigationContext.cssrc/Brouter/Bit.Brouter/BrouterOptions.cssrc/Brouter/Bit.Brouter/BrouterOutlet.cssrc/Brouter/Bit.Brouter/BrouterParameterAttribute.cssrc/Brouter/Bit.Brouter/BrouterRoute.cssrc/Brouter/Bit.Brouter/BrouterRouteConstraint.cssrc/Brouter/Bit.Brouter/BrouterRouteParameters.cssrc/Brouter/Bit.Brouter/BrouterRouteRenderer.cssrc/Brouter/Bit.Brouter/BrouterRouteTemplate.cssrc/Brouter/Bit.Brouter/BrouterScrollMode.cssrc/Brouter/Bit.Brouter/BrouterService.cssrc/Brouter/Bit.Brouter/BrouterTemplateParser.cssrc/Brouter/Bit.Brouter/BrouterTemplateSegment.cssrc/Brouter/Bit.Brouter/BrouterTypeRouteConstraint.cssrc/Brouter/Bit.Brouter/IBrouter.cssrc/Brouter/Bit.Brouter/wwwroot/BitBrouter.jssrc/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/BrouterLinkTests.cssrc/Brouter/Tests/Bit.Brouter.Tests/BrouterTests.cssrc/Brouter/Tests/Bit.Brouter.Tests/BunitTestContext.cssrc/Brouter/Tests/Bit.Brouter.Tests/GuardAndLoaderTests.cssrc/Brouter/Tests/Bit.Brouter.Tests/GuardHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/LinkHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/LoaderHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/NamedRouteHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/OptionalParamHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/QueryBindingTests.cssrc/Brouter/Tests/Bit.Brouter.Tests/QueryConsumer.razorsrc/Brouter/Tests/Bit.Brouter.Tests/QueryHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/ResolveUrlTests.cssrc/Brouter/Tests/Bit.Brouter.Tests/SimpleHomeHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/SpecificityHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/TemplateParserTests.cssrc/Brouter/Tests/Bit.Brouter.Tests/ValueReader.razorsrc/Brouter/Tests/Bit.Brouter.Tests/_Imports.razor
Member
Author
|
@coderabbitai re-review |
|
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/Brouter/Bit.Brouter/BrouterOutlet.cs (1)
66-72:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRender-tree sequence collision between component attributes and child content.
ApplyTypedParametersemits attribute frames starting at sequence 1, butAddContentat line 72 also uses sequence 1. This collision can cause Blazor's diffing algorithm to behave incorrectly.Consider having
ApplyTypedParametersreturn the next available sequence number, or use a higher fixed sequence forAddContent:Suggested fix
else if (_matchedChild.Component is not null) { b2.OpenComponent(0, _matchedChild.Component); - BrouterRouteRenderer.ApplyTypedParameters(b2, _matchedChild.Component, _parameters, _matchedChild.Brouter?.CurrentLocation); + var nextSeq = BrouterRouteRenderer.ApplyTypedParameters(b2, _matchedChild.Component, _parameters, _matchedChild.Brouter?.CurrentLocation); b2.CloseComponent(); } // Render any descendant routes declared as ChildContent. - b2.AddContent(1, _matchedChild.ChildContent); + b2.AddContent(100, _matchedChild.ChildContent); // Use a high sequence to avoid collision#!/bin/bash # Verify ApplyTypedParameters sequence usage rg -n -A 20 'ApplyTypedParameters' src/Brouter/Bit.Brouter/BrouterRouteRenderer.cs | head -40🤖 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/BrouterOutlet.cs` around lines 66 - 72, The render-tree sequence numbers collide because BrouterRouteRenderer.ApplyTypedParameters emits attribute frames starting at sequence 1 while BrouterOutlet.cs calls b2.AddContent(1, _matchedChild.ChildContent);; update the code so sequence numbers never overlap — either modify ApplyTypedParameters (function ApplyTypedParameters in BrouterRouteRenderer) to return the next available sequence number after it writes attributes and use that returned value when calling AddContent, or keep ApplyTypedParameters unchanged but change the AddContent call in BrouterOutlet.cs to use a higher fixed sequence (e.g., 2 or the returned next sequence) to avoid the collision with attribute frames.
🤖 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/Demo/Bit.Brouter.Demo.Core/Pages/CounterPage.razor`:
- Line 14: The outer conditional in CounterPage.razor currently checks
Parameters for "init", "id", or "name" so the "age" route-parameter panel (the
block that renders lines ~28–32) never appears for URLs that only provide age;
update the condition that references Parameters to also include "age" (i.e.,
check Parameters.Contains("age") alongside "init", "id", and "name") so the age
panel becomes reachable when only age is present.
In `@src/Brouter/Demo/Bit.Brouter.Demo.Core/wwwroot/app.css`:
- Line 21: The CSS contains legacy ::deep selectors and an unquoted mono font
stack in the --bb-mono variable; remove or replace the ::deep selectors
(occurrences tied to the styles currently using ::deep) with standard scoped
selectors or :where/:is as appropriate, and update the --bb-mono custom property
value (the --bb-mono token) to wrap each font family that contains spaces or
special characters in quotes (e.g., "SFMono-Regular", "SF Mono", "Liberation
Mono") so the font-family list is fully quoted and lint-clean.
---
Duplicate comments:
In `@src/Brouter/Bit.Brouter/BrouterOutlet.cs`:
- Around line 66-72: The render-tree sequence numbers collide because
BrouterRouteRenderer.ApplyTypedParameters emits attribute frames starting at
sequence 1 while BrouterOutlet.cs calls b2.AddContent(1,
_matchedChild.ChildContent);; update the code so sequence numbers never overlap
— either modify ApplyTypedParameters (function ApplyTypedParameters in
BrouterRouteRenderer) to return the next available sequence number after it
writes attributes and use that returned value when calling AddContent, or keep
ApplyTypedParameters unchanged but change the AddContent call in
BrouterOutlet.cs to use a higher fixed sequence (e.g., 2 or the returned next
sequence) to avoid the collision with attribute frames.
🪄 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: 298ddb1f-8790-41f8-b3d9-ef11198da7bb
⛔ 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 (95)
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/BrouterNavigationContext.cssrc/Brouter/Bit.Brouter/BrouterOptions.cssrc/Brouter/Bit.Brouter/BrouterOutlet.cssrc/Brouter/Bit.Brouter/BrouterParameterAttribute.cssrc/Brouter/Bit.Brouter/BrouterRoute.cssrc/Brouter/Bit.Brouter/BrouterRouteConstraint.cssrc/Brouter/Bit.Brouter/BrouterRouteParameters.cssrc/Brouter/Bit.Brouter/BrouterRouteRenderer.cssrc/Brouter/Bit.Brouter/BrouterRouteTemplate.cssrc/Brouter/Bit.Brouter/BrouterScrollMode.cssrc/Brouter/Bit.Brouter/BrouterService.cssrc/Brouter/Bit.Brouter/BrouterTemplateParser.cssrc/Brouter/Bit.Brouter/BrouterTemplateSegment.cssrc/Brouter/Bit.Brouter/BrouterTypeRouteConstraint.cssrc/Brouter/Bit.Brouter/IBrouter.cssrc/Brouter/Bit.Brouter/wwwroot/BitBrouter.jssrc/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.Core/wwwroot/app.csssrc/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/BrouterLinkTests.cssrc/Brouter/Tests/Bit.Brouter.Tests/BrouterTests.cssrc/Brouter/Tests/Bit.Brouter.Tests/BunitTestContext.cssrc/Brouter/Tests/Bit.Brouter.Tests/GuardAndLoaderTests.cssrc/Brouter/Tests/Bit.Brouter.Tests/GuardHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/LinkHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/LoaderHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/NamedRouteHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/OptionalParamHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/QueryBindingTests.cssrc/Brouter/Tests/Bit.Brouter.Tests/QueryConsumer.razorsrc/Brouter/Tests/Bit.Brouter.Tests/QueryHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/ResolveUrlTests.cssrc/Brouter/Tests/Bit.Brouter.Tests/SimpleHomeHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/SpecificityHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/TemplateParserTests.cssrc/Brouter/Tests/Bit.Brouter.Tests/ValueReader.razorsrc/Brouter/Tests/Bit.Brouter.Tests/_Imports.razor
Member
Author
|
@coderabbitai re-review |
|
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/Brouter/Demo/Bit.Brouter.Demo.Core/wwwroot/app.css (1)
21-21:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winQuote font-family tokens in
--bb-monoto keep stylelint clean.Line 21 still uses unquoted font tokens (
SFMono-Regular,Menlo,Consolas), which triggers the current lint rule set.Suggested fix
- --bb-mono: ui-monospace, SFMono-Regular, "SF Mono", Menlo, Consolas, "Liberation Mono", monospace; + --bb-mono: ui-monospace, "SFMono-Regular", "SF Mono", "Menlo", "Consolas", "Liberation Mono", monospace;🤖 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/wwwroot/app.css` at line 21, The CSS custom property --bb-mono currently lists font-family tokens without quotes (e.g., SFMono-Regular, Menlo, Consolas) which triggers stylelint; update the --bb-mono declaration to wrap multi-word or special-name font tokens in quotes (for example "SFMono-Regular", "Menlo", "Consolas") so the font-family list is properly quoted and stylelint passes, ensuring the rest of the stack (ui-monospace, "Liberation Mono", monospace) remains unchanged.src/Brouter/Demo/Bit.Brouter.Demo.Maui/Bit.Brouter.Demo.Maui.csproj (1)
55-55:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRe-check
Microsoft.Extensions.Logging.Debugversion availability before merge.Line 55 pins
Microsoft.Extensions.Logging.Debugto10.0.8; this was previously flagged as not published on NuGet and can break restore.#!/bin/bash set -euo pipefail PACKAGE_LOWER="microsoft.extensions.logging.debug" echo "Checking versions published on NuGet for $PACKAGE_LOWER ..." curl -fsSL "https://api.nuget.org/v3-flatcontainer/${PACKAGE_LOWER}/index.json" | jq -r '.versions[-15:][]'If
10.0.8is absent, pin Line 55 to a published version consistent with the MAUI package set.🤖 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.Maui/Bit.Brouter.Demo.Maui.csproj` at line 55, The csproj pins Microsoft.Extensions.Logging.Debug to 10.0.8 which may not exist on NuGet; verify the package versions for "Microsoft.Extensions.Logging.Debug" (and the MAUI package set) via the NuGet API or tooling and, if 10.0.8 is not published, update the PackageReference in Bit.Brouter.Demo.Maui.csproj (the <PackageReference Include="Microsoft.Extensions.Logging.Debug" ... /> entry) to a published version compatible with your MAUI dependencies (e.g., the latest 10.0.x that appears in the NuGet index) so that restore succeeds.
🤖 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/BrouterService.cs`:
- Around line 111-150: The code treats an optional parameter with an empty
string as present and then throws; change ResolveUrl logic so that after
computing rawValue via FormatRouteValue(normalizedParams![segment.Value]) you
check if segment.IsOptional and string.IsNullOrEmpty(rawValue) and, if so, treat
it like the earlier "omitted optional" path: drop a trailing '/', set
optionalOmitted = true, set omittedOptionalName ??= segment.Value, and continue
(skipping the subsequent empty-value/ordering checks). This ensures empty
optional values are handled the same as missing ones.
---
Duplicate comments:
In `@src/Brouter/Demo/Bit.Brouter.Demo.Core/wwwroot/app.css`:
- Line 21: The CSS custom property --bb-mono currently lists font-family tokens
without quotes (e.g., SFMono-Regular, Menlo, Consolas) which triggers stylelint;
update the --bb-mono declaration to wrap multi-word or special-name font tokens
in quotes (for example "SFMono-Regular", "Menlo", "Consolas") so the font-family
list is properly quoted and stylelint passes, ensuring the rest of the stack
(ui-monospace, "Liberation Mono", monospace) remains unchanged.
In `@src/Brouter/Demo/Bit.Brouter.Demo.Maui/Bit.Brouter.Demo.Maui.csproj`:
- Line 55: The csproj pins Microsoft.Extensions.Logging.Debug to 10.0.8 which
may not exist on NuGet; verify the package versions for
"Microsoft.Extensions.Logging.Debug" (and the MAUI package set) via the NuGet
API or tooling and, if 10.0.8 is not published, update the PackageReference in
Bit.Brouter.Demo.Maui.csproj (the <PackageReference
Include="Microsoft.Extensions.Logging.Debug" ... /> entry) to a published
version compatible with your MAUI dependencies (e.g., the latest 10.0.x that
appears in the NuGet index) so that restore succeeds.
🪄 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: 701bd1cf-ee98-4ca2-a8a2-b6067d11a08d
⛔ 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 (95)
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/BrouterNavigationContext.cssrc/Brouter/Bit.Brouter/BrouterOptions.cssrc/Brouter/Bit.Brouter/BrouterOutlet.cssrc/Brouter/Bit.Brouter/BrouterParameterAttribute.cssrc/Brouter/Bit.Brouter/BrouterRoute.cssrc/Brouter/Bit.Brouter/BrouterRouteConstraint.cssrc/Brouter/Bit.Brouter/BrouterRouteParameters.cssrc/Brouter/Bit.Brouter/BrouterRouteRenderer.cssrc/Brouter/Bit.Brouter/BrouterRouteTemplate.cssrc/Brouter/Bit.Brouter/BrouterScrollMode.cssrc/Brouter/Bit.Brouter/BrouterService.cssrc/Brouter/Bit.Brouter/BrouterTemplateParser.cssrc/Brouter/Bit.Brouter/BrouterTemplateSegment.cssrc/Brouter/Bit.Brouter/BrouterTypeRouteConstraint.cssrc/Brouter/Bit.Brouter/IBrouter.cssrc/Brouter/Bit.Brouter/wwwroot/BitBrouter.jssrc/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.Core/wwwroot/app.csssrc/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/BrouterLinkTests.cssrc/Brouter/Tests/Bit.Brouter.Tests/BrouterTests.cssrc/Brouter/Tests/Bit.Brouter.Tests/BunitTestContext.cssrc/Brouter/Tests/Bit.Brouter.Tests/GuardAndLoaderTests.cssrc/Brouter/Tests/Bit.Brouter.Tests/GuardHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/LinkHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/LoaderHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/NamedRouteHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/OptionalParamHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/QueryBindingTests.cssrc/Brouter/Tests/Bit.Brouter.Tests/QueryConsumer.razorsrc/Brouter/Tests/Bit.Brouter.Tests/QueryHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/ResolveUrlTests.cssrc/Brouter/Tests/Bit.Brouter.Tests/SimpleHomeHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/SpecificityHost.razorsrc/Brouter/Tests/Bit.Brouter.Tests/TemplateParserTests.cssrc/Brouter/Tests/Bit.Brouter.Tests/ValueReader.razorsrc/Brouter/Tests/Bit.Brouter.Tests/_Imports.razor
Comment on lines
+111
to
+150
| var hasValue = normalizedParams is not null && normalizedParams.TryGetValue(segment.Value, out var raw) && raw is not null; | ||
| if (hasValue is false) | ||
| { | ||
| if (segment.IsOptional) | ||
| { | ||
| // Drop trailing '/' for the absent optional segment. | ||
| if (sb.Length > 0 && sb[^1] == '/') sb.Length--; | ||
| optionalOmitted = true; | ||
| omittedOptionalName ??= segment.Value; | ||
| continue; | ||
| } | ||
| throw new ArgumentException( | ||
| $"Missing value for required route parameter '{segment.Value}' when resolving route '{name}'.", | ||
| nameof(parameters)); | ||
| } | ||
|
|
||
| // A trailing optional with a value can't follow an omitted earlier optional, since | ||
| // optionals only ever live at the tail of a template (TemplateParser enforces this). | ||
| // Allowing it would emit a URL that re-binds to the wrong parameter. | ||
| if (segment.IsOptional && optionalOmitted) | ||
| { | ||
| throw new ArgumentException( | ||
| $"Cannot resolve route '{name}': optional parameter '{omittedOptionalName}' is missing " + | ||
| $"but a later optional parameter '{segment.Value}' has a value. " + | ||
| "Trailing optionals must be filled in order from left to right.", | ||
| nameof(parameters)); | ||
| } | ||
|
|
||
| var rawValue = FormatRouteValue(normalizedParams![segment.Value]); | ||
|
|
||
| // Required non-catch-all segments must round-trip through the router; an empty | ||
| // formatted value would emit a stray '/' (e.g. "/users//edit") that the matcher | ||
| // can't bind back. Catch-all is exempt because it has its own empty handling | ||
| // below (it just drops the trailing slash). Optional missing values are already | ||
| // handled above via the 'hasValue is false' branch. | ||
| if (segment.IsCatchAll is false && string.IsNullOrEmpty(rawValue)) | ||
| { | ||
| throw new ArgumentException( | ||
| $"Missing value for required route parameter '{segment.Value}' when resolving route '{name}'.", | ||
| nameof(parameters)); |
There was a problem hiding this comment.
Treat empty optional route values as omitted.
hasValue only checks raw is not null, so ResolveUrl(..., ["id"] = "") treats {id?} as present and then throws from the empty-value path below. That makes optional parameters stricter than missing parameters and also trips the later-optional ordering guard for empty strings. Handle an empty formatted value on optional segments the same way as an omitted value.
Possible fix
- // A trailing optional with a value can't follow an omitted earlier optional, since
- // optionals only ever live at the tail of a template (TemplateParser enforces this).
- // Allowing it would emit a URL that re-binds to the wrong parameter.
- if (segment.IsOptional && optionalOmitted)
- {
- throw new ArgumentException(
- $"Cannot resolve route '{name}': optional parameter '{omittedOptionalName}' is missing " +
- $"but a later optional parameter '{segment.Value}' has a value. " +
- "Trailing optionals must be filled in order from left to right.",
- nameof(parameters));
- }
-
var rawValue = FormatRouteValue(normalizedParams![segment.Value]);
+
+ if (segment.IsOptional && string.IsNullOrEmpty(rawValue))
+ {
+ if (sb.Length > 0 && sb[^1] == '/') sb.Length--;
+ optionalOmitted = true;
+ omittedOptionalName ??= segment.Value;
+ continue;
+ }
+
+ // A trailing optional with a value can't follow an omitted earlier optional, since
+ // optionals only ever live at the tail of a template (TemplateParser enforces this).
+ // Allowing it would emit a URL that re-binds to the wrong parameter.
+ if (segment.IsOptional && optionalOmitted)
+ {
+ throw new ArgumentException(
+ $"Cannot resolve route '{name}': optional parameter '{omittedOptionalName}' is missing " +
+ $"but a later optional parameter '{segment.Value}' has a value. " +
+ "Trailing optionals must be filled in order from left to right.",
+ nameof(parameters));
+ }
// Required non-catch-all segments must round-trip through the router; an empty
// formatted value would emit a stray '/' (e.g. "/users//edit") that the matcher
// can't bind back. Catch-all is exempt because it has its own empty handling
// below (it just drops the trailing slash). Optional missing values are already🤖 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 111 - 150, The code
treats an optional parameter with an empty string as present and then throws;
change ResolveUrl logic so that after computing rawValue via
FormatRouteValue(normalizedParams![segment.Value]) you check if
segment.IsOptional and string.IsNullOrEmpty(rawValue) and, if so, treat it like
the earlier "omitted optional" path: drop a trailing '/', set optionalOmitted =
true, set omittedOptionalName ??= segment.Value, and continue (skipping the
subsequent empty-value/ordering checks). This ensures empty optional values are
handled the same as missing ones.
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