Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,21 @@ internal static class BitAppShellJsRuntimeExtensions
{
internal static ValueTask BitAppShellInitScroll(this IJSRuntime jsRuntime, ElementReference container, string url)
{
return jsRuntime.InvokeVoid("BitBlazorUI.AppShell.initScroll", container, url);
return jsRuntime.FastInvokeVoid("BitBlazorUI.AppShell.initScroll", container, url);
}

internal static ValueTask BitAppShellLocationChangedScroll(this IJSRuntime jsRuntime, string url)
{
return jsRuntime.InvokeVoid("BitBlazorUI.AppShell.locationChangedScroll", url);
return jsRuntime.FastInvokeVoid("BitBlazorUI.AppShell.locationChangedScroll", url);
}

internal static ValueTask BitAppShellAfterRenderScroll(this IJSRuntime jsRuntime, string url)
{
return jsRuntime.InvokeVoid("BitBlazorUI.AppShell.afterRenderScroll", url);
return jsRuntime.FastInvokeVoid("BitBlazorUI.AppShell.afterRenderScroll", url);
}

internal static ValueTask BitAppShellDisposeScroll(this IJSRuntime jsRuntime)
{
return jsRuntime.InvokeVoid("BitBlazorUI.AppShell.disposeScroll");
return jsRuntime.FastInvokeVoid("BitBlazorUI.AppShell.disposeScroll");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -147,13 +147,21 @@ protected override async Task OnAfterRenderAsync(bool firstRender)
await _js.BitChartJsSetupChart(Config);
}

// Always signal completion on first render, matching the long-standing behavior: consumers may
// rely on SetupCompletedCallback firing once the component has rendered regardless of whether the
// chart setup itself succeeded (e.g. when Config is still null, or when interop was unavailable
// and the result was swallowed on the WebAssembly fast path).
await SetupCompletedCallback.InvokeAsync(this);

Comment on lines +150 to +155

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

SetupCompletedCallback documentation contradicts its actual firing condition.

The XML documentation (lines 30–33) states the callback fires "when the chart has been setup through interop and the JavaScript chart object is available," but the code now invokes it unconditionally on first render regardless of whether Config is null or setup succeeded. Consumers relying on this callback to signal "chart is ready" will receive false positives when Config is null or when the interop call fails/returns false on WebAssembly's fast path.

Either update the XML docs to match the actual behavior (fires after first render regardless of setup outcome), or move the callback invocation inside the if (Config is not null) block and only invoke it when setup completes successfully.

🤖 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.razor.cs` around
lines 150 - 155, The XML documentation for SetupCompletedCallback states it
fires when the chart has been setup through interop and the JavaScript object is
available, but the current implementation invokes it unconditionally on first
render regardless of whether Config is null or setup succeeded. Either move the
await SetupCompletedCallback.InvokeAsync(this) call to execute only when setup
completes successfully by placing it inside the if (Config is not null) block,
or update the XML documentation comments to accurately reflect that the callback
fires unconditionally on first render. The recommended approach is to move the
callback invocation to only fire when setup actually succeeds so that consumers
relying on this callback to signal chart readiness do not receive false
positives.

return;
}

if (Config is not null)
{
await _js.BitChartJsSetupChart(Config);
// Re-runs setup after a Config change. The readiness result is intentionally discarded here:
// SetupCompletedCallback is raised only once, on first render, so subsequent re-setups don't
// re-signal readiness.
_ = await _js.BitChartJsSetupChart(Config);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ namespace BitBlazorUI {

public static updateChart(config: BitChartConfiguration): boolean {
if (!BitChart._bitCharts.has(config.canvasId))
throw `Could not find a chart with the given id. ${config.canvasId}`;
throw `Could not find a chart with the given id: ${config.canvasId}`;

let myChart = BitChart._bitCharts.get(config.canvasId);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,33 +22,39 @@ internal static class BitChartJsInterop

public static ValueTask BitChartJsRemoveChart(this IJSRuntime jsRuntime, string? canvasId)
{
return jsRuntime.InvokeVoid("BitBlazorUI.BitChart.removeChart", canvasId);
return jsRuntime.FastInvokeVoid("BitBlazorUI.BitChart.removeChart", canvasId);
}

/// <summary>
/// Set up a new chart. Call only once.
/// </summary>
/// <param name="jsRuntime"></param>
/// <param name="chartConfig">The config for the new chart.</param>
/// <returns></returns>
public static ValueTask<bool> BitChartJsSetupChart(this IJSRuntime jsRuntime, BitChartConfigBase chartConfig)
/// <returns>
/// <see langword="true"/> when setup succeeded, <see langword="false"/> when the chart could not be updated in place,
/// or <see langword="null"/> when interop could not run or an error was swallowed on the in-process (WASM) path.
/// </returns>
public static ValueTask<bool?> BitChartJsSetupChart(this IJSRuntime jsRuntime, BitChartConfigBase chartConfig)
{
var dynParam = StripNulls(chartConfig);
Dictionary<string, object> param = ConvertExpandoObjectToDictionary(dynParam!);
return jsRuntime.Invoke<bool>("BitBlazorUI.BitChart.setupChart", param);
return jsRuntime.FastInvoke<bool?>("BitBlazorUI.BitChart.setupChart", param);
}

/// <summary>
/// Update an existing chart. Make sure that the Chart with this <see cref="BitChartConfigBase.CanvasId"/> already exists.
/// </summary>
/// <param name="jsRuntime"></param>
/// <param name="chartConfig">The updated config of the chart you want to update.</param>
/// <returns></returns>
public static ValueTask<bool> BitChartJsUpdateChart(this IJSRuntime jsRuntime, BitChartConfigBase chartConfig)
/// <returns>
/// <see langword="true"/> when the chart was updated, <see langword="false"/> when the chart instance was missing,
/// or <see langword="null"/> when interop could not run or an error was swallowed on the in-process (WASM) path.
/// </returns>
public static ValueTask<bool?> BitChartJsUpdateChart(this IJSRuntime jsRuntime, BitChartConfigBase chartConfig)
{
var dynParam = StripNulls(chartConfig);
var param = ConvertExpandoObjectToDictionary(dynParam!);
return jsRuntime.Invoke<bool>("BitBlazorUI.BitChart.updateChart", param);
return jsRuntime.FastInvoke<bool?>("BitBlazorUI.BitChart.updateChart", param);
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ namespace BitBlazorUI {
colOptions.style.transform = `translateX(${applyOffset}px)`;
}

colOptions.scrollIntoViewIfNeeded();
colOptions.scrollIntoViewIfNeeded?.();

const autoFocusElem = colOptions.querySelector('[autofocus]');
if (autoFocusElem) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,22 @@

internal static class BitDataGridJsRuntimeExtensions
{
public static async ValueTask<IJSObjectReference> BitDataGridInit(this IJSRuntime jsRuntime, ElementReference tableElement)
// FastInvoke returns default (null) when the runtime can't service interop or a JSON/JS interop
// error is swallowed on the in-process (WASM) path. Callers must null-check before using the
// reference; a null result means DataGrid JS hooks were not initialized.
public static async ValueTask<IJSObjectReference?> BitDataGridInit(this IJSRuntime jsRuntime, ElementReference tableElement)
{
return await jsRuntime.Invoke<IJSObjectReference>("BitBlazorUI.DataGrid.init", tableElement);
const string identifier = "BitBlazorUI.DataGrid.init";
var result = await jsRuntime.FastInvoke<IJSObjectReference>(identifier, tableElement);
return jsRuntime.ReportIfUnexpectedNull(identifier, result);
}

// This is a fire-and-forget call from OnAfterRenderAsync that runs DOM-heavy positioning logic
// (getBoundingClientRect, scrollIntoViewIfNeeded, focus). It deliberately uses the regular async
// invocation rather than FastInvokeVoid: on WebAssembly FastInvokeVoid runs synchronously and can
// alter Promise/ordering and error-propagation semantics, so we use the async Invoke pattern to keep
// any JS-side failure (e.g. scrollIntoViewIfNeeded being unsupported) contained within the returned
// task instead of letting it escape synchronously into the render loop.
public static async ValueTask BitDataGridCheckColumnOptionsPosition(this IJSRuntime jsRuntime, ElementReference tableElement)
{
await jsRuntime.InvokeVoid("BitBlazorUI.DataGrid.checkColumnOptionsPosition", tableElement);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,18 @@ public static ValueTask BitInfiniteScrollingSetup<T>(this IJSRuntime jsRuntime,
decimal? threshold,
DotNetObjectReference<BitInfiniteScrolling<T>> dotnetObj)
{
return jsRuntime.InvokeVoid("BitBlazorUI.InfiniteScrolling.setup", id, scrollerSelector, rootElement, lastElement, threshold, dotnetObj);
return jsRuntime.FastInvokeVoid("BitBlazorUI.InfiniteScrolling.setup", id, scrollerSelector, rootElement, lastElement, threshold, dotnetObj);
}

public static ValueTask BitInfiniteScrollingReobserve(this IJSRuntime jsRuntime,
string id,
ElementReference lastElement)
{
return jsRuntime.InvokeVoid("BitBlazorUI.InfiniteScrolling.reobserve", id, lastElement);
return jsRuntime.FastInvokeVoid("BitBlazorUI.InfiniteScrolling.reobserve", id, lastElement);
}

public static ValueTask BitInfiniteScrollingDispose(this IJSRuntime jsRuntime, string id)
{
return jsRuntime.InvokeVoid("BitBlazorUI.InfiniteScrolling.dispose", id);
return jsRuntime.FastInvokeVoid("BitBlazorUI.InfiniteScrolling.dispose", id);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ namespace BitBlazorUI {
}

public static parse(md: string) {
// The `async: false` option MUST remain. This method is FastInvoked (FastInvoke<string>)
// from the .NET side, which requires a synchronous string return. marked.parse returns a
// Promise when async is true, which would silently turn the FastInvoke call into a
// fire-and-forget with no test catching the regression. Use parseAsync for async needs.
let html = marked.parse(md, { async: false });

return html;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,19 @@

internal static class BitMarkdownViewerJsRuntimeExtensions
{
public static ValueTask<bool> BitMarkdownViewerCheckScriptLoaded(this IJSRuntime jsRuntime, string script)
// FastInvoke returns null when the runtime can't service interop or a JSON/JS interop error is
// swallowed on the in-process (WASM) path. Nullable distinguishes that from a legitimate false.
public static ValueTask<bool?> BitMarkdownViewerCheckScriptLoaded(this IJSRuntime jsRuntime, string script)
{
return jsRuntime.FastInvoke<bool>("BitBlazorUI.MarkdownViewer.checkScriptLoaded", script);
return jsRuntime.FastInvoke<bool?>("BitBlazorUI.MarkdownViewer.checkScriptLoaded", script);
}

public static ValueTask<string> BitMarkdownViewerParse(this IJSRuntime jsRuntime, string markdown, string? middleware)
// FastInvoke/Invoke return null when the runtime can't service interop or a JSON/JS interop error
// is swallowed on the in-process (WASM) path. Nullable surfaces that so call sites can coalesce.
public static ValueTask<string?> BitMarkdownViewerParse(this IJSRuntime jsRuntime, string markdown, string? middleware)
{
return OperatingSystem.IsBrowser() && middleware.HasNoValue()
? jsRuntime.FastInvoke<string>("BitBlazorUI.MarkdownViewer.parse", markdown)
: jsRuntime.Invoke<string>("BitBlazorUI.MarkdownViewer.parseAsync", markdown, middleware);
? jsRuntime.FastInvoke<string?>("BitBlazorUI.MarkdownViewer.parse", markdown)
: jsRuntime.Invoke<string?>("BitBlazorUI.MarkdownViewer.parseAsync", markdown, middleware);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,21 @@ public static ValueTask<int> BitPdfReaderSetup(this IJSRuntime jsRuntime, BitPdf

public static ValueTask BitPdfReaderRenderPage(this IJSRuntime jsRuntime, string id, int pageNumber)
{
// The JS renderPage is async (awaits pdf.js page rendering). FastInvoke would use the
// synchronous in-process path in WASM, discarding the returned Promise (fire-and-forget),
// so callers would proceed/raise events before rendering completes and errors would be lost.
return jsRuntime.InvokeVoid("BitBlazorUI.PdfReader.renderPage", id, pageNumber);
}

public static ValueTask BitPdfReaderRefreshPage(this IJSRuntime jsRuntime, BitPdfReaderConfig config, int pageNumber)
{
// The JS refreshPage is async (awaits renderPage). See BitPdfReaderRenderPage for why
// the asynchronous invocation must be used instead of the synchronous fast-invoke.
return jsRuntime.InvokeVoid("BitBlazorUI.PdfReader.refreshPage", config, pageNumber);
}

public static ValueTask BitPdfReaderDispose(this IJSRuntime jsRuntime, string id)
{
return jsRuntime.InvokeVoid("BitBlazorUI.PdfReader.dispose", id);
return jsRuntime.FastInvokeVoid("BitBlazorUI.PdfReader.dispose", id);
}
Comment thread
msynk marked this conversation as resolved.
}
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ protected override async Task OnAfterRenderAsync(bool firstRender)
if (_isOpen && _activeIndex >= 0 && _activeIndex != _lastScrolledIndex)
{
_lastScrolledIndex = _activeIndex;
await _js.BitExtrasScrollOptionIntoView(GetOptionId(_activeIndex));
await _js.BitExtrasScrollElementIntoView(GetOptionId(_activeIndex));
}

await base.OnAfterRenderAsync(firstRender);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -317,11 +317,11 @@ private async Task ToggleScroll(bool isOpen)

if (ScrollerElement.HasValue)
{
_offsetTop = await _js.BitUtilsToggleOverflow(ScrollerElement.Value, isOpen);
_offsetTop = await _js.BitUtilsToggleOverflow(ScrollerElement.Value, isOpen) ?? 0;
}
else
{
_offsetTop = await _js.BitUtilsToggleOverflow(ScrollerSelector ?? "body", isOpen);
_offsetTop = await _js.BitUtilsToggleOverflow(ScrollerSelector ?? "body", isOpen) ?? 0;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,17 @@ internal static class ExtrasJsRuntimeExtensions
{
internal static ValueTask BitExtrasApplyRootClasses(this IJSRuntime jsRuntime, List<string> cssClasses, Dictionary<string, string> cssVariables)
{
return jsRuntime.InvokeVoid("BitBlazorUI.Extras.applyRootClasses", cssClasses, cssVariables);
return jsRuntime.FastInvokeVoid("BitBlazorUI.Extras.applyRootClasses", cssClasses, cssVariables);
}

internal static ValueTask BitExtrasGoToTop(this IJSRuntime jsRuntime, ElementReference element, BitScrollBehavior? behavior = null)
{
return jsRuntime.InvokeVoid("BitBlazorUI.Extras.goToTop", element, behavior?.ToString().ToLowerInvariant());
return jsRuntime.FastInvokeVoid("BitBlazorUI.Extras.goToTop", element, behavior?.ToString().ToLowerInvariant());
}

internal static ValueTask BitExtrasScrollBy(this IJSRuntime jsRuntime, ElementReference element, decimal x, decimal y)
{
return jsRuntime.InvokeVoid("BitBlazorUI.Extras.scrollBy", element, x, y);
return jsRuntime.FastInvokeVoid("BitBlazorUI.Extras.scrollBy", element, x, y);
}

public static ValueTask BitExtrasInitScripts(this IJSRuntime jsRuntime, IEnumerable<string> scripts, bool isModule = false)
Expand All @@ -29,16 +29,16 @@ public static ValueTask BitExtrasInitStylesheets(this IJSRuntime jsRuntime, IEnu

internal static ValueTask BitExtrasSetPreventKeys(this IJSRuntime jsRuntime, ElementReference element, string[] keys)
{
return jsRuntime.InvokeVoid("BitBlazorUI.Extras.setPreventKeys", element, keys);
return jsRuntime.FastInvokeVoid("BitBlazorUI.Extras.setPreventKeys", element, keys);
}

internal static ValueTask BitExtrasDisposePreventKeys(this IJSRuntime jsRuntime, ElementReference element)
{
return jsRuntime.InvokeVoid("BitBlazorUI.Extras.disposePreventKeys", element);
return jsRuntime.FastInvokeVoid("BitBlazorUI.Extras.disposePreventKeys", element);
}

internal static ValueTask BitExtrasScrollOptionIntoView(this IJSRuntime jsRuntime, string optionId)
internal static ValueTask BitExtrasScrollElementIntoView(this IJSRuntime jsRuntime, string elementId)
{
return jsRuntime.InvokeVoid("BitBlazorUI.Extras.scrollOptionIntoView", optionId);
return jsRuntime.FastInvokeVoid("BitBlazorUI.Extras.scrollElementIntoView", elementId);
}
}
Loading
Loading