feat(Button): add release OnClickWithouRender reference#7892
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideEnsures ButtonBase clears the OnClickWithoutRender callback reference during asynchronous disposal to prevent lingering event handlers or memory leaks. Class diagram for updated ButtonBase disposal behaviorclassDiagram
class ButtonBase {
+bool IsAsync
+EventCallback OnClick
+EventCallback OnClickWithoutRender
+ValidateFormComponent ValidateForm
+ValueTask DisposeAsync(bool disposing)
}
class ValidateFormComponent {
+void UnregisterAsyncSubmitButton(ButtonBase button)
}
ButtonBase --> ValidateFormComponent : uses
Flow diagram for ButtonBase.DisposeAsync with OnClickWithoutRender clearingflowchart TD
A[DisposeAsync disposing == true] --> B{OnClick != EventCallback.Empty?}
B -- yes --> C[Set OnClick to EventCallback.Empty]
B -- no --> D[Skip clearing OnClick]
C --> E{OnClickWithoutRender != null?}
D --> E
E -- yes --> F[Set OnClickWithoutRender to null]
E -- no --> G[Skip clearing OnClickWithoutRender]
F --> H{IsAsync == true and ValidateForm != null?}
G --> H
H -- yes --> I[ValidateForm.UnregisterAsyncSubmitButton this]
H -- no --> J[Skip unregister]
I --> K[Continue disposal]
J --> K[Continue disposal]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- For consistency with the existing
OnClickcleanup, consider resettingOnClickWithoutRenderin the same style (e.g., to an appropriate empty/default callback) rather than just setting it tonull, assuming it is also anEventCallbackor similar delegate. - The null check before
OnClickWithoutRender = null;is redundant; you can safely assignnulldirectly without the conditional block.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- For consistency with the existing `OnClick` cleanup, consider resetting `OnClickWithoutRender` in the same style (e.g., to an appropriate empty/default callback) rather than just setting it to `null`, assuming it is also an `EventCallback` or similar delegate.
- The null check before `OnClickWithoutRender = null;` is redundant; you can safely assign `null` directly without the conditional block.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
This PR updates the ButtonBase component cleanup logic to explicitly release the OnClickWithoutRender delegate reference during disposal, aligning with the goal of reducing retained references after a button is removed.
Changes:
- Clear
OnClickWithoutRenderinDisposeAsyncto release the delegate reference when the component is disposed.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7892 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 764 764
Lines 34173 34177 +4
Branches 4702 4703 +1
=========================================
+ Hits 34173 34177 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Link issues
fixes #7891
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Bug Fixes: