Use fast invoke js extension methods in sync calls (#9917)#12426
Open
msynk wants to merge 38 commits into
Open
Use fast invoke js extension methods in sync calls (#9917)#12426msynk wants to merge 38 commits into
msynk wants to merge 38 commits into
Conversation
msynk
requested review from
Cyrus-Sushiant,
mhrastegari,
yasmoradi and
zahra-yousefi
|
Warning Review limit reached
More reviews will be available in 56 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (57)
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:
WalkthroughThe PR switches many Blazor JS interop calls to FastInvoke/FastInvokeVoid, changes several interop results to nullable contracts, adds defensive null and teardown handling in components and utilities, rewrites Extras resource loading, and adds runtime and TypeScript contract tests. ChangesFastInvoke JS interop migration and resilience hardening
Estimated code review effort🎯 5 (Critical) | ⏱️ ~90+ minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/UtilsJsRuntimeExtensions.cs (1)
7-7:⚠️ Potential issue | 🟡 MinorMigrate the remaining
BitBlazorUI.Utilswrappers toFastInvoke(they’re synchronous)
src/BlazorUI/Bit.BlazorUI/Scripts/Utils.tsimplementsgetBodyWidth,setProperty,getProperty,getBoundingClientRect,scrollElementIntoView,selectText, andsetStyleas non-asyncfunctions that directly return values /void(no Promise usage). Update the correspondingUtilsJsRuntimeExtensions.csmethods to usejsRuntime.FastInvoke/FastInvokeVoidinstead ofInvoke/InvokeVoidfor consistency withBitUtilsToggleOverflow.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/UtilsJsRuntimeExtensions.cs` at line 7, The wrappers in UtilsJsRuntimeExtensions.cs that call BitBlazorUI.Utils should be converted from synchronous jsRuntime.Invoke/InvokeVoid to the FastInvoke/FastInvokeVoid helpers used by BitUtilsToggleOverflow; locate the methods that call "BitBlazorUI.Utils.getBodyWidth", "setProperty", "getProperty", "getBoundingClientRect", "scrollElementIntoView", "selectText" and "setStyle" and replace Invoke<T>/Invoke with jsRuntime.FastInvoke<T>/FastInvoke or InvokeVoid with FastInvokeVoid while preserving argument lists and return types so the calls remain synchronous and consistent with the non-async implementations in Utils.ts.src/BlazorUI/Bit.BlazorUI/Components/Utilities/PullToRefresh/BitPullToRefresh.ts (1)
15-15:⚠️ Potential issue | 🟠 MajorType contract mismatch:
setupparameter should accept nullabledotnetObj.The C# side declares
DotNetObjectReference<BitPullToRefresh>?as nullable, but the TSsetupmethod parameter is typed as requiredDotNetObject. The method will crash at runtime when callingdotnetObj.invokeMethodAsync()(lines 38, 64, 71, 75) if null is passed.While the current call site (BitPullToRefresh.razor.cs:148) always creates a non-null reference, the type contract allows null and should be enforced defensively.
Recommended fix: Make the parameter optional and add a null guard:
dotnetObj: DotNetObject | undefined) { if (!dotnetObj) return;Alternatively, change the C# signature to use non-nullable
DotNetObjectReference<BitPullToRefresh>if null should never be passed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI/Components/Utilities/PullToRefresh/BitPullToRefresh.ts` at line 15, The setup function's dotnetObj parameter in BitPullToRefresh.ts is declared non-nullable but the C# contract allows null; update the setup signature (in the setup function) to accept DotNetObject | undefined (or optional) and add an early null guard (if (!dotnetObj) return;) before any calls to dotnetObj.invokeMethodAsync() (these occur in the setup function and in any helper closures referenced there such as the touch/move handlers), or alternatively change the C# DotNetObjectReference<BitPullToRefresh> to be non-nullable if null will never be passed.
🧹 Nitpick comments (1)
src/BlazorUI/Bit.BlazorUI/Components/Surfaces/Splitter/BitSplitter.ts (1)
27-29: ⚡ Quick winRemove redundant
!element.stylecheck.The
!element.stylecondition is unreachable becauseHTMLElementalways has astyleproperty. Once the!elementcheck passes,element.styleis guaranteed to exist.♻️ Simplify the guard clause
- public static resetPaneDimensions(element: HTMLElement | undefined) { - if (!element || !element.style) return; - + public static resetPaneDimensions(element: HTMLElement | undefined) { + if (!element) return; +🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI/Components/Surfaces/Splitter/BitSplitter.ts` around lines 27 - 29, The guard in BitSplitter.resetPaneDimensions redundantly checks !element.style after confirming element exists; remove the unreachable "!element.style" condition so the guard becomes "if (!element) return;" and update any surrounding comments accordingly, referencing the public static method resetPaneDimensions on BitSplitter to locate the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/PdfReader/BitPdfReaderJsRuntimeExtensions.cs`:
- Around line 20-23: BitPdfReaderRenderPage and BitPdfReaderRefreshPage are
still calling the old InvokeVoid; change both to use FastInvokeVoid for
consistency with BitPdfReaderDispose (which already uses FastInvokeVoid). Locate
the methods BitPdfReaderRenderPage and BitPdfReaderRefreshPage and replace
jsRuntime.InvokeVoid("BitBlazorUI.PdfReader.renderPage", ...) and
jsRuntime.InvokeVoid("BitBlazorUI.PdfReader.refreshPage", ...) with
jsRuntime.FastInvokeVoid("BitBlazorUI.PdfReader.renderPage", ...) and
jsRuntime.FastInvokeVoid("BitBlazorUI.PdfReader.refreshPage", ...) respectively,
leaving BitPdfReaderSetup (Invoke<int>) unchanged.
---
Outside diff comments:
In
`@src/BlazorUI/Bit.BlazorUI/Components/Utilities/PullToRefresh/BitPullToRefresh.ts`:
- Line 15: The setup function's dotnetObj parameter in BitPullToRefresh.ts is
declared non-nullable but the C# contract allows null; update the setup
signature (in the setup function) to accept DotNetObject | undefined (or
optional) and add an early null guard (if (!dotnetObj) return;) before any calls
to dotnetObj.invokeMethodAsync() (these occur in the setup function and in any
helper closures referenced there such as the touch/move handlers), or
alternatively change the C# DotNetObjectReference<BitPullToRefresh> to be
non-nullable if null will never be passed.
In `@src/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/UtilsJsRuntimeExtensions.cs`:
- Line 7: The wrappers in UtilsJsRuntimeExtensions.cs that call
BitBlazorUI.Utils should be converted from synchronous
jsRuntime.Invoke/InvokeVoid to the FastInvoke/FastInvokeVoid helpers used by
BitUtilsToggleOverflow; locate the methods that call
"BitBlazorUI.Utils.getBodyWidth", "setProperty", "getProperty",
"getBoundingClientRect", "scrollElementIntoView", "selectText" and "setStyle"
and replace Invoke<T>/Invoke with jsRuntime.FastInvoke<T>/FastInvoke or
InvokeVoid with FastInvokeVoid while preserving argument lists and return types
so the calls remain synchronous and consistent with the non-async
implementations in Utils.ts.
---
Nitpick comments:
In `@src/BlazorUI/Bit.BlazorUI/Components/Surfaces/Splitter/BitSplitter.ts`:
- Around line 27-29: The guard in BitSplitter.resetPaneDimensions redundantly
checks !element.style after confirming element exists; remove the unreachable
"!element.style" condition so the guard becomes "if (!element) return;" and
update any surrounding comments accordingly, referencing the public static
method resetPaneDimensions on BitSplitter to locate the change.
🪄 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: 73675b8f-81fa-4452-afd7-53844f2dc3f3
📒 Files selected for processing (20)
src/BlazorUI/Bit.BlazorUI.Extras/Components/AppShell/BitAppShellJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Chart/BitChart.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Chart/JsInterop/BitChartJsInterop.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/InfiniteScrolling/BitInfiniteScrollingJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/PdfReader/BitPdfReaderJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Extensions/JsInterop/ExtrasJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/CircularTimePicker/BitCircularTimePickerJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ColorPicker/BitColorPickerJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUploadJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/OtpInput/BitOtpInputJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/SearchBox/BitSearchBoxJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Swiper/BitSwiperJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Modal/BitModalJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Splitter/BitSplitter.tssrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Splitter/BitSplitterJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Utilities/PullToRefresh/BitPullToRefresh.tssrc/BlazorUI/Bit.BlazorUI/Components/Utilities/PullToRefresh/BitPullToRefreshJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/UtilsJsRuntimeExtensions.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/IJSRuntimeFastExtensionsTests.cs
Member
Author
|
@coderabbitai re-review |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/UtilsJsRuntimeExtensions.cs`:
- Line 25: The JS interop helper BitUtilsGetBoundingClientRect in
UtilsJsRuntimeExtensions currently returns
jsRuntime.FastInvoke<BoundingClientRect>(...) which can be null; update
BitUtilsGetBoundingClientRect to guard the FastInvoke result and return a
non-null BoundingClientRect (e.g. coalesce to a new BoundingClientRect with
Left/Top/Width/Height = 0) so callers like BitColorPicker.UpdateColor and
BitCircularTimePicker.UpdateTime can safely read properties without null checks
or NREs.
🪄 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: 401888d2-353d-481f-b50a-076162576f0f
📒 Files selected for processing (4)
src/BlazorUI/Bit.BlazorUI.Extras/Components/PdfReader/BitPdfReaderJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Splitter/BitSplitter.tssrc/BlazorUI/Bit.BlazorUI/Components/Utilities/PullToRefresh/BitPullToRefreshJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/UtilsJsRuntimeExtensions.cs
🚧 Files skipped from review as they are similar to previous changes (2)
- src/BlazorUI/Bit.BlazorUI/Components/Utilities/PullToRefresh/BitPullToRefreshJsRuntimeExtensions.cs
- src/BlazorUI/Bit.BlazorUI/Components/Surfaces/Splitter/BitSplitter.ts
Member
Author
|
@coderabbitai re-review |
|
✅ Action performedReview finished.
|
Member
Author
|
@coderabbitai re-review |
|
✅ Action performedFull review finished. |
Member
Author
|
@coderabbitai re-review |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/BlazorUI/Bit.BlazorUI.Extras/Scripts/Extras.ts`:
- Line 42: The current detection of an already-loaded script in the document
(the Array.from(document.scripts).find callback that assigns existingTag) uses
s.src.includes(url), which can produce false positives; change the predicate to
a stricter match — e.g., compare the script src filename or use
s.src.endsWith(url) (or strip query/hash and compare the last path segment) so
only exact filename/ending matches are considered; update the find call that
sets existingTag accordingly to use this precise match logic.
🪄 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: bcc2884c-09ab-4c8c-a64e-75e6bb4e0ee6
📒 Files selected for processing (9)
src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Scripts/Extras.tssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUploadJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/IJSRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/IJSRuntimeFastExtensions.cssrc/BlazorUI/Bit.BlazorUI/Scripts/Utils.tssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/FastInvokeSyncContractTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/IJSRuntimeFastExtensionsTests.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUploadJsRuntimeExtensions.cs
Member
Author
|
@coderabbitai re-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/IJSRuntimeFastExtensionsTests.cs (1)
15-20: ⚡ Quick winSerialize the tests that mutate
OnError.
IJSRuntimeFastExtensions.OnErroris process-global. These cases set/reset it per test, so they can race and flake if MSTest parallelization is enabled for the assembly or CI run. Mark this class[DoNotParallelize]or guard the hook with class-level synchronization.Also applies to: 76-80, 93-96, 109-112
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/IJSRuntimeFastExtensionsTests.cs` around lines 15 - 20, The tests mutate the process-global IJSRuntimeFastExtensions.OnError which can race when MSTest runs tests in parallel; update the test class to prevent parallel execution by adding the MSTest DoNotParallelize attribute (apply to the test class declaration) or alternatively wrap all accesses to IJSRuntimeFastExtensions.OnError used in the tests (setup/teardown and tests referencing it at lines with ResetErrorHandler and the tests at 76-80, 93-96, 109-112) with a class-level lock to serialize access; pick one approach and apply it consistently so the OnError hook is never concurrently read/modified across tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/BlazorUI/Bit.BlazorUI.Extras/Scripts/Extras.ts`:
- Around line 44-49: The script-matching currently reduces URLs to pathname only
via the normalize function and then compares normalize(s.src) to targetPath,
which conflates distinct origins and query strings; update the logic so
normalize preserves origin and query (or otherwise compute a full absolute URL
string) when generating targetPath and when comparing against document.scripts,
then use that full URL comparison to find existingTag (refer to the normalize
arrow function, targetPath constant and existingTag lookup) so scripts like
different CDNs or different ?v= queries are considered distinct.
In
`@src/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/FastInvokeSyncContractTests.cs`:
- Around line 38-40: CollectAsyncJsMethods currently relies on
TsReturnsPromiseRegex to detect Promise-returning methods by body heuristics,
which yields false negatives for delegated returns; instead tighten the
convention so only explicit async functions or those with an explicit Promise
return type are treated as async. Update CollectAsyncJsMethods to stop using
TsReturnsPromiseRegex and only add methods to asyncJsMethods when the method has
the async modifier or its return annotation explicitly contains "Promise";
adjust any test expectations that assumed body-based detection and update
references to TsReturnsPromiseRegex (and related logic) accordingly.
---
Nitpick comments:
In
`@src/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/IJSRuntimeFastExtensionsTests.cs`:
- Around line 15-20: The tests mutate the process-global
IJSRuntimeFastExtensions.OnError which can race when MSTest runs tests in
parallel; update the test class to prevent parallel execution by adding the
MSTest DoNotParallelize attribute (apply to the test class declaration) or
alternatively wrap all accesses to IJSRuntimeFastExtensions.OnError used in the
tests (setup/teardown and tests referencing it at lines with ResetErrorHandler
and the tests at 76-80, 93-96, 109-112) with a class-level lock to serialize
access; pick one approach and apply it consistently so the OnError hook is never
concurrently read/modified across tests.
🪄 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: 2defda38-b1ea-4eb2-8611-b1ad9efc9060
📒 Files selected for processing (30)
src/BlazorUI/Bit.BlazorUI.Extras/Components/AppShell/BitAppShellJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Chart/BitChart.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Chart/JsInterop/BitChartJsInterop.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/InfiniteScrolling/BitInfiniteScrollingJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/PdfReader/BitPdfReaderJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Extensions/JsInterop/ExtrasJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Scripts/Extras.tssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/CircularTimePicker/BitCircularTimePicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/CircularTimePicker/BitCircularTimePickerJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ColorPicker/BitColorPicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ColorPicker/BitColorPickerJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUploadJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/OtpInput/BitOtpInputJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/SearchBox/BitSearchBoxJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Carousel/BitCarousel.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Swiper/BitSwiperJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Modal/BitModalJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Splitter/BitSplitter.tssrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Splitter/BitSplitterJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Utilities/PullToRefresh/BitPullToRefresh.tssrc/BlazorUI/Bit.BlazorUI/Components/Utilities/PullToRefresh/BitPullToRefreshJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/IJSRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/IJSRuntimeFastExtensions.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/UtilsJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Scripts/Utils.tssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Extras/AppShell/BitAppShellTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/FastInvokeSyncContractTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/IJSRuntimeFastExtensionsTests.cs
Member
Author
|
@coderabbitai re-review |
|
✅ Action performedFull review finished. |
Member
Author
|
@coderabbitai re-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/IsRuntimeInvalidFrameworkContractTests.cs (1)
47-53: 🩺 Stability & Availability | 🔵 TrivialVerified: Framework assumptions hold for .NET 8, 9, and 10, but reliance on internal APIs is unsupported
The reflection assumptions regarding
WebViewJSRuntimeandRemoteJSRuntime.IsInitializedare currently valid across the target frameworks. However,RemoteJSRuntimeis an internal class, and Microsoft explicitly states that relying onIsInitialized(even via reflection) is unsupported and may change in future versions.While the tests function as intended for now, this approach carries a risk of breakage with upcoming .NET updates. Consider implementing a more robust detection mechanism that does not rely on internal framework implementation details if long-term stability is critical.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/IsRuntimeInvalidFrameworkContractTests.cs` around lines 47 - 53, The test in IsRuntimeInvalidFrameworkContractTests relies on internal framework details such as WebViewJSRuntime and RemoteJSRuntime.IsInitialized via reflection, which is unsupported. Update the IsRuntimeInvalid test coverage to use a more stable, public-facing runtime detection path instead of reflecting into internal Blazor types. Keep the behavior verified by the existing IsRuntimeInvalid() extension, but adjust CreateFrameworkRuntime and the related framework-hybrid test setup so it no longer depends on internal implementation details.src/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.razor.cs (1)
2334-2346: 🩺 Stability & Availability | 🔵 TrivialReorder
_dotnetObjdisposal to occur after JS teardown calls.In
src/BlazorUI/Bit.BlazorUI/Scripts/Swipes.ts, thedispose()method invokes thedisposercallback before cleaning up thedotnetObj. The current C# implementation disposes_dotnetObjbefore callingBitSwipesDispose, which means the .NET object may be invalid if the JSdisposercallback attempts to interact with it during teardown.Align the disposal order with
BitDropdown.DisposeAsyncby moving_dotnetObj?.Dispose()to execute after the JavaScript interop calls complete.Current Code
_dotnetObj?.Dispose(); try { await _js.BitCalloutClearCallout(_calloutId); await _js.BitSwipesDispose(_calloutId); } catch (JSDisconnectedException) { }Suggested Order
try { await _js.BitCalloutClearCallout(_calloutId); await _js.BitSwipesDispose(_calloutId); } catch (JSDisconnectedException) { } _dotnetObj?.Dispose();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.razor.cs` around lines 2334 - 2346, The disposal order in the DateRangePicker teardown is wrong: `_dotnetObj` is being disposed before the JS cleanup that may still call back into it. Update `BitDateRangePicker.DisposeAsync` to keep the existing `_js.BitCalloutClearCallout` and `_js.BitSwipesDispose` calls first, then dispose `_dotnetObj` afterward, matching the pattern used by `BitDropdown.DisposeAsync`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/InfiniteScrolling/BitInfiniteScrollingJsRuntimeExtensions.cs`:
- Around line 13-25: The JS interop methods in
BitInfiniteScrollingJsRuntimeExtensions are fine with FastInvokeVoid, but the
infinite scrolling setup callback needs error handling. Update the
BitBlazorUI.InfiniteScrolling.setup logic so the async IntersectionObserver
callback wraps dotnetObj.invokeMethodAsync("Load") in try/catch and handles
rejection safely, while keeping the
BitInfiniteScrollingSetup/BitInfiniteScrollingReobserve/BitInfiniteScrollingDispose
entry points unchanged.
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/PhoneInput/BitPhoneInput.razor.cs`:
- Line 370: The scrolling behavior in BitPhoneInput is targeting the active
option element through BitExtrasScrollElementIntoView, which can cause the page
viewport to move instead of the list container. Update the PhoneInput scroll
logic in BitPhoneInput to explicitly scroll the element identified by
_scrollContainerId rather than the option returned by GetOptionId(_activeIndex),
so the active option stays visible within the callout/dropdown list without
affecting the page.
In
`@src/BlazorUI/Bit.BlazorUI/Components/Utilities/PullToRefresh/BitPullToRefresh.razor.cs`:
- Around line 247-257: The async cleanup in BitPullToRefresh.DisposeAsync only
handles JSDisconnectedException, so _dotnetObj can leak when
_js.BitPullToRefreshDispose(UniqueId) fails with other exceptions. Update the
DisposeAsync cleanup path in BitPullToRefresh.razor.cs to use deterministic
cleanup for _dotnetObj in all failure cases: catch the broader exception flow
around the BitPullToRefreshDispose call, dispose and clear _dotnetObj, then
rethrow so callers still see the original error.
In
`@src/BlazorUI/Bit.BlazorUI/Components/Utilities/SwipeTrap/BitSwipeTrap.razor.cs`:
- Around line 125-135: The disposal path in BitSwipeTrap.DisposeAsync only
cleans up _dotnetObj inside the JSDisconnectedException catch, so other teardown
failures can orphan the managed reference and skip base cleanup. Update the
BitSwipeTrap.razor.cs dispose flow to perform _dotnetObj disposal in a finally
or shared cleanup path, similar to BitPageVisibility.DisposeAsync and the
BitPullToRefresh fix, so _dotnetObj is always released regardless of the
exception type thrown by _js.BitSwipeTrapDispose.
---
Nitpick comments:
In
`@src/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.razor.cs`:
- Around line 2334-2346: The disposal order in the DateRangePicker teardown is
wrong: `_dotnetObj` is being disposed before the JS cleanup that may still call
back into it. Update `BitDateRangePicker.DisposeAsync` to keep the existing
`_js.BitCalloutClearCallout` and `_js.BitSwipesDispose` calls first, then
dispose `_dotnetObj` afterward, matching the pattern used by
`BitDropdown.DisposeAsync`.
In
`@src/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/IsRuntimeInvalidFrameworkContractTests.cs`:
- Around line 47-53: The test in IsRuntimeInvalidFrameworkContractTests relies
on internal framework details such as WebViewJSRuntime and
RemoteJSRuntime.IsInitialized via reflection, which is unsupported. Update the
IsRuntimeInvalid test coverage to use a more stable, public-facing runtime
detection path instead of reflecting into internal Blazor types. Keep the
behavior verified by the existing IsRuntimeInvalid() extension, but adjust
CreateFrameworkRuntime and the related framework-hybrid test setup so it no
longer depends on internal implementation details.
🪄 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: ea1489a4-c1cd-450d-9d6e-b79de93097ad
📒 Files selected for processing (59)
src/BlazorUI/Bit.BlazorUI.Extras/Components/AppShell/BitAppShellJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Chart/BitChart.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Chart/BitChart.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Chart/JsInterop/BitChartJsInterop.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/InfiniteScrolling/BitInfiniteScrollingJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/MarkdownViewer/BitMarkdownViewer.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/MarkdownViewer/BitMarkdownViewerJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/PdfReader/BitPdfReaderJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/PhoneInput/BitPhoneInput.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/ProModal/BitProModal.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Extensions/JsInterop/ExtrasJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Scripts/Extras.tssrc/BlazorUI/Bit.BlazorUI.Extras/Services/BitMarkdownService.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/CircularTimePicker/BitCircularTimePicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/CircularTimePicker/BitCircularTimePickerJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ColorPicker/BitColorPicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ColorPicker/BitColorPicker.tssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ColorPicker/BitColorPickerJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUpload.tssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUploadJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/OtpInput/BitOtpInputJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/SearchBox/BitSearchBox.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/SearchBox/BitSearchBoxJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Slider/BitSlider.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/TimePicker/BitTimePicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Carousel/BitCarousel.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Swiper/BitSwiper.tssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Swiper/BitSwiperJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Dialog/BitDialog.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Panel/BitPanel.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Splitter/BitSplitter.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Splitter/BitSplitter.tssrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Splitter/BitSplitterJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Utilities/Overlay/BitOverlay.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Utilities/PullToRefresh/BitPullToRefresh.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Utilities/PullToRefresh/BitPullToRefresh.tssrc/BlazorUI/Bit.BlazorUI/Components/Utilities/PullToRefresh/BitPullToRefreshJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Utilities/SwipeTrap/BitSwipeTrap.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Utilities/SwipeTrap/BitSwipeTrap.tssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/IJSRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/IJSRuntimeFastExtensions.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/JsInteropConstants.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/UtilsJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Scripts/PageVisibility.tssrc/BlazorUI/Bit.BlazorUI/Scripts/Swipes.tssrc/BlazorUI/Bit.BlazorUI/Scripts/Utils.tssrc/BlazorUI/Bit.BlazorUI/Utils/BitPageVisibility.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Bit.BlazorUI.Tests.csprojsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Extras/AppShell/BitAppShellTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/FastInvokeSyncContractTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/IJSRuntimeFastExtensionsTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/IsRuntimeInvalidFrameworkContractTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/TsPromiseMethodScanner.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/TsPromiseMethodScannerTests.cs
Member
Author
|
@coderabbitai re-review |
|
✅ Action performedFull review finished. |
Member
Author
|
@coderabbitai re-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razor.cs (1)
1793-1800: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDispose
_dotnetObjafter the JS teardown.Line 1793 releases the reference before
BitCalloutClearCallout/BitSwipesDispose, even though this component still hands that object to the same JS teardown path earlier in the file. That can break teardown callbacks or reintroduce double-dispose noise.BitDateRangePicker.DisposeAsyncin this PR already uses the safer ordering.Suggested fix
- _dotnetObj?.Dispose(); - try { await _js.BitCalloutClearCallout(_calloutId); await _js.BitSwipesDispose(_calloutId); } catch (JSDisconnectedException) { } // we can ignore this exception here + finally + { + _dotnetObj?.Dispose(); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razor.cs` around lines 1793 - 1800, Dispose order in BitDatePicker.DisposeAsync is wrong: _dotnetObj is released before the JS teardown calls, which can interfere with teardown callbacks used by _js.BitCalloutClearCallout and _js.BitSwipesDispose. Move the _dotnetObj?.Dispose() call to after the JS cleanup block, matching the safer ordering already used in BitDateRangePicker.DisposeAsync, and keep the JSDisconnectedException handling unchanged.src/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.razor.cs (1)
2338-2347: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winMove
_dotnetObjdisposal into afinally.If
BitCalloutClearCalloutorBitSwipesDisposethrows anything other thanJSDisconnectedException, control never reaches Line 2347, so the managed reference still leaks. The fallback cleanup added inBitPullToRefresh.DisposeAsyncsolves the same failure mode.Suggested fix
try { await _js.BitCalloutClearCallout(_calloutId); await _js.BitSwipesDispose(_calloutId); } catch (JSDisconnectedException) { } // we can ignore this exception here - - // Dispose the .NET reference after the JS cleanup so any callbacks the JS teardown makes still - // have a live target, matching BitDropdown.DisposeAsync. - _dotnetObj?.Dispose(); + finally + { + // Dispose the .NET reference after the JS cleanup so any callbacks the JS teardown makes + // still have a live target. + _dotnetObj?.Dispose(); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.razor.cs` around lines 2338 - 2347, The disposal path in BitDateRangePicker.DisposeAsync can leak _dotnetObj when BitCalloutClearCallout or BitSwipesDispose throws a non-JSDisconnectedException, so move the _dotnetObj?.Dispose() cleanup into a finally block around the JS teardown. Keep the existing try/catch for JSDisconnectedException, but ensure the managed reference is always released regardless of JS cleanup failures, following the same pattern used in BitPullToRefresh.DisposeAsync and the BitDropdown.DisposeAsync-style lifetime handling.src/BlazorUI/Bit.BlazorUI/Components/Inputs/TimePicker/BitTimePicker.razor.cs (1)
769-786: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winDispose the
DotNetObjectReferenceafter JS teardown.
_dotnetObjis still the callback target forBitSwipesSetup, so disposing it beforeBitSwipesDispose(_calloutId)leaves a teardown window where registered JS handlers can invoke an already-disposed reference.BitDropdownalready uses the safer pattern by disposing infinallyafter the JS cleanup completes.Suggested fix
protected override async ValueTask DisposeAsync(bool disposing) { if (IsDisposed || disposing is false) return; await base.DisposeAsync(disposing); _cancellationTokenSource?.Cancel(); _cancellationTokenSource?.Dispose(); OnValueChanged -= HandleOnValueChanged; - - _dotnetObj?.Dispose(); try { await _js.BitCalloutClearCallout(_calloutId); await _js.BitSwipesDispose(_calloutId); } catch (JSDisconnectedException) { } // we can ignore this exception here + finally + { + _dotnetObj?.Dispose(); + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/TimePicker/BitTimePicker.razor.cs` around lines 769 - 786, Dispose the DotNetObjectReference in BitTimePicker.DisposeAsync only after the JS teardown completes. In DisposeAsync, keep _dotnetObj alive until after the BitCalloutClearCallout and BitSwipesDispose calls finish, then dispose _dotnetObj in a finally block (matching the safer BitDropdown pattern) so JS callbacks cannot target an already-disposed reference during cleanup.
🧹 Nitpick comments (1)
src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.ts (1)
47-47: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winPrefer a standards-based fallback here.
Line 47 now becomes a silent no-op when
scrollIntoViewIfNeededis unavailable, so the column-options popup may still stay clipped on browsers that do not implement that non-standard API. Falling back toscrollIntoView({ block: 'nearest', inline: 'nearest' })keeps the visibility behavior intact.Suggested change
- colOptions.scrollIntoViewIfNeeded?.(); + if (typeof colOptions.scrollIntoViewIfNeeded === 'function') { + colOptions.scrollIntoViewIfNeeded(); + } else { + colOptions.scrollIntoView({ block: 'nearest', inline: 'nearest' }); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.ts` at line 47, The popup positioning in BitDataGrid is relying on the non-standard scrollIntoViewIfNeeded call, which becomes a no-op on unsupported browsers. Update the column-options scrolling logic in the relevant BitDataGrid popup/column-options method to use scrollIntoViewIfNeeded when available, and fall back to scrollIntoView with nearest block/inline alignment so the element still stays visible.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/BlazorUI/Bit.BlazorUI.Extras/Scripts/Extras.ts`:
- Around line 221-253: The module-script lookup in findExistingResource is too
strict because it only matches s.type === 'module', so existing host tags like
<script type="MODULE"> can be missed and the same URL injected twice. Update the
script branch in Extras.findExistingResource to compare module types
case-insensitively while keeping the classic branch unchanged and strict via
isExecutableClassicScriptType, so only executable classic script types still
qualify there.
In
`@src/BlazorUI/Bit.BlazorUI/Components/Utilities/PullToRefresh/BitPullToRefresh.ts`:
- Around line 171-176: The dispose path in BitPullToRefresh is swallowing
failures from dotnetObj.dispose(), which makes the cleanup look successful even
when the JS-side handoff failed. Update the DisposeAsync/cleanup logic around
this call so dispose errors are not caught-and-ignored; let the failure surface
or explicitly trigger the fallback _dotnetObj cleanup path, and keep the
dotnetObj reset only after the ownership handoff is handled correctly.
In
`@src/BlazorUI/Bit.BlazorUI/Components/Utilities/SwipeTrap/BitSwipeTrap.razor.cs`:
- Around line 123-145: The disposal logic in BitSwipeTrap.DisposeAsync skips
base cleanup when BitSwipeTrapDispose throws a non-JSDisconnectedException
because the catch block rethrows before reaching the base call. Refactor the
method so base.DisposeAsync(disposing) is always executed from the failure path
as well, while preserving the current _dotnetObj ownership handling and disposal
of the managed reference in both exception branches.
In
`@src/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/TsPromiseMethodScanner.cs`:
- Around line 46-49: The Promise annotation check in TsPromiseMethodScanner is
too broad because SkipReturnAnnotation currently scans nested return-type text,
causing false positives for cases like functions or objects that merely contain
Promise inside. Update the return-annotation handling in the scanning logic to
only consider the top-level return type, using the existing
TsPromiseMethodScanner and SkipReturnAnnotation flow as the main touchpoints,
and add a regression test covering nested Promise mentions so only true
async-returning methods are flagged.
---
Outside diff comments:
In
`@src/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razor.cs`:
- Around line 1793-1800: Dispose order in BitDatePicker.DisposeAsync is wrong:
_dotnetObj is released before the JS teardown calls, which can interfere with
teardown callbacks used by _js.BitCalloutClearCallout and _js.BitSwipesDispose.
Move the _dotnetObj?.Dispose() call to after the JS cleanup block, matching the
safer ordering already used in BitDateRangePicker.DisposeAsync, and keep the
JSDisconnectedException handling unchanged.
In
`@src/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.razor.cs`:
- Around line 2338-2347: The disposal path in BitDateRangePicker.DisposeAsync
can leak _dotnetObj when BitCalloutClearCallout or BitSwipesDispose throws a
non-JSDisconnectedException, so move the _dotnetObj?.Dispose() cleanup into a
finally block around the JS teardown. Keep the existing try/catch for
JSDisconnectedException, but ensure the managed reference is always released
regardless of JS cleanup failures, following the same pattern used in
BitPullToRefresh.DisposeAsync and the BitDropdown.DisposeAsync-style lifetime
handling.
In
`@src/BlazorUI/Bit.BlazorUI/Components/Inputs/TimePicker/BitTimePicker.razor.cs`:
- Around line 769-786: Dispose the DotNetObjectReference in
BitTimePicker.DisposeAsync only after the JS teardown completes. In
DisposeAsync, keep _dotnetObj alive until after the BitCalloutClearCallout and
BitSwipesDispose calls finish, then dispose _dotnetObj in a finally block
(matching the safer BitDropdown pattern) so JS callbacks cannot target an
already-disposed reference during cleanup.
---
Nitpick comments:
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.ts`:
- Line 47: The popup positioning in BitDataGrid is relying on the non-standard
scrollIntoViewIfNeeded call, which becomes a no-op on unsupported browsers.
Update the column-options scrolling logic in the relevant BitDataGrid
popup/column-options method to use scrollIntoViewIfNeeded when available, and
fall back to scrollIntoView with nearest block/inline alignment so the element
still stays visible.
🪄 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: 940a1e2d-fd5e-4ac2-9ce1-3aa5ddea1ea7
📒 Files selected for processing (57)
src/BlazorUI/Bit.BlazorUI.Extras/Components/AppShell/BitAppShellJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Chart/BitChart.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Chart/BitChart.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Chart/JsInterop/BitChartJsInterop.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/InfiniteScrolling/BitInfiniteScrolling.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/InfiniteScrolling/BitInfiniteScrollingJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/PdfReader/BitPdfReaderJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/PhoneInput/BitPhoneInput.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/ProModal/BitProModal.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Extensions/JsInterop/ExtrasJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Scripts/Extras.tssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/CircularTimePicker/BitCircularTimePicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/CircularTimePicker/BitCircularTimePickerJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ColorPicker/BitColorPicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ColorPicker/BitColorPicker.tssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ColorPicker/BitColorPickerJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUpload.tssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUploadJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/OtpInput/BitOtpInputJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/SearchBox/BitSearchBox.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/SearchBox/BitSearchBoxJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Slider/BitSlider.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/TimePicker/BitTimePicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Carousel/BitCarousel.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Swiper/BitSwiper.tssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Swiper/BitSwiperJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Dialog/BitDialog.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Panel/BitPanel.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Splitter/BitSplitter.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Splitter/BitSplitter.tssrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Splitter/BitSplitterJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Utilities/Overlay/BitOverlay.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Utilities/PullToRefresh/BitPullToRefresh.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Utilities/PullToRefresh/BitPullToRefresh.tssrc/BlazorUI/Bit.BlazorUI/Components/Utilities/PullToRefresh/BitPullToRefreshJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Utilities/SwipeTrap/BitSwipeTrap.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Utilities/SwipeTrap/BitSwipeTrap.tssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/IJSRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/IJSRuntimeFastExtensions.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/JsInteropConstants.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/UtilsJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Scripts/PageVisibility.tssrc/BlazorUI/Bit.BlazorUI/Scripts/Swipes.tssrc/BlazorUI/Bit.BlazorUI/Scripts/Utils.tssrc/BlazorUI/Bit.BlazorUI/Utils/BitPageVisibility.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Bit.BlazorUI.Tests.csprojsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Extras/AppShell/BitAppShellTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/FastInvokeSyncContractTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/IJSRuntimeFastExtensionsTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/IsRuntimeInvalidFrameworkContractTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/TsPromiseMethodScanner.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/TsPromiseMethodScannerTests.cs
Member
Author
|
@coderabbitai re-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/Chart/JsInterop/BitChartJsInterop.cs`:
- Around line 28-57: The XML docs on BitChartJsSetupChart and
BitChartJsUpdateChart advertise a false return path that does not exist in the
JavaScript contract. Update the summaries/returns on these methods to match the
actual BitBlazorUI.BitChart.setupChart and BitBlazorUI.BitChart.updateChart
behavior: either remove the false case and document null for failure on the
fast/in-process path, or change the JS implementation to return false instead of
throwing so the contract is consistent.
In `@src/BlazorUI/Bit.BlazorUI/Components/Surfaces/Panel/BitPanel.razor.cs`:
- Around line 260-273: The disposal logic in BitPanel’s teardown path only
releases _dotnetObj inside the JSDisconnectedException handler, so a normal
JSException can leak the GCHandle and keep the component alive. Update the
try/catch around _js.BitSwipesDispose in BitPanel to also dispose _dotnetObj
before rethrowing on non-disconnect failures, and clear the _dotnetObj field on
the successful JS ownership path after BitSwipesDispose completes.
In
`@src/BlazorUI/Bit.BlazorUI/Components/Utilities/PullToRefresh/BitPullToRefresh.razor.cs`:
- Around line 245-265: Ensure the failure path in BitPullToRefresh.DisposeAsync
still executes the base cleanup: the catch-all around BitPullToRefreshDispose
currently rethrows before base.DisposeAsync(disposing) can run. Update
BitPullToRefresh.DisposeAsync so the managed _dotnetObj release still happens on
errors, but base.DisposeAsync(disposing) is always awaited from the failure path
before propagating the exception.
In `@src/BlazorUI/Bit.BlazorUI/Components/Utilities/SwipeTrap/BitSwipeTrap.ts`:
- Around line 166-173: `BitSwipeTrap.dispose()` is swallowing failures from
`dotnetObj.dispose()`, which hides JS-to-.NET handoff errors. Update the
`dispose` method to stop catching and only logging the exception; instead, let
the disposal failure surface so the matching `BitSwipeTrap.razor.cs` cleanup
paths can handle `_dotnetObj` correctly.
In
`@src/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/FastInvokeSyncContractTests.cs`:
- Around line 158-217: `ResolveConstStringIdentifier` is using brace counting
and a file-wide name fallback that can pick the wrong `const string` when braces
appear in strings/comments or when another member/type has the same name. Update
`ResolveConstStringIdentifier` and `IsDeclarationInScopeAt` to use syntax-aware
scope detection from the C# source instead of raw text scanning, and restrict
resolution to the declaration that is actually visible at the call site. Remove
or tighten the broad fallback so it cannot bind a same-named const from an
unrelated scope.
In
`@src/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/TsPromiseMethodScanner.cs`:
- Around line 37-44: The raw regex scans in TsPromiseMethodScanner can falsely
match class and static method text inside comments or strings, which can add
bogus Class.Method entries to the map used by FastInvokeSyncContractTests.
Update the scanning logic around TsClassRegex and TsStaticMethodHeaderRegex to
ignore non-code content before matching, or replace the regex approach with a
real parser, and add a regression test covering commented-out or quoted
signatures.
🪄 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: ccffc541-31df-4ede-8144-964da14feb90
📒 Files selected for processing (57)
src/BlazorUI/Bit.BlazorUI.Extras/Components/AppShell/BitAppShellJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Chart/BitChart.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Chart/BitChart.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Chart/JsInterop/BitChartJsInterop.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/InfiniteScrolling/BitInfiniteScrolling.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/InfiniteScrolling/BitInfiniteScrollingJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/PdfReader/BitPdfReaderJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/PhoneInput/BitPhoneInput.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/ProModal/BitProModal.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Extensions/JsInterop/ExtrasJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Scripts/Extras.tssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/CircularTimePicker/BitCircularTimePicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/CircularTimePicker/BitCircularTimePickerJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ColorPicker/BitColorPicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ColorPicker/BitColorPicker.tssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ColorPicker/BitColorPickerJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUpload.tssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUploadJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/OtpInput/BitOtpInputJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/SearchBox/BitSearchBox.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/SearchBox/BitSearchBoxJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Slider/BitSlider.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/TimePicker/BitTimePicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Carousel/BitCarousel.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Swiper/BitSwiper.tssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Swiper/BitSwiperJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Dialog/BitDialog.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Panel/BitPanel.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Splitter/BitSplitter.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Splitter/BitSplitter.tssrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Splitter/BitSplitterJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Utilities/Overlay/BitOverlay.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Utilities/PullToRefresh/BitPullToRefresh.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Utilities/PullToRefresh/BitPullToRefresh.tssrc/BlazorUI/Bit.BlazorUI/Components/Utilities/PullToRefresh/BitPullToRefreshJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Utilities/SwipeTrap/BitSwipeTrap.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Utilities/SwipeTrap/BitSwipeTrap.tssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/IJSRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/IJSRuntimeFastExtensions.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/JsInteropConstants.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/UtilsJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Scripts/PageVisibility.tssrc/BlazorUI/Bit.BlazorUI/Scripts/Swipes.tssrc/BlazorUI/Bit.BlazorUI/Scripts/Utils.tssrc/BlazorUI/Bit.BlazorUI/Utils/BitPageVisibility.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Bit.BlazorUI.Tests.csprojsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Extras/AppShell/BitAppShellTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/FastInvokeSyncContractTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/IJSRuntimeFastExtensionsTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/IsRuntimeInvalidFrameworkContractTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/TsPromiseMethodScanner.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/TsPromiseMethodScannerTests.cs
Member
Author
|
@coderabbitai re-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/BlazorUI/Bit.BlazorUI.Extras/Components/Chart/BitChart.ts (1)
57-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThrow an
Errorobject here instead of a string. A primitive throw drops the standard error shape and stack, which makes this JS interop failure harder to diagnose.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Chart/BitChart.ts` at line 57, The Chart setup path is throwing a string instead of a proper error object, which breaks the standard error shape and stack trace. Update the throw in the chart initialization logic (the code that validates config.canvasId in BitChart.ts) to create and throw an Error instance with the same message, so JS interop failures are easier to diagnose.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Extras/AppShell/BitAppShellTests.cs`:
- Around line 738-739: The AppShell tests have incomplete bUnit JSInterop setups
for awaited void calls, so the setups for BitBlazorUI.AppShell.initScroll,
BitBlazorUI.AppShell.afterRenderScroll, and BitBlazorUI.AppShell.disposeScroll
should all be completed with SetVoidResult(), while keeping
BitBlazorUI.AppShell.locationChangedScroll unchanged because it is
fire-and-forget. Update the relevant JSInterop.SetupVoid calls in
BitAppShellTests so every awaited AppShell interop invocation is fully
configured.
In
`@src/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/IsRuntimeInvalidFrameworkContractTests.cs`:
- Around line 92-97: `CreateFrameworkRuntime` is instantiating an internal
runtime type with the default Activator overload, which can fail for
`UnsupportedJavaScriptRuntime` because its parameterless constructor is
non-public. Update the `Activator.CreateInstance` call in
`CreateFrameworkRuntime` to use the overload that allows non-public
constructors, so the prerender test can construct the framework runtime
successfully.
---
Nitpick comments:
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Chart/BitChart.ts`:
- Line 57: The Chart setup path is throwing a string instead of a proper error
object, which breaks the standard error shape and stack trace. Update the throw
in the chart initialization logic (the code that validates config.canvasId in
BitChart.ts) to create and throw an Error instance with the same message, so JS
interop failures are easier to diagnose.
🪄 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: fb00506e-7d70-47d9-86e0-4dc9af919f8f
📒 Files selected for processing (57)
src/BlazorUI/Bit.BlazorUI.Extras/Components/AppShell/BitAppShellJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Chart/BitChart.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Chart/BitChart.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Chart/JsInterop/BitChartJsInterop.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/InfiniteScrolling/BitInfiniteScrolling.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/InfiniteScrolling/BitInfiniteScrollingJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/PdfReader/BitPdfReaderJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/PhoneInput/BitPhoneInput.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/ProModal/BitProModal.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Extensions/JsInterop/ExtrasJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Scripts/Extras.tssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/CircularTimePicker/BitCircularTimePicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/CircularTimePicker/BitCircularTimePickerJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ColorPicker/BitColorPicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ColorPicker/BitColorPicker.tssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ColorPicker/BitColorPickerJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUpload.tssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUploadJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/OtpInput/BitOtpInputJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/SearchBox/BitSearchBox.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/SearchBox/BitSearchBoxJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Slider/BitSlider.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/TimePicker/BitTimePicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Carousel/BitCarousel.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Swiper/BitSwiper.tssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Swiper/BitSwiperJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Dialog/BitDialog.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Panel/BitPanel.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Splitter/BitSplitter.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Splitter/BitSplitter.tssrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Splitter/BitSplitterJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Utilities/Overlay/BitOverlay.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Utilities/PullToRefresh/BitPullToRefresh.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Utilities/PullToRefresh/BitPullToRefresh.tssrc/BlazorUI/Bit.BlazorUI/Components/Utilities/PullToRefresh/BitPullToRefreshJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Utilities/SwipeTrap/BitSwipeTrap.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Utilities/SwipeTrap/BitSwipeTrap.tssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/IJSRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/IJSRuntimeFastExtensions.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/JsInteropConstants.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/UtilsJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Scripts/PageVisibility.tssrc/BlazorUI/Bit.BlazorUI/Scripts/Swipes.tssrc/BlazorUI/Bit.BlazorUI/Scripts/Utils.tssrc/BlazorUI/Bit.BlazorUI/Utils/BitPageVisibility.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Bit.BlazorUI.Tests.csprojsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Extras/AppShell/BitAppShellTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/FastInvokeSyncContractTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/IJSRuntimeFastExtensionsTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/IsRuntimeInvalidFrameworkContractTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/TsPromiseMethodScanner.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/TsPromiseMethodScannerTests.cs
Member
Author
|
@coderabbitai re-review |
|
✅ Action performedFull review finished. |
Member
Author
|
@coderabbitai re-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Chart/BitChart.razor.cs`:
- Around line 30-34: The XML doc comment on BitChart should no longer imply that
this first-render callback is the right place to register plugins or custom JS
options, since BitChartJsSetupChart(Config) has already been attempted by then.
Update the remarks on the first-render callback in BitChart.razor.cs to either
remove that guidance or clearly state that plugin/custom option registration
must happen before first render or before Config is assigned.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Scripts/Extras.ts`:
- Around line 244-255: The classic script reuse path in findExistingResource is
too permissive and can match <script nomodule> tags that will not execute in
module-capable browsers. Update the script branch in Extras.findExistingResource
so the non-module predicate also excludes noModule host scripts, while keeping
the existing type and load-failure checks intact.
In
`@src/BlazorUI/Bit.BlazorUI/Components/Inputs/ColorPicker/BitColorPickerJsRuntimeExtensions.cs`:
- Around line 8-13: The setup helper currently hides a failed JS setup by
returning an empty string, which can prevent later disposal of the
DotNetObjectReference. Update
BitColorPickerJsRuntimeExtensions.BitColorPickerSetup to preserve the nullable
failure state from FastInvoke or explicitly dispose the BitColorPicker reference
when setup returns no id, and make BitColorPicker.razor.cs handle that case
before storing _abortControllerId so BitColorPickerDispose can still clean up
correctly.
In `@src/BlazorUI/Bit.BlazorUI/Components/Surfaces/Panel/BitPanel.razor.cs`:
- Around line 170-175: The swipe setup in BitPanel creates _dotnetObj before
calling BitSwipesSetup, but if that JS call throws the reference is left
undisposed and can leak. Update the BitPanel.razor.cs setup flow so the
DotNetObjectReference created in the swipe initialization path is disposed when
BitSwipesSetup fails, and only kept for later cleanup on the success path
handled by DisposeAsync.
In `@src/BlazorUI/Bit.BlazorUI/Scripts/Swipes.ts`:
- Around line 259-264: The disposal block in Swipes.ts is swallowing failures
from dotnetObj.dispose() and then clearing the reference, which hides teardown
errors and removes any retry path. Update the dispose/cleanup logic in the
relevant method to let the exception surface instead of catching and logging it,
keeping behavior consistent with the other disposal paths in this cohort. Make
sure the dotnetObj cleanup in this section preserves the failure signal before
setting dotnetObj to undefined.
🪄 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: ac523a3a-96ab-4aee-b277-d586fe30b2c6
📒 Files selected for processing (57)
src/BlazorUI/Bit.BlazorUI.Extras/Components/AppShell/BitAppShellJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Chart/BitChart.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Chart/BitChart.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Chart/JsInterop/BitChartJsInterop.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/InfiniteScrolling/BitInfiniteScrolling.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/InfiniteScrolling/BitInfiniteScrollingJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/PdfReader/BitPdfReaderJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/PhoneInput/BitPhoneInput.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/ProModal/BitProModal.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Extensions/JsInterop/ExtrasJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Scripts/Extras.tssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/CircularTimePicker/BitCircularTimePicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/CircularTimePicker/BitCircularTimePickerJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ColorPicker/BitColorPicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ColorPicker/BitColorPicker.tssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ColorPicker/BitColorPickerJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUpload.tssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUploadJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/OtpInput/BitOtpInputJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/SearchBox/BitSearchBox.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/SearchBox/BitSearchBoxJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Slider/BitSlider.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/TimePicker/BitTimePicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Carousel/BitCarousel.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Swiper/BitSwiper.tssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Swiper/BitSwiperJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Dialog/BitDialog.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Panel/BitPanel.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Splitter/BitSplitter.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Splitter/BitSplitter.tssrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Splitter/BitSplitterJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Utilities/Overlay/BitOverlay.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Utilities/PullToRefresh/BitPullToRefresh.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Utilities/PullToRefresh/BitPullToRefresh.tssrc/BlazorUI/Bit.BlazorUI/Components/Utilities/PullToRefresh/BitPullToRefreshJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Utilities/SwipeTrap/BitSwipeTrap.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Utilities/SwipeTrap/BitSwipeTrap.tssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/IJSRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/IJSRuntimeFastExtensions.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/JsInteropConstants.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/UtilsJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Scripts/PageVisibility.tssrc/BlazorUI/Bit.BlazorUI/Scripts/Swipes.tssrc/BlazorUI/Bit.BlazorUI/Scripts/Utils.tssrc/BlazorUI/Bit.BlazorUI/Utils/BitPageVisibility.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Bit.BlazorUI.Tests.csprojsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Extras/AppShell/BitAppShellTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/FastInvokeSyncContractTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/IJSRuntimeFastExtensionsTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/IsRuntimeInvalidFrameworkContractTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/TsPromiseMethodScanner.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Extensions/JsInterop/TsPromiseMethodScannerTests.cs
Member
Author
|
@coderabbitai re-review |
|
✅ Action performedFull review finished. |
Member
Author
|
@coderabbitai re-review |
|
✅ Action performedFull review finished. |
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
1 participant
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 #9917
Summary by CodeRabbit