Skip to content

Commit d3946dd

Browse files
committed
refactor(forms): convert FieldNode.controlValue to a WritableSignal
Remove `setControlValue()` from `Field` and convert `controlValue` to a `WritableSignal` whose setter implements the debounced syncing behavior of `setControlValue()`. Also make sure that all signal reads during a sync are untracked.
1 parent 0424710 commit d3946dd

5 files changed

Lines changed: 80 additions & 50 deletions

File tree

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,7 @@ function initializeCustomControl(
346346
directiveIndex,
347347
outputName,
348348
outputName,
349-
wrapListener(tNode, lView, (value: unknown) => fieldDirective.state().setControlValue(value)),
349+
wrapListener(tNode, lView, (value: unknown) => fieldDirective.state().controlValue.set(value)),
350350
);
351351

352352
const directiveDef = tView.data[directiveIndex] as DirectiveDef<unknown>;
@@ -372,7 +372,7 @@ function initializeCustomControl(
372372
function initializeInteropControl(fieldDirective: ɵFormFieldDirective<unknown>): void {
373373
const interopControl = fieldDirective.ɵinteropControl!;
374374
interopControl.registerOnChange((value: unknown) =>
375-
fieldDirective.state().setControlValue(value),
375+
fieldDirective.state().controlValue.set(value),
376376
);
377377
interopControl.registerOnTouched(() => fieldDirective.state().markAsTouched());
378378
}
@@ -421,7 +421,7 @@ function initializeNativeControl(
421421

422422
const inputListener = () => {
423423
const state = fieldDirective.state();
424-
state.setControlValue(getNativeControlValue(element, state.value));
424+
state.controlValue.set(getNativeControlValue(element, state.value));
425425
};
426426
listenToDomEvent(
427427
tNode,

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

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ export interface ɵFieldState<T> {
190190
* buffer debounced updates from the control to the field. This will also not take into account
191191
* the {@link controlValue} of children.
192192
*/
193-
readonly controlValue: Signal<T>;
193+
readonly controlValue: WritableSignal<T>;
194194

195195
/**
196196
* Sets the dirty status of the field to `true`.
@@ -201,9 +201,4 @@ export interface ɵFieldState<T> {
201201
* Sets the touched status of the field to `true`.
202202
*/
203203
markAsTouched(): void;
204-
205-
/**
206-
* Sets {@link controlValue} immediately and triggers synchronization to {@link value}.
207-
*/
208-
setControlValue(value: T): void;
209204
}

packages/forms/signals/src/field/node.ts

Lines changed: 51 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,16 @@
77
*/
88

99
import {computed, linkedSignal, type Signal, untracked, type WritableSignal} from '@angular/core';
10+
import {
11+
createLinkedSignal,
12+
LinkedSignalGetter,
13+
LinkedSignalNode,
14+
linkedSignalSetFn,
15+
linkedSignalUpdateFn,
16+
SIGNAL,
17+
SignalGetter,
18+
SignalNode,
19+
} from '@angular/core/primitives/signals';
1020
import type {FormField} from '../api/form_field_directive';
1121
import {
1222
MAX,
@@ -58,6 +68,7 @@ export class FieldNode implements FieldState<unknown> {
5868
readonly nodeState: FieldNodeState;
5969
readonly submitState: FieldSubmitState;
6070
readonly fieldAdapter: FieldAdapter;
71+
readonly controlValue: WritableSignal<unknown>;
6172

6273
private _context: FieldContext<unknown> | undefined = undefined;
6374
get context(): FieldContext<unknown> {
@@ -78,6 +89,7 @@ export class FieldNode implements FieldState<unknown> {
7889
this.nodeState = this.fieldAdapter.createNodeState(this, options);
7990
this.metadataState = new FieldMetadataState(this);
8091
this.submitState = new FieldSubmitState(this);
92+
this.controlValue = this.controlValueSignal();
8193
}
8294

8395
focusBoundControl(options?: FocusOptions): void {
@@ -136,11 +148,6 @@ export class FieldNode implements FieldState<unknown> {
136148
return this.structure.value;
137149
}
138150

139-
private _controlValue = linkedSignal(() => this.value());
140-
get controlValue(): Signal<unknown> {
141-
return this._controlValue.asReadonly();
142-
}
143-
144151
get keyInParent(): Signal<string | number> {
145152
return this.structure.keyInParent;
146153
}
@@ -279,15 +286,26 @@ export class FieldNode implements FieldState<unknown> {
279286
}
280287

281288
/**
282-
* Sets the control value of the field. This value may be debounced before it is synchronized with
283-
* the field's {@link value} signal, depending on the debounce configuration.
289+
* Creates a linked signal that initialiates a {@link debounceSync} when set.
284290
*/
285-
setControlValue(newValue: unknown): void {
286-
untracked(() => {
287-
this._controlValue.set(newValue);
291+
private controlValueSignal(): WritableSignal<unknown> {
292+
const signal = createLinkedSignal(this.value, identityFn);
293+
const node = signal[SIGNAL] as LinkedSignalNode<unknown, unknown>;
294+
const writableSignal = signal as LinkedSignalGetter<unknown, unknown> & WritableSignal<unknown>;
295+
296+
writableSignal.set = (newValue) => {
297+
linkedSignalSetFn(node, newValue);
288298
this.markAsDirty();
289299
this.debounceSync();
290-
});
300+
};
301+
writableSignal.update = (updateFn) => {
302+
linkedSignalUpdateFn(node, updateFn);
303+
this.markAsDirty();
304+
this.debounceSync();
305+
};
306+
writableSignal.asReadonly = signalAsReadonlyFn.bind(signal);
307+
308+
return writableSignal;
291309
}
292310

293311
/**
@@ -312,14 +330,16 @@ export class FieldNode implements FieldState<unknown> {
312330
* Initiates a debounced {@link sync}.
313331
*
314332
* If a debouncer is configured, the synchronization will occur after the debouncer resolves. If
315-
* no debouncer is configured, the synchronization happens immediately. If {@link setControlValue}
316-
* is called again while a debounce is pending, the previous debounce operation is aborted in
317-
* favor of the new one.
333+
* no debouncer is configured, the synchronization happens immediately. If {@link controlValue} is
334+
* updated again while a debounce is pending, the previous debounce operation is aborted in favor
335+
* of the new one.
318336
*/
319337
private async debounceSync() {
320-
this.pendingSync()?.abort();
338+
const debouncer = untracked(() => {
339+
this.pendingSync()?.abort();
340+
return this.nodeState.debouncer();
341+
});
321342

322-
const debouncer = this.nodeState.debouncer();
323343
if (debouncer) {
324344
const controller = new AbortController();
325345
const promise = debouncer(controller.signal);
@@ -415,3 +435,18 @@ function firstInDom<T extends FormField<unknown>>(
415435
const position = a.element.compareDocumentPosition(b.element);
416436
return position & Node.DOCUMENT_POSITION_PRECEDING ? b : a;
417437
}
438+
439+
function identityFn<T>(v: T): T {
440+
return v;
441+
}
442+
443+
// TODO: Can we export/share this from @angular/core?
444+
function signalAsReadonlyFn<T>(this: SignalGetter<T>): Signal<T> {
445+
const node = this[SIGNAL] as SignalNode<T> & {readonlyFn?: Signal<T>};
446+
if (node.readonlyFn === undefined) {
447+
const readonlyFn = () => this();
448+
(readonlyFn as any)[SIGNAL] = node;
449+
node.readonlyFn = readonlyFn as Signal<T>;
450+
}
451+
return node.readonlyFn;
452+
}

packages/forms/signals/test/node/api/debounce.spec.ts

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ describe('debounce', () => {
2323
);
2424
const street = addressForm.street();
2525

26-
street.setControlValue('1600 Amphitheatre Pkwy');
26+
street.controlValue.set('1600 Amphitheatre Pkwy');
2727
expect(street.controlValue()).toBe('1600 Amphitheatre Pkwy');
2828
expect(street.value()).toBe('1600 Amphitheatre Pkwy');
2929
});
@@ -39,7 +39,7 @@ describe('debounce', () => {
3939
);
4040
const street = addressForm.street();
4141

42-
street.setControlValue('1600 Amphitheatre Pkwy');
42+
street.controlValue.set('1600 Amphitheatre Pkwy');
4343
expect(street.controlValue()).toBe('1600 Amphitheatre Pkwy');
4444
expect(street.value()).toBe('');
4545

@@ -58,7 +58,7 @@ describe('debounce', () => {
5858
);
5959
const street = addressForm.street();
6060

61-
street.setControlValue('1600 Amphitheatre Pkwy');
61+
street.controlValue.set('1600 Amphitheatre Pkwy');
6262
expect(street.controlValue()).toBe('1600 Amphitheatre Pkwy');
6363
expect(street.value()).toBe('');
6464

@@ -79,7 +79,7 @@ describe('debounce', () => {
7979
);
8080
const street = addressForm.street();
8181

82-
street.setControlValue('1600 Amphitheatre Pkwy');
82+
street.controlValue.set('1600 Amphitheatre Pkwy');
8383
expect(street.controlValue()).toBe('1600 Amphitheatre Pkwy');
8484
expect(street.value()).toBe('1600 Amphitheatre Pkwy');
8585
});
@@ -95,7 +95,7 @@ describe('debounce', () => {
9595
);
9696
const street = addressForm.street();
9797

98-
street.setControlValue('1600 Amphitheatre Pkwy');
98+
street.controlValue.set('1600 Amphitheatre Pkwy');
9999
expect(street.controlValue()).toBe('1600 Amphitheatre Pkwy');
100100
expect(street.value()).toBe('');
101101

@@ -115,7 +115,7 @@ describe('debounce', () => {
115115
);
116116
const street = addressForm.street();
117117

118-
street.setControlValue('1600 Amphitheatre Pkwy');
118+
street.controlValue.set('1600 Amphitheatre Pkwy');
119119
expect(street.controlValue()).toBe('1600 Amphitheatre Pkwy');
120120
expect(street.value()).toBe('');
121121

@@ -142,10 +142,10 @@ describe('debounce', () => {
142142
);
143143
const street = addressForm.street();
144144

145-
street.setControlValue('1600 Amphitheatre Pkwy');
145+
street.controlValue.set('1600 Amphitheatre Pkwy');
146146
expect(street.value()).toBe('');
147147

148-
street.setControlValue('2000 N Shoreline Blvd');
148+
street.controlValue.set('2000 N Shoreline Blvd');
149149
expect(street.value()).toBe('');
150150

151151
first.resolve();
@@ -172,7 +172,7 @@ describe('debounce', () => {
172172
const street = addressForm.street();
173173

174174
// Set `controlValue` which will trigger a debounce update.
175-
street.setControlValue('1600 Amphitheatre Pkwy');
175+
street.controlValue.set('1600 Amphitheatre Pkwy');
176176
expect(street.controlValue()).toBe('1600 Amphitheatre Pkwy');
177177
expect(street.value()).toBe('');
178178

@@ -204,10 +204,10 @@ describe('debounce', () => {
204204
);
205205
const street = addressForm.street();
206206

207-
street.setControlValue('1600 Amphitheatre Pkwy');
207+
street.controlValue.set('1600 Amphitheatre Pkwy');
208208
expect(abortSpy).not.toHaveBeenCalled();
209209

210-
street.setControlValue('2000 N Shoreline Blvd');
210+
street.controlValue.set('2000 N Shoreline Blvd');
211211
expect(abortSpy).toHaveBeenCalledTimes(1);
212212

213213
resolve();
@@ -230,7 +230,7 @@ describe('debounce', () => {
230230
);
231231
const street = addressForm.street();
232232

233-
street.setControlValue('1600 Amphitheatre Pkwy');
233+
street.controlValue.set('1600 Amphitheatre Pkwy');
234234
expect(abortSpy).not.toHaveBeenCalled();
235235

236236
street.markAsTouched();
@@ -255,7 +255,7 @@ describe('debounce', () => {
255255
);
256256
const street = addressForm.street();
257257

258-
street.setControlValue('1600 Amphitheatre Pkwy');
258+
street.controlValue.set('1600 Amphitheatre Pkwy');
259259
expect(addListenerSpy).toHaveBeenCalledOnceWith('abort', jasmine.any(Function), {
260260
once: true,
261261
});
@@ -280,7 +280,7 @@ describe('debounce', () => {
280280
);
281281
const street = addressForm.street();
282282

283-
street.setControlValue('1600 Amphitheatre Pkwy');
283+
street.controlValue.set('1600 Amphitheatre Pkwy');
284284
expect(street.controlValue()).toBe('1600 Amphitheatre Pkwy');
285285
expect(street.value()).toBe('');
286286

@@ -300,7 +300,7 @@ describe('debounce', () => {
300300
);
301301
const street = addressForm.street();
302302

303-
street.setControlValue('1600 Amphitheatre Pkwy');
303+
street.controlValue.set('1600 Amphitheatre Pkwy');
304304
expect(street.controlValue()).toBe('1600 Amphitheatre Pkwy');
305305
expect(street.value()).toBe('1600 Amphitheatre Pkwy');
306306
});
@@ -316,7 +316,7 @@ describe('debounce', () => {
316316
options(),
317317
);
318318

319-
addressForm().setControlValue({street: '1600 Amphitheatre Pkwy'});
319+
addressForm().controlValue.set({street: '1600 Amphitheatre Pkwy'});
320320
expect(addressForm().controlValue()).toEqual({street: '1600 Amphitheatre Pkwy'});
321321
expect(addressForm().value()).toEqual({street: ''});
322322

@@ -334,10 +334,10 @@ describe('debounce', () => {
334334
options(),
335335
);
336336

337-
addressForm.street().setControlValue('1600 Amphitheatre Pkwy');
337+
addressForm.street().controlValue.set('1600 Amphitheatre Pkwy');
338338
expect(addressForm().value()).toEqual({street: '', city: ''});
339339

340-
addressForm.city().setControlValue('Mountain View');
340+
addressForm.city().controlValue.set('Mountain View');
341341
expect(addressForm().value()).toEqual({street: '', city: 'Mountain View'});
342342

343343
await timeout(0);
@@ -361,7 +361,7 @@ describe('debounce', () => {
361361
);
362362
const street = addressForm.street();
363363

364-
street.setControlValue('1600 Amphitheatre Pkwy');
364+
street.controlValue.set('1600 Amphitheatre Pkwy');
365365
expect(street.controlValue()).toBe('1600 Amphitheatre Pkwy');
366366
expect(street.value()).toBe('1600 Amphitheatre Pkwy');
367367
});
@@ -384,7 +384,7 @@ describe('debounce', () => {
384384
);
385385
const street = addressForm.street();
386386

387-
street.setControlValue('1600 Amphitheatre Pkwy');
387+
street.controlValue.set('1600 Amphitheatre Pkwy');
388388
expect(street.controlValue()).toBe('1600 Amphitheatre Pkwy');
389389
expect(street.value()).toBe('');
390390

@@ -417,12 +417,12 @@ describe('debounce', () => {
417417
);
418418
const street = addressForm.street();
419419

420-
street.setControlValue('1600 Amphitheatre Pkwy');
420+
street.controlValue.set('1600 Amphitheatre Pkwy');
421421
expect(street.controlValue()).toBe('1600 Amphitheatre Pkwy');
422422
expect(street.value()).toBe('1600 Amphitheatre Pkwy');
423423

424424
debounced.set(true);
425-
street.setControlValue('2000 N Shoreline Blvd');
425+
street.controlValue.set('2000 N Shoreline Blvd');
426426
expect(street.controlValue()).toBe('2000 N Shoreline Blvd');
427427
expect(street.value()).toBe('1600 Amphitheatre Pkwy');
428428

@@ -448,15 +448,15 @@ describe('debounce', () => {
448448
);
449449
const street = addressForm.street();
450450

451-
street.setControlValue('1600 Amphitheatre Pkwy');
451+
street.controlValue.set('1600 Amphitheatre Pkwy');
452452
expect(street.controlValue()).toBe('1600 Amphitheatre Pkwy');
453453
expect(street.value()).toBe('');
454454

455455
await timeout(0);
456456
expect(street.value()).toBe('1600 Amphitheatre Pkwy');
457457

458458
debounced.set(false);
459-
street.setControlValue('2000 N Shoreline Blvd');
459+
street.controlValue.set('2000 N Shoreline Blvd');
460460
expect(street.value()).toBe('2000 N Shoreline Blvd');
461461
});
462462
});

packages/forms/signals/test/node/field_node.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -616,7 +616,7 @@ describe('FieldNode', () => {
616616
// Same object identity because there was no change to flush to the name.
617617
expect(myForm().value()).toBe(product);
618618

619-
myForm.name().setControlValue('b');
619+
myForm.name().controlValue.set('b');
620620
// Same object identity because the name change is still pending.
621621
expect(myForm().value()).toBe(product);
622622

0 commit comments

Comments
 (0)