Skip to content

Commit 9748b0d

Browse files
leonsenftcrisbeto
authored andcommitted
fix(forms): support custom controls with non signal-based models
* Recognize directives with non signal-based models as valid custom controls * Relax type checker to allow non signal-based models The `FormValueControl` and `FormCheckboxControl` interfaces still require a `model()`-input, however, a custom control need not implement either interface to be bound by the `Field` directive. All of the following examples can be used to define a custom control: ```ts // Preferred: model() class MyFormControl implements FormValueControl<string> { readonly value: model.required<string>(); } // Supported: input() + output() class MyFormControl { readonly value: input.required<string>(); readonly valueChange: output<string>(); } // Supported: @input() + @output() class MyFormControl { @input({required: true}) value!: string; @output() valueChange: new EventEmitter<string>(); } ``` The latter two may still choose to implement `FormUiControl` for other properties, but again it is not required. Fix #65478 (cherry picked from commit cb09fb8)
1 parent ab86c90 commit 9748b0d

File tree

3 files changed

+166
-10
lines changed

3 files changed

+166
-10
lines changed

packages/compiler-cli/src/ngtsc/typecheck/src/ops/signal_forms.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -440,10 +440,9 @@ function extractFieldValue(expression: AST, tcb: Context, scope: Scope): ts.Expr
440440
);
441441
}
442442

443-
/** Checks whether a directive has a model input with a specific name. */
443+
/** Checks whether a directive has a model-like input with a specific name. */
444444
function hasModelInput(name: string, meta: TypeCheckableDirectiveMeta): boolean {
445445
return (
446-
!!meta.inputs.getByBindingPropertyName(name)?.some((v) => v.isSignal) &&
447-
meta.outputs.hasBindingPropertyName(name + 'Change')
446+
meta.inputs.hasBindingPropertyName(name) && meta.outputs.hasBindingPropertyName(name + 'Change')
448447
);
449448
}

packages/core/src/render3/instructions/control.ts

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import {assertFirstCreatePass} from '../assert';
1212
import {bindingUpdated} from '../bindings';
1313
import {ɵCONTROL, ɵControl, ɵFieldState} from '../interfaces/control';
1414
import {DirectiveDef} from '../interfaces/definition';
15-
import {InputFlags} from '../interfaces/input_flags';
1615
import {TElementNode, TNode, TNodeFlags, TNodeType} from '../interfaces/node';
1716
import {Renderer} from '../interfaces/renderer';
1817
import {SanitizerFn} from '../interfaces/sanitization';
@@ -296,15 +295,19 @@ function getControlDirective<T>(tNode: TNode, lView: LView): ɵControl<T> | null
296295
return index === -1 ? null : lView[index];
297296
}
298297

299-
/** Returns whether the specified `directiveDef` has a model input named `name`. */
298+
/**
299+
* Returns whether the specified `directiveDef` has a model-like input named `name`.
300+
*
301+
* A model-like input is an input-output pair where the input is named `name` and the output is
302+
* named `name + 'Change'`.
303+
*/
300304
function hasModelInput(directiveDef: DirectiveDef<unknown>, name: string): boolean {
301-
return hasSignalInput(directiveDef, name) && hasOutput(directiveDef, name + 'Change');
305+
return hasInput(directiveDef, name) && hasOutput(directiveDef, name + 'Change');
302306
}
303307

304-
/** Returns whether the specified `directiveDef` has a signal-based input named `name`.*/
305-
function hasSignalInput(directiveDef: DirectiveDef<unknown>, name: string): boolean {
306-
const input = directiveDef.inputs[name];
307-
return input && (input[1] & InputFlags.SignalBased) !== 0;
308+
/** Returns whether the specified `directiveDef` has an input named `name`.*/
309+
function hasInput(directiveDef: DirectiveDef<unknown>, name: string): boolean {
310+
return name in directiveDef.inputs;
308311
}
309312

310313
/** Returns whether the specified `directiveDef` has an output named `name`. */

packages/forms/signals/test/web/field_directive.spec.ts

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,16 @@ import {
1212
computed,
1313
Directive,
1414
ElementRef,
15+
EventEmitter,
1516
inject,
1617
Injector,
1718
input,
1819
inputBinding,
20+
Input,
1921
model,
2022
numberAttribute,
23+
output,
24+
Output,
2125
resource,
2226
signal,
2327
viewChild,
@@ -1495,6 +1499,80 @@ describe('field directive', () => {
14951499
expect(cmp.f().value()).toBe('typing');
14961500
});
14971501

1502+
it('synchronizes with a custom value control with separate input and output properties', () => {
1503+
@Component({
1504+
selector: 'my-input',
1505+
template: '<input #i [value]="value()" (input)="valueChange.emit(i.value)" />',
1506+
})
1507+
class CustomInput {
1508+
readonly value = input.required<string>();
1509+
readonly valueChange = output<string>();
1510+
}
1511+
1512+
@Component({
1513+
imports: [Field, CustomInput],
1514+
template: `<my-input [field]="f" />`,
1515+
})
1516+
class TestCmp {
1517+
f = form<string>(signal('test'));
1518+
}
1519+
1520+
const fix = act(() => TestBed.createComponent(TestCmp));
1521+
const element = fix.nativeElement.firstChild.firstChild as HTMLInputElement;
1522+
const cmp = fix.componentInstance as TestCmp;
1523+
1524+
// Initial state
1525+
expect(element.value).toBe('test');
1526+
1527+
// Model -> View
1528+
act(() => cmp.f().value.set('testing'));
1529+
expect(element.value).toBe('testing');
1530+
1531+
// View -> Model
1532+
act(() => {
1533+
element.value = 'typing';
1534+
element.dispatchEvent(new Event('input'));
1535+
});
1536+
expect(cmp.f().value()).toBe('typing');
1537+
});
1538+
1539+
it('synchronizes with a custom value control with separate @Input and @Output properties', () => {
1540+
@Component({
1541+
selector: 'my-input',
1542+
template: '<input #i [value]="value" (input)="valueChange.emit(i.value)" />',
1543+
})
1544+
class CustomInput {
1545+
@Input({required: true}) value!: string;
1546+
@Output() valueChange = new EventEmitter<string>();
1547+
}
1548+
1549+
@Component({
1550+
imports: [Field, CustomInput],
1551+
template: `<my-input [field]="f" />`,
1552+
})
1553+
class TestCmp {
1554+
f = form<string>(signal('test'));
1555+
}
1556+
1557+
const fix = act(() => TestBed.createComponent(TestCmp));
1558+
const element = fix.nativeElement.firstChild.firstChild as HTMLInputElement;
1559+
const cmp = fix.componentInstance as TestCmp;
1560+
1561+
// Initial state
1562+
expect(element.value).toBe('test');
1563+
1564+
// Model -> View
1565+
act(() => cmp.f().value.set('testing'));
1566+
expect(element.value).toBe('testing');
1567+
1568+
// View -> Model
1569+
act(() => {
1570+
element.value = 'typing';
1571+
element.dispatchEvent(new Event('input'));
1572+
});
1573+
expect(cmp.f().value()).toBe('typing');
1574+
});
1575+
14981576
it('synchronizes with a custom checkbox control', () => {
14991577
@Component({
15001578
selector: 'my-checkbox',
@@ -1532,6 +1610,82 @@ describe('field directive', () => {
15321610
expect(cmp.f().value()).toBe(true);
15331611
});
15341612

1613+
it('synchronizes with a custom checkbox control with separate input and output properties', () => {
1614+
@Component({
1615+
selector: 'my-checkbox',
1616+
template:
1617+
'<input type="checkbox" #i [checked]="checked()" (input)="checkedChange.emit(i.checked)" />',
1618+
})
1619+
class CustomCheckbox {
1620+
readonly checked = input.required<boolean>();
1621+
readonly checkedChange = output<boolean>();
1622+
}
1623+
1624+
@Component({
1625+
imports: [Field, CustomCheckbox],
1626+
template: `<my-checkbox [field]="f" />`,
1627+
})
1628+
class TestCmp {
1629+
f = form<boolean>(signal(true));
1630+
}
1631+
1632+
const fix = act(() => TestBed.createComponent(TestCmp));
1633+
const element = fix.nativeElement.querySelector('input') as HTMLInputElement;
1634+
const cmp = fix.componentInstance as TestCmp;
1635+
1636+
// Initial state
1637+
expect(element.checked).toBe(true);
1638+
1639+
// Model -> View
1640+
act(() => cmp.f().value.set(false));
1641+
expect(element.checked).toBe(false);
1642+
1643+
// View -> Model
1644+
act(() => {
1645+
element.checked = true;
1646+
element.dispatchEvent(new Event('input'));
1647+
});
1648+
expect(cmp.f().value()).toBe(true);
1649+
});
1650+
1651+
it('synchronizes with a custom checkbox control with separate @Input and @Output properties', () => {
1652+
@Component({
1653+
selector: 'my-checkbox',
1654+
template:
1655+
'<input type="checkbox" #i [checked]="checked" (input)="checkedChange.emit(i.checked)" />',
1656+
})
1657+
class CustomCheckbox {
1658+
@Input({required: true}) checked!: boolean;
1659+
@Output() checkedChange = new EventEmitter<boolean>();
1660+
}
1661+
1662+
@Component({
1663+
imports: [Field, CustomCheckbox],
1664+
template: `<my-checkbox [field]="f" />`,
1665+
})
1666+
class TestCmp {
1667+
f = form<boolean>(signal(true));
1668+
}
1669+
1670+
const fix = act(() => TestBed.createComponent(TestCmp));
1671+
const element = fix.nativeElement.querySelector('input') as HTMLInputElement;
1672+
const cmp = fix.componentInstance as TestCmp;
1673+
1674+
// Initial state
1675+
expect(element.checked).toBe(true);
1676+
1677+
// Model -> View
1678+
act(() => cmp.f().value.set(false));
1679+
expect(element.checked).toBe(false);
1680+
1681+
// View -> Model
1682+
act(() => {
1683+
element.checked = true;
1684+
element.dispatchEvent(new Event('input'));
1685+
});
1686+
expect(cmp.f().value()).toBe(true);
1687+
});
1688+
15351689
it('synchronizes with a custom checkbox control directive', () => {
15361690
@Directive({
15371691
selector: 'input[my-checkbox]',

0 commit comments

Comments
 (0)