fix(elements): Trigger additional change detection when method is invoked in elements - #17580
fix(elements): Trigger additional change detection when method is invoked in elements#17580mddragnev wants to merge 4 commits into
Conversation
mddragnev
added
❌ status: awaiting-test
There was a problem hiding this comment.
🟡 Changes recommended
The current implementation schedules change detection unconditionally after method calls, which can cause redundant global ticks in zone-enabled builds and should be gated to the zoneless case.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR addresses missing change detection in Angular Elements when running without zone.js by explicitly scheduling Angular change detection after invoking configured component instance methods from the custom element wrapper (motivated by #17581 affecting grid search clear behavior).
Changes:
- Add
notifyChanges()to the custom elements strategy to manually notify Angular’s change detection scheduler. - Wrap generated element method proxies to schedule change detection after invoking the underlying component method.
- Add a unit test covering grid search clear behavior to prevent the “mirrored cell value” stale rendering state.
File summaries
| File | Description |
|---|---|
| projects/igniteui-angular-elements/src/app/custom-strategy.ts | Adds notifyChanges() hook for manually scheduling change detection; minor local refactor (let → const). |
| projects/igniteui-angular-elements/src/app/create-custom-element.ts | Updates method proxy generation to invoke notifyChanges() after calling the component method. |
| projects/igniteui-angular-elements/src/app/custom-strategy.spec.ts | Adds a regression test ensuring UI updates after invoking search-related methods (findNext, clearSearch). |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
damyanpetev
left a comment
damyanpetev
left a comment
There was a problem hiding this comment.
Minor note on the comment, otherwise LGTM
| // Angular normally wraps listeners and schedules change detection to preserve Zone.js behavior. | ||
| // Like Angular Elements' setInputValue, we notify the scheduler explicitly because custom-element methods bypass that listener path. | ||
| // This behavior may change in a future Angular version. | ||
| // https://github.com/angular/angular/blob/9a58353b1b680f162a55969965ae6a90ae20316d/packages/core/src/change_detection/scheduling/zoneless_scheduling_impl.ts#L140 |
There was a problem hiding this comment.
https://github.com/angular/angular/blob/9a58353b1b680f162a55969965ae6a90ae20316d/packages/core/src/render3/view/listeners.ts#L46 might be the more useful link here, though it's a combo with this one.
Closes #17581
Description
Needed because of
zone.jsremovalMotivation / Context
Without zone.js, native browser event listeners do not automatically trigger Angular change detection when component methods are invoked from event callbacks. Because UI state changes may occur across various public methods, change detection is defensively scheduled after every method call to ensure consistent state updates.
Type of Change (check all that apply):
Component(s) / Area(s) Affected:
How Has This Been Tested?
Test Configuration:
Screenshots / Recordings
Checklist:
feature/README.MDupdates for the feature docsREADME.MDCHANGELOG.MDupdates for newly added functionalityng updatemigrations for the breaking changes (migrations guidelines)