Repository navigation
Conversation
A `compatForm()` field's `errorSummary` is always empty, so the root's `errorSummary()` omits every `FormControl` error while the form is invalid. Compat errors also never get a `fieldTree`. These specs assert the expected behavior and fail on purpose. The triage notes explain the root cause. A fix will replace the notes once maintainers confirm the approach.
4 of 14 tasks
Member
|
Going to close the PRs as they're won't me merged as is. Let's move the discussions to the issues. |
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
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.
Issue #71634 | Triage notes | Repro specs
Why the change
A
compatForm()field'serrorSummaryis always empty and its errors carry nofieldTree, so the root'serrorSummary()hides everyFormControlerror while the form is invalid; this draft adds failing repro specs and asks you to confirm the fix shape before I write it.Special things to note
fieldTree: the root summary sorts errors by DOM position througherror.fieldTree(), so returning compat errors without it would throw.CompatValidationState, so whichever lands second needs a rebase.Change outline
What the branch adds:
State of
f = compatForm(signal({name: new FormControl('', Validators.required)})):Why
fieldTreehas to come first:Recommended fix
Point each
FormControlerror at its compat field withaddDefaultField(), the helper plainvalidation rules already use, and make the summary the field's own errors
(
compat/src/compat_validation_state.ts):extractNestedReactiveErrors()creates new error objects on every status change, so settingfieldTreeon them doesn't touch shared state.Alternative considered
Add a
fieldTreeparameter toCompatValidationError's constructor. That's a public API change,and
extractNestedReactiveErrors()would need the field passed down. Not recommended.Design decisions
FormGrouporFormArrayvalue. All of them point at the compat field, because nested controlshave no
FieldTreeof their own.fieldTreeis set. InCompatValidationState, so the publicCompatValidationErrorclass doesn't change.
[formControl]instead of[formField]has nobound element, so its errors sort to the end of the root summary.
Questions for maintainers
errors()the right summary, with nestedFormControlerrors pointingat the compat field?
addDefaultField()inCompatValidationStatethe right place to setfieldTree, rather thanthe
CompatValidationErrorconstructor?[formField]binding sort last?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?
Issue Number: #71634
What is the new behavior?
No behavior changes yet. This PR adds failing repro specs and triage notes. The fix will replace the
notes on this branch once the questions above are answered.
Does this PR introduce a breaking change?