Sitelet https://github.com/angular/angular/commit/b0710b8db6fc86b8cc1f901ffe2d45787c2ab944
Skip to content

Commit b0710b8

Browse files
arturovtNothingEverHappens
arturovt
authored and
NothingEverHappens
committed
fix(forms): don't crash when control is missing in setUpValidators
setUpValidators was reading control._rawValidators without checking whether control even exists. If a directive's ngOnChanges fires before its control is wired up (this can happen in some race conditions), control is null or undefined, and it throws a raw TypeError that doesn't tell you anything useful. Added a check at the top: if control is null or undefined, throw a proper RuntimeError (code 1003) instead. The check itself is always on, not gated by ngDevMode, since the whole point is that this happens in production builds, where the existing ngDevMode-only guards get stripped out. The message text is still gated behind ngDevMode though, the same way MISSING_CONTROL_VALUE (-1002) does. Added a test that reproduces this with a real `@Component` using [formGroup], where the form input is never set, so it goes through Angular's actual change detection and lifecycle hooks instead of just calling the function directly. (cherry picked from commit fc3dc89)
1 parent 7bc6a1b commit b0710b8

3 files changed

Lines changed: 68 additions & 19 deletions

File tree

‎packages/forms/src/directives/reactive_directives/abstract_form.directive.ts‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
computed,
1919
signal,
2020
untracked,
21+
ɵRuntimeError as RuntimeError,
2122
} from '@angular/core';
2223

2324
import {FormGroup} from '../../model/form_group';
@@ -26,7 +27,8 @@ import {AbstractControl, FormSubmittedEvent} from '../../model/abstract_model';
2627
import {FormControl, isFormControl} from '../../model/form_control';
2728
import {ControlContainer} from '../control_container';
2829
import type {Form} from '../form_interface';
29-
import {missingFormException} from '../reactive_errors';
30+
import {RuntimeErrorCode} from '../../errors';
31+
import {formControlNameExample} from '../error_examples';
3032
import {
3133
CALL_SET_DISABLED_STATE,
3234
cleanUpControl,
@@ -129,7 +131,17 @@ export abstract class AbstractFormDirective
129131

130132
/** @nodoc */
131133
protected onChanges(changes: SimpleChanges): void {
132-
this._checkFormPresent();
134+
if (!this.form) {
135+
throw new RuntimeError(
136+
RuntimeErrorCode.FORM_GROUP_MISSING_INSTANCE,
137+
ngDevMode &&
138+
`formGroup expects a FormGroup instance. Please pass one in.
139+
140+
Example:
141+
142+
${formControlNameExample}`,
143+
);
144+
}
133145
if (Object.hasOwn(changes, 'form')) {
134146
this._updateValidators();
135147
this._updateDomValue();
@@ -377,10 +389,4 @@ export abstract class AbstractFormDirective
377389
cleanUpValidators(this._oldForm, this);
378390
}
379391
}
380-
381-
private _checkFormPresent() {
382-
if (!this.form && (typeof ngDevMode === 'undefined' || ngDevMode)) {
383-
throw missingFormException();
384-
}
385-
}
386392
}

‎packages/forms/src/directives/reactive_errors.ts‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -57,17 +57,6 @@ export function ngModelGroupException(): Error {
5757
);
5858
}
5959

60-
export function missingFormException(): Error {
61-
return new RuntimeError(
62-
RuntimeErrorCode.FORM_GROUP_MISSING_INSTANCE,
63-
`formGroup expects a FormGroup instance. Please pass one in.
64-
65-
Example:
66-
67-
${formControlNameExample}`,
68-
);
69-
}
70-
7160
export function groupParentException(): Error {
7261
return new RuntimeError(
7362
RuntimeErrorCode.FORM_GROUP_NAME_MISSING_PARENT,

‎packages/forms/test/reactive_integration_spec.ts‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
forwardRef,
1616
Input,
1717
OnDestroy,
18+
OnInit,
1819
Type,
1920
ViewChild,
2021
} from '@angular/core';
@@ -4227,6 +4228,59 @@ describe('reactive forms integration tests', () => {
42274228
);
42284229
});
42294230

4231+
it("should throw a coded error, instead of crashing, if a form isn't passed into formGroup in production mode", async () => {
4232+
const _global: {ngDevMode: any} = global as any;
4233+
const originalNgDevMode = _global.ngDevMode;
4234+
try {
4235+
_global.ngDevMode = false;
4236+
const fixture = initTest(FormGroupComp);
4237+
const error = await getRenderError(fixture);
4238+
// `FORM_GROUP_MISSING_INSTANCE`
4239+
expect((error as any).code).toBe(1052);
4240+
} finally {
4241+
_global.ngDevMode = originalNgDevMode;
4242+
}
4243+
});
4244+
4245+
it('reports a coded error (not an opaque TypeError) when an async-loaded form has not arrived yet in a production build', async () => {
4246+
@Component({
4247+
selector: 'async-form-app',
4248+
standalone: true,
4249+
imports: [ReactiveFormsModule],
4250+
template: `
4251+
<form [formGroup]="form">
4252+
<input formControlName="login" />
4253+
</form>
4254+
`,
4255+
})
4256+
class AsyncFormApp implements OnInit {
4257+
form!: FormGroup;
4258+
4259+
ngOnInit(): void {
4260+
Promise.resolve().then(() => {
4261+
this.form = new FormGroup({login: new FormControl('')});
4262+
});
4263+
}
4264+
}
4265+
4266+
const _global: {ngDevMode: any} = global as any;
4267+
const originalNgDevMode = _global.ngDevMode;
4268+
try {
4269+
_global.ngDevMode = false;
4270+
4271+
TestBed.configureTestingModule({imports: [AsyncFormApp]});
4272+
const fixture = TestBed.createComponent(AsyncFormApp);
4273+
4274+
const error = await getRenderError(fixture);
4275+
expect((error as any).code).toBe(1052);
4276+
expect(error.message).toContain('NG01052');
4277+
expect(error.message).not.toContain('validator');
4278+
expect(error.message).not.toContain('formGroup expects a FormGroup instance');
4279+
} finally {
4280+
_global.ngDevMode = originalNgDevMode;
4281+
}
4282+
});
4283+
42304284
it('should throw if formControlName is used without a control container', async () => {
42314285
TestBed.overrideComponent(FormGroupComp, {
42324286
set: {

0 commit comments

Comments
 (0)