Skip to content

[Menu] Unregister disposed menu instances - #5035

Merged
vnbaaij merged 1 commit into
microsoft:devfrom
JamesNK:jamesnk/fix-fluentmenu-disposal
Jul 21, 2026
Merged

[Menu] Unregister disposed menu instances#5035
vnbaaij merged 1 commit into
microsoft:devfrom
JamesNK:jamesnk/fix-fluentmenu-disposal

Conversation

@JamesNK

@JamesNK JamesNK commented Jul 21, 2026

Copy link
Copy Markdown
Member

Pull Request

📖 Description

FluentMenu registered service-backed instances during initialization but never called the existing IMenuService.Remove operation during disposal. The scoped service therefore retained disposed menus, including their render fragments, callbacks, and parent component state, for the circuit lifetime.

This change tracks the exact service instance that accepted each registration and unregisters the menu before disposing JavaScript resources. MenuService.Remove now removes the exact component instance and notifies the provider only when removal succeeds, keeping repeated disposal idempotent and allowing provider render state to release the menu.

🎫 Issues

Fixes #5032

👩‍💻 Reviewer Notes

Please focus on the service-backed menu lifecycle in FluentMenu.DisposeAsync and the successful-removal notification in MenuService.Remove.

No smoke test is required; the regression is covered at the component/service lifecycle boundary.

📑 Test Plan

  • dotnet test tests\Core\Microsoft.FluentUI.AspNetCore.Components.Tests.csproj --no-restore --filter "FullyQualifiedName~Microsoft.FluentUI.AspNetCore.Components.Tests.Menu.FluentMenuTests" --verbosity minimal
  • 6 FluentMenu tests passed, including registration/unregistration, provider notification, repeated disposal, and duplicate-ID isolation.

✅ Checklist

General

  • I have added tests for my changes.
  • I have tested my changes.
  • I have updated the project documentation to reflect my changes.
  • I have read the CONTRIBUTING documentation and followed the standards for this project.

Component-specific

  • I have added a new component
  • I have added Unit Tests for my new component
  • I have modified an existing component
  • I have validated the Unit Tests for an existing component

⏭ Next Steps

None.

Unregister service-backed menus during disposal and notify the provider after successful removal. Add lifecycle regression coverage for registration, idempotency, and duplicate IDs.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 8541ad78-5e1c-4374-a3a5-7482a945eb69
Copilot AI review requested due to automatic review settings July 21, 2026 10:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a lifecycle leak in the menu system by ensuring FluentMenu instances registered into the scoped IMenuService are properly unregistered during disposal, allowing provider render state and captured component graphs to be released for the circuit lifetime.

Changes:

  • Track the exact IMenuService instance used to register a FluentMenu, and unregister it in FluentMenu.DisposeAsync() before JS cleanup.
  • Update MenuService.Remove to remove the exact component instance (not “first by Id”) and only notify the provider when a removal actually occurs (idempotent repeat disposal).
  • Add unit tests covering unregister-on-dispose, idempotent repeated disposal, and duplicate-ID isolation.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
tests/Core/_ToDo/Menu/FluentMenuTests.razor.cs Adds coverage for registration/unregistration behavior and provider notification semantics during disposal.
src/Core/Components/Menu/Services/MenuService.cs Makes removal instance-precise and only invokes OnMenuUpdated when a removal succeeds.
src/Core/Components/Menu/FluentMenu.razor.cs Tracks the registering service and unregisters from it during DisposeAsync before JS resource disposal.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@vnbaaij
vnbaaij merged commit 737f589 into microsoft:dev Jul 21, 2026
2 checks passed
This was referenced Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FluentMenu instances are never removed from IMenuService when disposed

3 participants