Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

.keep(ReactiveFormsModule) causes FormBuilder to malfunction #377

Closed
garrappachc opened this issue Apr 9, 2021 · 13 comments · Fixed by #384
Closed

.keep(ReactiveFormsModule) causes FormBuilder to malfunction #377

garrappachc opened this issue Apr 9, 2021 · 13 comments · Fixed by #384

Comments

@garrappachc
Copy link

garrappachc commented Apr 9, 2021

Let's say I have a decently simple component that uses the FormBuilder:

@Component({
  selector: 'app-form',
  templateUrl: './form.component.html',
  styleUrls: ['./form.component.css']
})
export class FormComponent {

  form = this.fb.group({
    name: [],
  });

  constructor(
    private fb: FormBuilder,
  ) { }

}
<form [formGroup]="form">
  <input type="text" formGroupName="name">
</form>

When I test it with ngMocks, like that:

describe('FormComponent with ngMocks', () => {
  let component: FormComponent;
  let fixture: MockedComponentFixture<FormComponent>;

  beforeEach(() => MockBuilder(FormComponent).keep(ReactiveFormsModule));

  beforeEach(() => {
    fixture = MockRender(FormComponent);
    component = fixture.point.componentInstance;
    fixture.detectChanges();
  });

  it('should create', () => {
    expect(component).toBeTruthy();
  });
});

the injected FormBuilder instance spits out undefined value, causing angular to fail the unit test:

Error: formGroup expects a FormGroup instance. Please pass one in.

           Example:

           
        <div [formGroup]="myGroup">
          <input formControlName="firstName">
        </div>

        In your class:

        this.myGroup = new FormGroup({
           firstName: new FormControl()
        });

This error is thrown because the value of form is undefined.

I have uploaded a sample repo that reproduces this error: https://github.com/garrappachc/ng-mocks-reactive-forms-bug
In form.component.spec.ts you can see two unit tests - one that works (without using ngMocks) and one that doesn't.

Also, I am not sure when was this error introduced, all I know is that Angular version 11.2.5 was working fine and 11.2.6 started to cause the issue - but Angular itself is fine.

Here's my Angular version bump pr that started failing: tf2pickup-org/client#803

@satanTime
Copy link
Member

Hi, thanks for the report.

Yep. Sorry. My bad. I implemented some wrong changes around 11.2.6.

Could update to the latest version of ng-mocks? It shouldn't have the issue.

@satanTime
Copy link
Member

Ah okay. You mean Angular itself.

Then I'll dig into the issue next days and try to provide a fix ASAP.

@satanTime
Copy link
Member

For now, I would say, that Angular fixed the issue they had.

In a mock component, the form is undefined. However, ReactiveFormsModule hasn't been mocked. Therefore [formGroup]="form" is the same as [formGroup]="undefined" and I would expect it to fail.

A mix of mock components and real FormsModule or ReactiveFormsModule always has some tricky issues.

Might you describe, why you need to keep ReactiveFormsModule?
I would suggest to either keep both of the things or to mock both of them.

@garrappachc
Copy link
Author

I do not want to mock either of them, FormsModule is not needed at all - take a look at the repo I created, in app.module.ts the only imports are BrowserModule and ReactiveFormsModule.

You are right saying that the Angular test fails because form is undefined, the problem is it should be defined.

 form = this.fb.group({
    name: [],
  });

Here, form should not be undefined.

@satanTime
Copy link
Member

Yep ReactiveFormsModule imports FormsModule itself, that's only what I meant.

Good, then I'll dig into it later today / tomorrow.

@satanTime
Copy link
Member

Hi, found the issue. ng-mocks didn't detect that FormBuilder is provided via ReactiveFormsModule and mocks it, which breaks the behavior.

The fix will take one week more.

A workaround is to keep FormBuilder too: MockBuilder(FormComponent).keep(ReactiveFormsModule).keep(FormBuilder).

Sorry for the delay.

@satanTime
Copy link
Member

satanTime commented Apr 11, 2021

This is the change: angular/angular@b93fb79, angular/angular#41126

FormBuilder isn't more provided via ReactiveFormsModule and has a new feature like @Injectable({providedIn: ReactiveFormsModule}).

I'll cover it on the next weekend.

@satanTime
Copy link
Member

satanTime commented Apr 12, 2021

Hi @garrappachc,

could you confirm that the new version works well for you?

The package: ng-mocks.zip

@garrappachc
Copy link
Author

It works like a charm :)
Thank you for all your work!

satanTime added a commit that referenced this issue Apr 12, 2021
fix(#377): respect of providedIn in Injectable
@satanTime
Copy link
Member

v11.10.1 has been released and contains a fix for the issue. Feel free to reopen the issue or to submit a new one if you meet any problems.

@Simon-Hayden-iteratec
Copy link

Hi, I'm in the process of updating our repo to Angular 15 with ng-mocks 14.6.0 and I got the same error as described above.

Previously we just added .keep(ReactiveFormsModule) to use the FormBuilder/NonNullableFormBuilder, but after the upgrade I have to change everything to .keep(NonNullableFormBuilder). Not the biggest deal, but it looks like this is an regression then?

@satanTime
Copy link
Member

Hi @Simon-Hayden-iteratec,

right, this came from Angular updates. They started to make all module providers root providers to have better visibility in tree-shaking, because of that FormBuilder doesn't have a reference to ReactiveFormsModule and FormsModule anymore and there is no way to detect whether it should be kept or mocked.

I've covered that in FAQ: https://ng-mocks.sudo.eu/troubleshooting/faq#error-ng01052-formgroup-expects-a-formgroup-instance-please-pass-one-in

My recommendation is to add .keep(NG_MOCKS_ROOT_PROVIDERS), also, it will be default in ng-mocks@15: #4701

@Simon-Hayden-iteratec
Copy link

Ah, thanks a lot! Sorry for not finding the FAQ :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants