Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible race condition showing Blazor NimbleDialog during OnAfterRenderAsync #2188

Open
epetersoni opened this issue Jun 12, 2024 · 8 comments
Assignees
Labels
bug Something isn't working waiting for response Blocked on response from reporter

Comments

@epetersoni
Copy link
Contributor

🐛 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<string> _dialog = null!;

    protected override async Task OnAfterRenderAsync(bool firstRender)
    {
        if (firstRender)
        {
            await _dialog.ShowAsync();
        }
        await base.OnAfterRenderAsync(firstRender);
    }
}
  1. 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
@epetersoni epetersoni added bug Something isn't working triage New issue that needs to be reviewed labels Jun 12, 2024
@rajsite
Copy link
Member

rajsite commented Jun 12, 2024

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:

  • If the nimble script is always the first script (before any other blazor scripts, third party scripts, etc) does the issue reproduce?
  • Can you confirm how the script is loaded synchronously with <script src="_content/NimbleBlazor/nimble-components/all-components-bundle.min.js"></script>, note it does not have type=module, or defer / async specified.

@epetersoni
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor Author

@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.

@m-akinc m-akinc added needs investigation Further research is needed to identify solution and removed triage New issue that needs to be reviewed labels Jun 18, 2024
@msmithNI msmithNI mentioned this issue Jul 26, 2024
11 tasks
@msmithNI msmithNI self-assigned this Sep 25, 2024
@msmithNI msmithNI removed the needs investigation Further research is needed to identify solution label Oct 28, 2024
@msmithNI
Copy link
Contributor

@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>

)

rajsite pushed a commit that referenced this issue Nov 1, 2024
# Pull Request

## 🤨 Rationale

Resolves #1667
- Split off resolving more code analysis suppressions to #2453 
- #2355 : Resolved by updates to Nimble/Spright JS initialization
scripts for .NET 8
- #2188 : Not reproducible by Nimble team, can debug further once
changes published if client can still reproduce

## 👩‍💻 Implementation

- Projects in Blazor solution now target `net8.0` (formerly `net6.0`)
- Update other .NET dependencies, and code analyzer package versions
- Update docs
- Re-build `Demo.Server` example based on new Blazor Web App template
- Update acceptance test projects to use similar setup as Blazor Web App
templates
- `NimbleBlazor.Tests.Acceptance.Client`: Existing tests use the
`InteractiveServer` render mode. The project also now supports tests
using `InteractiveWebAssembly` and static SSR render modes. Add one
example test for each.
- TODO: Spright acceptance test project currently don't use the new
setup yet. If the changes look reasonable and we want to do the same for
the Spright tests, I can do so in an update.

## 🧪 Testing

- Manual testing
- Autotests (unit/acceptance) pass. New tests for Nimble for
InteractiveWebAssembly / static SSR render modes.

## ✅ Checklist

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed.
@msmithNI
Copy link
Contributor

msmithNI commented Nov 1, 2024

@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.

@msmithNI msmithNI added the waiting for response Blocked on response from reporter label Nov 1, 2024
@epetersoni
Copy link
Contributor Author

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working waiting for response Blocked on response from reporter
Projects
Status: Backlog
Development

No branches or pull requests

5 participants