Skip to content

test(forms): add repro specs for compat errorSummary - #71630

Closed
Flarette wants to merge 1 commit into
angular:mainfrom
Flarette:triage/forms-compat-error-summary
Closed

Flarette wants to merge 1 commit into
angular:mainfrom
Flarette:triage/forms-compat-error-summary

Conversation

@Flarette

@Flarette Flarette commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Issue #71634 | Triage notes | Repro specs

Why the change

A compatForm() field's errorSummary is always empty and its errors carry no fieldTree, so the root's errorSummary() hides every FormControl error 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

  • This is a triage draft with no fix. The 2 repro specs fail on purpose, so CI is red.
  • The summary fix depends on fieldTree: the root summary sorts errors by DOM position through
    error.fieldTree(), so returning compat errors without it would throw.
  • test(forms): add repro specs for compat parse errors #71628 also changes CompatValidationState, so whichever lands second needs a rebase.

Change outline

What the branch adds:

packages/forms/signals/
├── test/node/compat/compat_error_summary.spec.ts   # 2 failing repros, 1 passing control spec
└── triage/compat-error-summary.md                  # observed output, root cause, sort hazard

State of f = compatForm(signal({name: new FormControl('', Validators.required)})):

 f.name().errors()
-  → [{kind: 'required', fieldTree: undefined}]
+  → [{kind: 'required', fieldTree: f.name}]
 f.name().errorSummary()                 shared EMPTY_ARRAY_SIGNAL today
-  → []
+  → [{kind: 'required', fieldTree: f.name}]
 f().errorSummary()                      the root's errors, then each child's summary
-  → []
+  → [{kind: 'required', fieldTree: f.name}]
 f().invalid()
   → true

Why fieldTree has to come first:

f().errorSummary()
  reduceChildren(errors, child.errorSummary())
  errors.sort(compareErrorPosition)           client only
    getFirstBoundElement(error)
      error.formField?.element                undefined for FormControl errors
      error.fieldTree().formFieldBindings()   TypeError when fieldTree is undefined

Recommended fix

Point each FormControl error at its compat field with addDefaultField(), the helper plain
validation rules already use, and make the summary the field's own errors
(compat/src/compat_validation_state.ts):

-    this.errors = getControlStatusSignal(options, extractNestedReactiveErrors);
+    // Point each `FormControl` error at this field, like plain validation rules do.
+    this.errors = getControlStatusSignal(options, (c) =>
+      addDefaultField(extractNestedReactiveErrors(c), this.node.fieldTree),
+    );

-  errorSummary: Signal<ValidationError.WithFieldTree[]> = EMPTY_ARRAY_SIGNAL;
+  // A compat field has no children, so its summary is its own errors.
+  get errorSummary(): Signal<ValidationError.WithFieldTree[]> {
+    return this.errors;
+  }

extractNestedReactiveErrors() creates new error objects on every status change, so setting
fieldTree on them doesn't touch shared state.

Alternative considered

Add a fieldTree parameter to CompatValidationError's constructor. That's a public API change,
and extractNestedReactiveErrors() would need the field passed down. Not recommended.

Design decisions

  1. What a compat field's summary holds. Its own errors, including the nested errors of a
    FormGroup or FormArray value. All of them point at the compat field, because nested controls
    have no FieldTree of their own.
  2. Where fieldTree is set. In CompatValidationState, so the public CompatValidationError
    class doesn't change.
  3. Sort order. A compat field that's bound with [formControl] instead of [formField] has no
    bound element, so its errors sort to the end of the root summary.

Questions for maintainers

  1. Is a compat field's own errors() the right summary, with nested FormControl errors pointing
    at the compat field?
  2. Is addDefaultField() in CompatValidationState the right place to set fieldTree, rather than
    the CompatValidationError constructor?
  3. Is it acceptable that errors of compat fields without a [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?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.dev application / infrastructure changes
  • Other... Please describe: triage. Failing repro specs and triage notes, with no library changes.

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?

  • Yes
  • No

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.
@JeanMeche

Copy link
Copy Markdown
Member

Going to close the PRs as they're won't me merged as is. Let's move the discussions to the issues.

@JeanMeche JeanMeche closed this Oct 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants