fix(material/chips): emit null instead of undefined when deselecting the only selected chip - #33708
Open
CSchulz wants to merge 1 commit into
Open
fix(material/chips): emit null instead of undefined when deselecting the only selected chip#33708CSchulz wants to merge 1 commit into
CSchulz wants to merge 1 commit into
Conversation
…the only selected chip MatChipListbox._propagateChanges() emitted `undefined` as the control value whenever a single-select listbox went from one chip selected to none selected. Consumers that treat `undefined` and `null` differently (e.g. Angular Signal Forms, which uses `undefined` to mean "this field no longer exists" internally) can crash on this transition. Emit `null` instead, matching the "nothing selected" convention already used elsewhere in Angular forms (e.g. FormControl's own initial value). The multi-select branch is unaffected: it already emits `[]` for an empty selection.
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.
Summary
MatChipListbox._propagateChanges()emitsundefinedas the control value whenever a single-select listbox transitions from one chip selected to none selected (e.g. the user clicks the selected chip again to deselect it). This is inconsistent with:[]for an empty selection rather thanundefined.nullis the conventional "nothing selected" sentinel (e.g.FormControl's own default value).More importantly, some consumers assign a different meaning to
undefinedthan to an empty/absent value. We hit this with Angular's new Signal Forms (@angular/forms/signals):its internal
FieldNodeStructure.computeChildrenMap()treats a model property whose value is strictlyundefinedas "this field no longer exists" and removes its childFieldNodeaccordingly. Whenmat-chip-listboxwritesundefinedinto a signal-forms-bound field on deselect, the field's own node gets deleted from the framework's internal map. Any subsequent access (includingFormField's ownstatecomputed, which doesthis.field()()) then finds no field there, and the readundefined()throwsTypeError: this.field(...) is not a function. This is easy to trigger with a plain double-click (select, then deselect) on a chip bound via[formField].Since
undefinedandnullare not interchangeable to that kind of consumer, andnullis the type-agnostic, form-idiomatic choice (unlike'', which would be wrong for non-string chip values), this PR changes the single-select branch to emitnullinstead.Root cause, verified
Confirmed by reading
computeChildrenMap()in@angular/forms's signals implementation: it explicitly special-caseschildValue === undefinedto delete the corresponding child field entry, but has no such handling fornull. Reproduced the crash in an isolated minimal Angular 22 app (mat-chip-listbox [formField]="field", single-select, backed by aform()from@angular/forms/signals) and confirmed:dblclick()on a chip crashes immediately withTypeError: this.field(...) is not a function; repeating the interaction produced 16 crashes across 6 toggles.''for a string field, viaFormField's own value normalization) instead of vanishing.Test plan
should propagate null, not undefined, when the selected chip is deselectedtochip-listbox.spec.ts, alongside the existing single-selection reactive-forms tests.material/chipsunit test suite (pnpm test chips) — 244/244 passing.toBeUndefined()/toBe(undefined)assertion in the spec file; none of them exercise the code path this changes (they cover.selectedafter chip removal, a disabled-chip path that never reaches_propagateChanges(), and the multi-select branch, which already emitted[]).MatChipListbox/MatChipListboxChange— none exist outside thematerial/chipspackage itself, so there's no cross-package fallout.Fixes #33705