ni / nimble

The NI Nimble Design System
https://nimble.ni.dev
MIT License
32 stars 8 forks source link

Possible race condition showing Blazor NimbleDialog during OnAfterRenderAsync #2188

Open epetersoni opened 5 months ago

epetersoni commented 5 months ago

πŸ› Bug Report

I have a Blazor component containing a NimbleDialog that we want to have open when the component first renders. Intermittently (maybe 30% of the time), I get this JavaScript error about not finding the show function:

Warning: Unhandled exception rendering component: "dialogReference.show is not a function TypeError: dialogReference.show is not a function at Object.show ( http://localhost:59798/_content/NimbleBlazor/NimbleBlazor.lib.module.js:151:50) at http://localhost:59798/_framework/blazor.server.js:1:3244 at new Promise () at y.beginInvokeJSFromDotNet ( http://localhost:59798/_framework/blazor.server.js:1:3201) at Xt._invokeClientMethod ( http://localhost:59798/_framework/blazor.server.js:1:61001) at Xt._processIncomingData ( http://localhost:59798/_framework/blazor.server.js:1:58476) at Xt.connection.onreceive ( http://localhost:59798/_framework/blazor.server.js:1:52117) at s.onmessage ( http://localhost:59798/_framework/blazor.server.js:1:80262)"%22)

πŸ’» Repro or Code Sample

I created a simple reproducing example as follows:

  1. Create a new .NET 8 Blazor project.
  2. Add references to NimbleBlazor and Radzen.Blazor nuget packages.
  3. Add references to _content/NimbleBlazor/nimble-components/all-components-bundle.min.js and _content/Radzen.Blazor/Radzen.Blazor.js scripts in App.razor.
  4. Replace the code in Home.razor with
    
    @page "/"

<NimbleBlazor.NimbleDialog TCloseReason="string" @ref="_dialog"> Dialog content </NimbleBlazor.NimbleDialog>

@code { private NimbleDialog _dialog = null!;

protected override async Task OnAfterRenderAsync(bool firstRender)
{
    if (firstRender)
    {
        await _dialog.ShowAsync();
    }
    await base.OnAfterRenderAsync(firstRender);
}

}


5. Run the application. If the JavaScript error doesn't happen, you can refresh the page and it should happen eventually.

Note that I could not get this to reproduce without the Radzen script there in addition to the Nimble one. I suspect the extra script is affecting the timing of loading the Nimble one, such that sometimes it has not been loaded by the time the component renders.

## πŸ€” Expected Behavior

The dialog should successfully render every time.

## 😯 Current Behavior

The dialog fails to render sometimes and we get a Blazor error.

## πŸ’ Possible Solution

A way to resolve the race condition (if that's what happening), or guidance on a way we could have our component wait until it knows the script is ready before trying to open the dialog.

## πŸ”¦ Context

For Armstrong, we're going to be requesting that the WPF app hosting the sequencer in a web view (InstrumentStudio) be able to show a modal dialog with a web view that displays a Blazor dialog. To implement that Blazor dialog we're creating a component like the one described here, that opens immediately after rendering. Our web app has the Nimble + Radzen scripts as described here, so I'm seeing the issue some of the time.

Note that this is not a requirement for our end of June release, but is required for our C3 work.

## 🌍 Your Environment

* OS & Device: Windows 10
* Browser: Firefox
rajsite commented 5 months ago

Add references to [...] scripts in App.razor.

Our docs say to include the script in _Layout.cshtml which to my understanding is the top-level layout page where the <body> / <head> tags are defined by default. Do you have the timing issue if the nimble script tags are included in the _Layout.cshtml file instead of in App.razor as described in the docs?

Some other ideas on top of moving the script loading to _Layout.cshtml:

epetersoni commented 5 months ago

The default Blazor project templates have seen some significant updates with the last couple .NET releases. I believe _Layout.cshtml was the default place with <body> / <head> tags for the .NET 6 Blazor project templates. For our actual Armstrong code, we're still based off of the .NET 7 template, which doesn't have _Layout.cshtml and uses Pages/_Host.cshtml instead.

With the .NET 8 template, which is what I used for the reproducing example, those tags have moved into App.razor and there is no longer a .cshtml file included.

I'm not sure what has changed, but today I've been unable to reproduce using the above steps to reproduce. But it still does reproduce in our full application.

I'll try moving the Nimble script first in our full app and see if that helps.

epetersoni commented 5 months ago

With the Nimble script moved first in our full app, I've been unable to reproduce this after dozens of attempts. With the Blazor script moved back in front, I can reproduce within a few attempts.

So that appears to be a fix, and I don't notice any other differences in behavior. @rajsite Do you think that's the correct fix, or is there something else I should try?

atmgrifter00 commented 5 months ago

With the Nimble script moved first in our full app, I've been unable to reproduce this after dozens of attempts. With the Blazor script moved back in front, I can reproduce within a few attempts.

So that appears to be a fix, and I don't notice any other differences in behavior. @rajsite Do you think that's the correct fix, or is there something else I should try?

@epetersoni, could you provide some more clarity around what specific scripts you are arranging? When you say "Nimble script" are you referring to the workaround script (I presume not, as I think we've realized that this isn't relevant), or the all-components script? Presuming the latter case, it's not clear why your changes would be correcting the problem, but we don't believe it should be problematic either.

The larger concern, which I think @rajsite can expand on if needed, is whether or not there is an underlying issue (or expectation) that Microsoft has introduced for how scripts are loaded in a Blazor app. The working theory is that no scripts would attempt to be executed until after window.onload finishes.

epetersoni commented 5 months ago

@atmgrifter00 It is the latter case - I moved the all-components script to before the Blazor server script.

Some of our end-to-end tests which were already slightly intermittent started to fail much more frequently around the time of this change, but I'm not clear on how that might be related.

msmithNI commented 3 weeks ago

@epetersoni - I've re-tested this along with the .NET 8 changes going into Nimble soon ( #2454 ), but I'm unable to reproduce it still (in a standalone test app). If you still see it once we publish the .NET 8 changes, we can diagnose it further then.

(What I tried - new Blazor Web App in Visual Studio, InteractiveServer render mode (interactivity location = global), made the changes you outlined in the original issue, and the scripts are in App.razor as the following:

<script src="_framework/blazor.web.js"></script>
<script src="_content/Radzen.Blazor/Radzen.Blazor.js"></script>
<script src="_content/NimbleBlazor/nimble-components/all-components-bundle.min.js"></script>

)

msmithNI commented 3 weeks ago

@epetersoni NimbleBlazor 19.0.0 is now available which targets .NET 8.0. Note though like I said above we were unable to reproduce this on our side, so no specific changes went in for this issue. Marking this Waiting for Response for now so that you can test out the new version and see if you can still reproduce this.

epetersoni commented 2 weeks ago

@msmithNI I updated our application to Nimble 19.0.0, swapped the order of the Blazor Server / Nimble JS scripts back to having the Blazor one first, and was unable to reproduce the issue.

I did notice that with the update JS initialization was not working ( #2355 ). I'm assuming this is because we need to update our Blazor initialization code from the .NET 7 template -> .NET 8 template so that the updated Nimble JS initializers work. Doing so might end up reintroducing this bug.