Use fast invoke js extension methods in sync calls (#9917)#12426
Open
msynk wants to merge 11 commits into
Open
Use fast invoke js extension methods in sync calls (#9917)#12426msynk wants to merge 11 commits into
msynk wants to merge 11 commits into
Conversation
msynk
requested review from
Cyrus-Sushiant,
mhrastegari,
yasmoradi and
zahra-yousefi
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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:
WalkthroughMigrates many Blazor UI JSRuntime extension calls from Invoke/InvokeVoid to FastInvoke/FastInvokeVoid, adds runtime-validity caches and OnError routing, makes bounding-rect helpers nullable, rewrites Extras script loader to per-URL promises, and introduces tests validating FastInvoke runtime behavior and sync-contracts. ChangesFastInvoke JS Interop Migration
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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. |
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`:
- Around line 34-35: Normalize the incoming `url` to the same absolute form used
when matching existing script tags before using it as a cache key: compute the
normalized/absolute URL (the same normalization used when checking
`existingTag.src`) and use that normalized value for all accesses and
assignments to `Extras._scriptPromises` (instead of raw `url`) so lookups,
writes and deletions (the spots around `existingPromise =
Extras._scriptPromises[url]`, the existingTag checks, and the promise
set/resolve/remove locations) consistently use the same key and avoid races
where an equivalent URL misses the cache.
🪄 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: 5c6f6c0f-1b50-4b3f-adc9-9d106c7d049a
📒 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
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
Performance
Bug Fixes / Robustness
Tests