Required validator on FormControl not working in unit tests when using jest

[Ben348]

I'm submitting a...

[ ] Regression (a behavior that used to work and stopped working in a new release)
[x] Bug report  
[ ] Performance issue
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request
[ ] Other... Please describe:

Current behavior

When running unit tests in a jest environment they fail when using the required validator on a FormControl because it always returns true even when the value is invalid. Using the FormControl from the @angular/forms package works fine.

This only happens in a jest environment, the actual functionality in a browser or when using jasmine for the unit tests works as expected. I'm sorry if I've raised this bug in the wrong place, it could be a problem with jest or jsdom. I'm going to keep trying to debug and see if I can find out the reason why.

The following issue might be relevant from what I found when searching the closed issues:
#86

Expected behavior

When the FormControl is invalid then it should return false when checking the valid property for unit tests running with jest.

Minimal reproduction of the problem with instructions

I've attached a zip file to this ticket with a sample angular project that shows the tests passing when using jasmine and failing when using jest.

typed-forms.zip

To run the jasmine unit tests that work use this command:
npm run test

To run the jest unit tests that fail use this command:
npm run test:jest

What is the motivation / use case for changing the behavior?

Unit tests pass when using jest and this project.

Environment

Angular version: 11.2.13

Browser:
- [x] TS Jest version 26.5.6 (jsdom 16.5.3)
 
For Tooling issues:
- Node version: v14.9.0
- Platform: Mac

[markwhitfeld]

Ok, I did a bit of investigation, and I think that it is because jest is using the umd bundle instead of the fesm2015 bundle.
I'm not sure what is responsible for the bad umd bundle, but the problem is clear once you see the code.
Basically the validator and asyncValidator function of FormControl and FormGroup are reading and writing the value to the prototype and not to the instance. The umd code for the validator prop is as follows:

        Object.defineProperty(FormControl.prototype, "validator", {
            get: function () {
                return _super.prototype.validator;
            },
            set: function (validator) {
                _super.prototype.validator = validator;
            },
            enumerable: false,
            configurable: true
        });

As you can see, the _super.protoype would be given as the this reference to the 'validator' property getter for angular's FormControl. So the validator is being retrieved from the prototype and not the instance!

For comparison, the other functions are defined like this:

        FormControl.prototype.reset = function (formState, options) {
            _super.prototype.reset.call(this, formState, options);
        };

Note the use of call in order to pass the this reference.

I will submit a PR to fix this.
The bug was unknowingly introduced in this commit: 6d858af#diff-a1146272390d550036583d550f6f9281128345c105f31e7306a4d555cf647dd8R55-R68
(or potentially by an upgrade to the build tool that caused the umd translation issue)

[markwhitfeld]

So this is a typescript ES5 compiler issue.
See this playground: https://www.typescriptlang.org/play?target=1&ts=4.2.3#code/LAKFGMBsEMGdYAQBECmAzBBvUCEAcAnASwDdoAXFBAfUtnIC4F7iA7AcwQF4EByXgNygc+AK4AjSEXAJ2KcgjrkAFAEomLIhywjcBeaIKtFACyKwAdLRT0hIXAF8ReCVJmx5imyrKRRKDXI2dlUde1xTcyslbgRffztHUCcwECg4RABBcRkUAA9KVgATRFQMbHCROQUlNUDgsIiEfXJDY1hRPBQCCyVEhBTcDxrvZXiA5iCtEMaIjq6emJ5x-pSU0EhPaBzY1hQAdwRs8DU7bfBe71jeAEYAJgBmQQgAe1ZYF82LSBf2ZV5zggtPRoKxwAFeAAaBDnVR2cBvD5fH5-AAGhBe5ExAE8uggXhhjrxEPNuul4AxUZCAPLiABWKHA5As1QACgRMTiutS0MpaQymSz5OzOeRcSgecpYao4UA

[markwhitfeld]

The bug was introduced in this angular commit: angular/angular@ad7046b
From this PR: angular/angular#37881

Because the Abstract control now uses property getters and setters for validator and asyncValidator, this lib was forced to create their own property getters and setters for these props, calling through to super. Unfortunately, typescript has a bug for UMD bundles where the behavior is not the same as ES6 classes. The super call to get and set a property ends up interacting with the prototype and not the instance.

[markwhitfeld]

As a temporary workaround, you can add this code to your test-setup.ts (or equivalent) file for jest:

import { FormArray, FormControl, FormGroup } from '@ngneat/reactive-forms';
// Temporary fix for bug in @ngneat/[email protected]
delete FormControl.prototype.validator;
delete FormControl.prototype.asyncValidator;
delete FormGroup.prototype.validator;
delete FormGroup.prototype.asyncValidator;
delete FormArray.prototype.validator;
delete FormArray.prototype.asyncValidator;

[Ben348]

Hello, @markwhitfeld thank you so much for the work you've done to investigate this and fix it. I got as far as figuring out the UMD bundle was the problem so then tried to get jest working with ECMAScript Modules instead. Thanks again, I really appreciate it 😄

[markwhitfeld]

Hi @Ben348, did you come right with the change to the dist that jest uses?
If you did, then could you share the changes to get this approach working?

[markwhitfeld]

PS. This ES5 downleveling issue was logged in Typescript 7 years ago (and 44000 issues ago) and unfortunately did not have a reasonable solution. It is just broken and that is the way it will always be. See: microsoft/TypeScript#338

My PR fixes the issue for this lib though by removing the passthrough properties in the subclass at runtime. This fixes the issue without the performance hit that the babel transpilation approach suffers from, and retains the typedef information that was intended with these property overrides.

[NetanelBasal]

@markwhitfeld thanks! I appreciated your work on this issue. I think we can get around this by using the code from this PR: https://github.com/ngneat/reactive-forms/pull/90/files

WDYT?

[markwhitfeld]

Yes, I think that is a good alternative approach, as I mentioned in my PR.
It does create a coupling to the knowledge that the superclass with the accessor is AbstractControl, but I think it is safe in the case of Angular. It also may have a very slight performance hit, but it would be negligible in the bigger picture.
I thought about using the Reflect.get and Reflect.set apis, but avoided them because they are not compatible with ES5.
Getting the property descriptors once using the Object apis is a good compromise.

[NetanelBasal]

Yes, I agree about the coupling, but we are all coupled to Angular anyway :)

[NetanelBasal]

Thanks for the effort.

[markwhitfeld]

PS. there was almost a carbon copy of this issue here: angular/angular#40327

[markwhitfeld]

Thanks for getting this merged and released so quickly 🎉 🚀

[Ben348]

@markwhitfeld Sorry for the late reply. I was trying to get it working but not had much luck so far. If I get it working I'll be sure to let you know.

You need to be using the next version of jest (27) and the next version of jest-preset-angular https://thymikee.github.io/jest-preset-angular/docs/next/getting-started/presets#the-presets

Thank you both for the investigation and fixes on this issue here

[markwhitfeld]

@Ben348 It is fixed in the latest release, so no workaround needed! 👍

[ALEX-ANV]

Hello, I have tried to use version 1.7.3 in angular 9.1 and it does not work.

core.js:6241 ERROR TypeError: Cannot read property 'set' of undefined at FormControl.set validator [as validator] (ngneat-reactive-forms.js:313) at new AbstractControl (forms.js:3878) at new FormControl (forms.js:4913) at new FormControl (ngneat-reactive-forms.js:291)

https://stackblitz.com/edit/angular-ivy-49cmlm