fix(forms): update status classes once deferred registration resolves the control#69311
Open
gregkaczan wants to merge 1 commit into
Open
fix(forms): update status classes once deferred registration resolves the control#69311gregkaczan wants to merge 1 commit into
gregkaczan wants to merge 1 commit into
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
… the control
`NgForm` registers `ngModel`/`ngModelGroup` controls in a deferred
microtask without notifying any view. When the `NgControlStatus`/
`NgControlStatusGroup` host bindings are first checked before that
registration lands, `control` is still `null`, the optional-chained
signal reads short-circuit, and the view never establishes a reactive
dependency. In zoneless OnPush apps an `ngModelGroup` host with no
rendered controls (e.g. a collapsed form section) therefore never
receives its `ng-*` status classes and never reflects later model
changes such as `markAllAsTouched()`. The same stale binding throws
NG0100 under `provideCheckNoChangesConfig({exhaustive: true})`.
Track an internal `_controlResolved` signal on
`AbstractControlDirective`: the status host bindings read it so the
first binding pass always creates a dependency, and `NgForm`'s deferred
`addControl`/`addFormGroup` callbacks set it once the control exists.
This also covers `addControl` swapping the `NgModel` control instance
for a previously registered one.
Fixes angular#69310
ca74800 to
0f8a61b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
NgFormregistersngModel/ngModelGroupcontrols in a deferred microtask (resolvedPromise.then(...)inaddControl/addFormGroup) without notifying any view. When theNgControlStatus/NgControlStatusGrouphost bindings are first checked before that registration lands,controlis stillnull, the optional-chained signal reads inAbstractControlStatusshort-circuit, and the view never establishes a reactive dependency.In zoneless OnPush apps, an
[ngModelGroup]host with no rendered controls (the collapsed accordion/section pattern) therefore never receives itsng-*status classes, and later model changes (markAllAsTouched()etc.) are never reflected — permanently stale DOM. The same stale binding throws NG0100 on every tick underprovideCheckNoChangesConfig({exhaustive: true}).Issue Number: #69310
What is the new behavior?
AbstractControlDirectivetracks an internal_controlResolvedsignal. The status host-binding getters read it, so the first binding pass always creates a reactive dependency even whilecontrolisnull;NgForm's deferredaddControl/addFormGroupcallbacks set it once the control is registered, which wakes the host view. This also coversaddControlswapping theNgModelcontrol instance for a previously registered one.Status classes on group hosts now appear once the deferred registration lands and keep tracking the control's
_touched/_pristine/_statussignals from the second pass onward — with no behavior change for views that already worked (the signal flips once, causing at most one extra refresh per directive).Verified against the issue's reproduction: the repro spec fails on stock 21.2.16 and passes with this change ported onto it. Live demo of the patched behavior: https://stackblitz.com/github/gregkaczan/ng69310-fix-demo (broken-state counterpart linked from #69310).
Does this PR introduce a breaking change?
Other information
Side observation from the issue —
isUntouched/isDirty/isInvalid/isPendingnot reading the corresponding signals — is left as-is: they are evaluated in the same host-binding pass as their signal-reading positive counterparts, which the existing comments rely on deliberately.