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

Clarification re: Type is part of the declaration of 2 modules #435

Closed
tanordheim opened this issue Apr 26, 2021 · 10 comments · Fixed by #469
Closed

Clarification re: Type is part of the declaration of 2 modules #435

tanordheim opened this issue Apr 26, 2021 · 10 comments · Fixed by #469

Comments

@tanordheim
Copy link

Hi, and thanks for a great library! I'm currently evaluating this for one of our Angular applications and I've come across one of the issues documented here where I have the following situation:

  • Module A and module B both import CommonModule from @angular/common.
  • Module B depends on module A.
  • I try to test a component in module B where I need to mock out something from CommonModule explicitly.

I have set up a StackBlitz showcasing what I try to do here: https://stackblitz.com/edit/angular-ivy-7cnyfm?file=src/app/feature/sample.component.spec.ts

I basically have:

  • SharedModule which imports CommonModule.
  • FeatureModule which imports SharedModule and CommonModule
  • A SampleComponent in SharedModule which takes a date input and passes it to the Angular DatePipe.

I set this up according to my understanding of the docs I linked earlier like this:

MockBuilder(SampleComponent)
  .keep(FeatureModule)
  .mock(SharedModule)
  .mock(DatePipe, value => 'MOCK:' + value);

This fails with the following error:

Error: Type NgClass is part of the declarations of 2 modules: CommonModule and MockOfCommonModule! Please consider moving NgClass to a higher module that imports CommonModule and MockOfCommonModule. You can also create a new NgModule that exports and includes NgClass then import that NgModule in CommonModule and MockOfCommonModule.

...and then repeated for more or less everything else in CommonModule.

Am I misunderstanding the docs here? I'd be happy to help clarify if I understood what the problem was, but as far as I'm able to tell this should be how things are intended to work.

This error also occurs if I stop including SharedModule in my FeatureModule and set the tests up like this, leading me to believe this isn't the same case as in the documentation:

MockBuilder(SampleComponent, FeatureModule)
  .mock(DatePipe, value => 'MOCK:' + value);

Setting it up like in the documentation with .keep(FeatureModule) and .mock(CommonModule) also yields the same results.

@satanTime
Copy link
Member

Hi,

thanks for the report.

yeah, a very good case :) CommonModule is never mocked and skipped everywhere, but finally there is a good example why we need to process its declarations.

Thanks! It will be fixed with the next release.

@tanordheim
Copy link
Author

Ah, right. That explains it. It probably also explains why I see the default implemented behavior of DatePipe if I just do MockBuilder(SampleComponent, FeatureModule) when my understanding was that all dependencies would be mocked by default even if I don't specify the mock myself.

Is there anything I can do to work around this for the time being? I don't see any clear points where this would happen in MockModule or MockBuilder after glancing through the code quickly but.

@satanTime
Copy link
Member

satanTime commented Apr 26, 2021

MockBuilder(SampleComponent, FeatureModule) is the right direction and it does mock everything, besides SampleComponent and CommonModule.

The idea around skipping CommonModule everywhere is that people usually don't expect ngIf, ngFor, async to be mocked. I think to change this approach so in the end CommonModule would be processed as .keep by default and anything inside of it could be adjusted for testing needs.

About a work around.... A good question :) I think you could try .exclude(CommonModule).mock(DatePipe, ...).keep(AnythingYouNeedFromCommonModule), but I'm not sure if it works on the current version.

@tanordheim
Copy link
Author

It works, sort of, except it's going to be a bit of a pain to set up all the keeps. I'll refactor the code slightly to use our own pipes for formatting these dates so its easier to mock - we should do that for other purposes as well so it's no big issue.

But then I've found the source to my issues here, so I'm gonna go ahead and close this issue. I'll revisit once the next release is out to see if we can do things differently at that point but for now this gets me further - thanks a lot for the help!

@satanTime satanTime reopened this Apr 26, 2021
@satanTime
Copy link
Member

satanTime commented Apr 26, 2021

Nice.

Btw, please keep the issue open, it works like a reminder for me to solve it when I have time. It will be closed during a release process of the next version.

@satanTime
Copy link
Member

So, actually ng-mocks does process CommonModule, but the problem is that BrowserDynamicTestingModule exports BrowserTestingModule which exports BrowserModule which exports CommonModule.

Looking for a solution how to inject MockOfCommonModule into BrowserDynamicTestingModule.

@satanTime
Copy link
Member

Hi @tanordheim,

could you verify whether this version works for you needs?
ng-mocks.zip

Thanks in advance!

@tanordheim
Copy link
Author

tanordheim commented May 3, 2021

For my usecase, this worked. The test case below yielded the expected "Type NgClass is part of the declarations of 2 modules" error on the latest release, and passed successfully with the version you attached.

@Component({
  template: '<p>{{ testDate | date:"short" }}</p>',
})
class TestComponent {
  testDate = new Date();
}

describe('DatePipe mock', () => {
  beforeEach(() => {
    return MockBuilder(TestComponent, SharedModule)
      .mock(DatePipe, (v: any) => 'mocked:' + v);
  });

  it('should use mocked date pipe', () => {
    const fixture = MockRender(TestComponent);
    expect(fixture.point.nativeElement.textContent).toContain('mocked');
  });
});

Thanks so much for the update!

@satanTime
Copy link
Member

Thanks for the feedback! On the upcoming weekend, I'll release a new version with the fix.

satanTime added a commit that referenced this issue May 3, 2021
fix(mock-builder): overrides mock modules for platform #435
@satanTime
Copy link
Member

v11.11.0 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.

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.

2 participants