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

Bug: ngMocks.findInstance unable to find instance (Angular 17) #7216

Closed
GipHub123 opened this issue Nov 9, 2023 · 19 comments · Fixed by #8757
Closed

Bug: ngMocks.findInstance unable to find instance (Angular 17) #7216

GipHub123 opened this issue Nov 9, 2023 · 19 comments · Fixed by #8757
Assignees
Labels
bug Something isn't working released v14.12.2

Comments

@GipHub123
Copy link

Description of the bug

It look's like something has changed in recently released Angular 17.
In some test cases ngMocks.findInstance is unable to find instance (Standalone).
ngMocks.findInstance(TestComponent);

TypeError: Cannot read properties of null (reading 'rootNodes')
at Ft (node_modules/ng-mocks/index.mjs:1:69900)
at node_modules/ng-mocks/index.mjs:1:70211
at Gt (node_modules/ng-mocks/index.mjs:1:70279)
at Wt (node_modules/ng-mocks/index.mjs:1:70594)
at Qt (node_modules/ng-mocks/index.mjs:1:71245)
at Qt (node_modules/ng-mocks/index.mjs:1:71254)
at Qt (node_modules/ng-mocks/index.mjs:1:71254)
at Qt (node_modules/ng-mocks/index.mjs:1:71254)
at Qt (node_modules/ng-mocks/index.mjs:1:71254)
at Qt (node_modules/ng-mocks/index.mjs:1:71254)

All failing test cases has been setup by using MockBuilder.
All test cases (~4000) were fine before updating to Angular 17.

I've mainly used an alternative method to find instances.
function get(fixture: ComponentFixture<any>, type: Type<any>): any { return fixture?.debugElement?.query(By.directive(type)).componentInstance }

I replaced ngMocks.findInstance with above get-function and it fixed the problem.

An example of the bug

Link:

Expected vs actual behavior

ngMocks.findInstance find an instance without an error.



Angular CLI: 17.0.0
Node: 20.9.0
Package Manager: npm 10.1.0
OS: win32 x64

Angular: 17.0.1
... animations, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... platform-server, router

Package                                    Version
--------------------------------------------------------------------
@angular-devkit/architect                  0.1700.0 (cli-only)
@angular-devkit/build-angular              17.0.0
@angular-devkit/core                       17.0.0 (cli-only)
@angular-devkit/schematics                 17.0.0
@angular/cdk                               17.0.0
@angular/cli                               17.0.0
@angular/google-maps                       17.0.0
@angular/material                          17.0.0
@angular/material-date-fns-adapter         17.0.0
@angular/ssr                               17.0.0
@nguniversal/module-map-ngfactory-loader   v8.2.6
@schematics/angular                        17.0.0
rxjs                                       7.8.1
typescript                                 5.2.2
zone.js                                    0.14.2
@GipHub123 GipHub123 added the bug Something isn't working label Nov 9, 2023
@satanTime
Copy link
Member

Hi @GipHub123,

thanks for reporting. I'll do my best to fix it asap. However, currently, my availability is very limited.

@GipHub123
Copy link
Author

GipHub123 commented Nov 9, 2023

No worries 😊 As I mentioned problem is quite easy to fix with a work around 👍

@satanTime
Copy link
Member

Hi @GipHub123,

unfortunately, I don't see the same errors on A17 with ng-mocks. Could you post a failing test here which I could debug?

@rlexa
Copy link

rlexa commented Nov 17, 2023

I have this issue with newest versions (ng 17.0.3, ng-mocks 14.11.0) in multiple component tests in multiple projects. It happens only when searched instances are dynamic in the template i.e. using @if or @for or similar. It does not happen for all of those cases though.
Example template:

@if (routes$ | async; as routes) {
  <rwe-navigation [routes]="routes" (actionPayload)="$event.next()" />
}

Then spec file:

...
  beforeEach(() => MockBuilder(RweActionSlotsComponent).provide({provide: RweDiActionSlots, useValue: of([slot])}));

  beforeEach(() => {
    fixture = MockRender(RweActionSlotsComponent);
    fixture.detectChanges();
  });

...

  it(`binds actions`, () =>
    expect(ngMocks.findInstance(RweNavigationComponent).routes).toEqual([
      {cssClass: 'cssClass', disabled: true, icon: 'icon', name: 'label', tooltip: 'tooltip', payload: slot.trigger$, testId: 'test-id'},
    ]));

Leads to:

TypeError: Cannot read properties of null (reading 'rootNodes')

      32 |
      33 |   it(`binds actions`, () =>
    > 34 |     expect(ngMocks.findInstance(RweNavigationComponent).routes).toEqual([
         |                    ^
      35 |       {cssClass: 'cssClass', disabled: true, icon: 'icon', name: 'label', tooltip: 'tooltip', payload: slot.trigger$, testId: 'test-id'},
      36 |     ]));
      37 |

      at f (node_modules/ng-mocks/webpack:/ng-mocks/libs/ng-mocks/src/lib/mock-helper/crawl/el-def-get-parent.ts:53:25)
      at index (node_modules/ng-mocks/webpack:/ng-mocks/libs/ng-mocks/src/lib/mock-helper/crawl/el-def-get-parent.ts:41:77)
      at t.default (node_modules/ng-mocks/webpack:/ng-mocks/libs/ng-mocks/src/lib/mock-helper/crawl/el-def-get-parent.ts:57:16)
      at t.default (node_modules/ng-mocks/index.js:1:131407)
      at u (node_modules/ng-mocks/index.js:1:133465)
      at u (node_modules/ng-mocks/index.js:1:133518)
      at t.default (node_modules/ng-mocks/index.js:1:128786)
      at Object.nativeNode [as findInstance] (node_modules/ng-mocks/webpack:/ng-mocks/libs/ng-mocks/src/lib/mock-helper/find-instance/mock-helper.find-instance.ts:30:53)

Currently I'm using the workaround to circumvent this.

@satanTime
Copy link
Member

Ah, new syntax. That makes sense. I haven't tested it yet.

@rlexa
Copy link

rlexa commented Nov 18, 2023

Ah, new syntax. That makes sense. I haven't tested it yet.

Sadly the answer is no, same happens when using *ngIf at those places.

@satanTime
Copy link
Member

satanTime commented Nov 18, 2023

Hi @rlexa,

I still cannot reproduce the issue, could you post a min failing example?

This is what I use, but it doesn't fail.

import { Component, Input, NgModule } from '@angular/core';
import { AsyncSubject, Observable } from 'rxjs';
import { CommonModule } from '@angular/common';
import { MockBuilder, MockRender, ngMocks } from 'ng-mocks';

@Component({
  selector: 'target',
  template: `
    <child
      *ngIf="routes$ | async as routes"
      [routes]="routes"
    ></child>
  `,
})
class TargetComponent {
  @Input() routes$: Observable<Array<string>> | null = null;

  targetComponent7216() {}
}

@Component({
  selector: 'child',
  template: ``,
})
class ChildComponent {
  @Input() routes: Array<string> | null = null;
}

@NgModule({
  imports: [CommonModule],
  declarations: [TargetComponent, ChildComponent],
})
class TargetModule {}

// @see https://github.com/help-me-mom/ng-mocks/issues/7216
describe('issue-7216', () => {
  beforeEach(() => MockBuilder(TargetComponent, TargetModule));

  it('finds ChildComponent', () => {
    const routes$ = new AsyncSubject<Array<string>>();
    routes$.next(['route1']);
    routes$.complete();

    MockRender(TargetComponent, { routes$ });

    const child = ngMocks.findInstance(ChildComponent);
    expect(child.routes).toEqual(['route1']);
  });
});

@rlexa
Copy link

rlexa commented Nov 20, 2023

I'm a bit swamped this week maybe will be able to crete a repro on the weekend, but here is more about the component in question (you can see that it's OnPush and standalone). The RweDestroy decorator can be ignored, I tried without, still fails.

@Component({
  selector: 'rwe-action-slots',
  template: `@if (routes$ | async; as routes) {
    <rwe-navigation [routes]="routes" (actionPayload)="$event.next()" />
  }`,
  changeDetection: ChangeDetectionStrategy.OnPush,
  standalone: true,
  imports: [CommonModule, RweNavigationComponent],
})
export class RweActionSlotsComponent {
  private readonly actions$ = inject(RweDiActionSlots);

  @RweDestroy() private readonly slots$ = new StateSubject<RweActionSlot[] | null>(null);

  /** Overwrites `RweDiActionSlots` token with input slots. */
  @Input() set slots(val: RweActionSlot[] | null | undefined) {
    this.slots$.next(val ?? null);
  }

  readonly routes$ = combineLatest([this.slots$, this.actions$]).pipe( /* complex flow but should not be relevant */);
}

and this is the relevant part of spec file (so .provide is used and also the runtime is jest):

import {ComponentFixture} from '@angular/core/testing';
import {MockBuilder, MockRender, ngMocks} from 'ng-mocks';
import {of} from 'rxjs';
import {RweNavigationComponent} from '../rwe-navigation/rwe-navigation.component';
import {RweActionSlot, RweDiActionSlots} from './rwe-action-slot';
import {RweActionSlotsComponent} from './rwe-action-slots.component';

describe('RweActionSlotsComponent', () => {
  let fixture: ComponentFixture<RweActionSlotsComponent>;

  const slot: RweActionSlot = {
    cssClass: of('cssClass'),
    disabled$: of(true),
    icon: 'icon',
    label: of('label'),
    trigger$: {next: jest.fn()} as any,
    tooltip: 'tooltip',
    testId: 'test-id',
  };

  beforeEach(() => MockBuilder(RweActionSlotsComponent).provide({provide: RweDiActionSlots, useValue: of([slot])}));

  beforeEach(() => {
    fixture = MockRender(RweActionSlotsComponent);
    fixture.detectChanges();
  });

  it(`creates instance`, () => expect(fixture.componentInstance).toBeTruthy());

  it(`renders`, () => expect(fixture).toMatchSnapshot());

  it(`binds actions`, () =>
    expect(ngMocks.findInstance(RweNavigationComponent).routes).toEqual([
      {cssClass: 'cssClass', disabled: true, icon: 'icon', name: 'label', tooltip: 'tooltip', payload: slot.trigger$, testId: 'test-id'},
    ]));
});

@rlexa
Copy link

rlexa commented Nov 26, 2023

OK while working on updating my private client I stumbled upon this in the dashboard.component.spec.ts file (https://github.com/rlexa/dd-playground-client/blob/ng17/src/app/module/widget/dashboard/dashboard.component.spec.ts) see in branch (https://github.com/rlexa/dd-playground-client/tree/ng17). You'll may have to npm i --force, when working in VS Code it should be possible to directly debug in this file with F5.

@LukasMachetanz
Copy link

What is the current status of this issue? I am experiencing the same problem using the new control-flow syntax. This is a major blocker as it prevents us to fully switch to the new approach.

@LukasMachetanz
Copy link

First of all I have to apologise for being impatient, but this bug is a definite blocker using ng-mocks together with Angular's new control-flow syntax.

@rlexa
Copy link

rlexa commented Feb 26, 2024

Using the mentioned workaround in the failing cases to unblock updates should be good enough until this is resolved. I added these functions in our testing library and use them whenever ngMocks fails:

/**
 * @example
 * findDirective(fixture, MyComponent).valueChange.emit(123);
 */
export const findDirective = <T, D>(fixture: ComponentFixture<T>, directive: Type<D>) =>
  fixture.debugElement.query(By.directive(directive)).componentInstance as D;

/**
 * @example
 * findDirectives(fixture, MyComponent)[0].valueChange.emit(123);
 */
export const findDirectives = <T, D>(fixture: ComponentFixture<T>, directive: Type<D>) =>
  fixture.debugElement.queryAll(By.directive(directive)).map((ii) => ii.componentInstance as D);

@LukasMachetanz
Copy link

In my opinion this is still a bug and should be updated as soon as possible. I would like to use the library as intended instead of introducing alternative code. Is there a rough estimation when this issue is tackled?

@LukasMachetanz
Copy link

I am wondering if this project is still actively maintained as this bug is already open for quite some time. Is there any plan to fix the currently available bugs soon?

@satanTime
Copy link
Member

satanTime commented Apr 3, 2024

Hi all, all issues are in my todo, but not much time to work on the lib recently. Hopefully, in a couple of weeks, I'll be back on track and can dedicate more time on open source.

Feel free to contribute and fix some issues, open source is fun and it's always right time to join.

@andreandersson
Copy link

So, I actually have a workaround for this. The actual error might be within Angular itself, but I'll have to dig a bit deeper to actually verify that. Here is a small test case which fails:

import { CommonModule } from '@angular/common';
import { Component, NgModule } from '@angular/core';

import { MockBuilder, MockRender, ngMocks } from 'ng-mocks';

@Component({
  selector: 'target',
  template: `
    @if (hasChild) {
      <child />
    }
  `,
})
class TargetComponent {
  public readonly hasChild = true;
}

@Component({
  selector: 'child',
  template: '',
})
class ChildComponent {}

@NgModule({
  imports: [CommonModule],
  declarations: [TargetComponent, ChildComponent],
})
class TargetModule {}

// @see https://github.com/help-me-mom/ng-mocks/issues/7216
describe('issue-7216', () => {
  beforeEach(() => MockBuilder(TargetComponent, TargetModule));

  it('finds child-element', () => {
    MockRender(TargetComponent);

    expect(ngMocks.findInstance(ChildComponent)).toBeTruthy();
  });
});

The error can be solved with a quick null-check in el-def-get-parent.ts:getScanViewRefRootNodes:

   const vr = vcr.get(vrIndex);
   if (!vr) {
     continue;
   }

The problem here is that ViewContainerRef returns a length of 1, but trying to fetch the first element returns 0. I'll create a PR for this will digging deeper.

andreandersson pushed a commit to andreandersson/ng-mocks that referenced this issue Apr 12, 2024
ViewContainerRef actually returns the wrong length in case of
Angular 17 Control Flow. By checking that the returned value is
not null we should be safe.

Solves help-me-mom#7216
andreandersson pushed a commit to andreandersson/ng-mocks that referenced this issue Apr 12, 2024
ViewContainerRef actually returns the wrong length in case of
Angular 17 Control Flow. By checking that the returned value is
not null we should be safe.

Solves help-me-mom#7216
andreandersson pushed a commit to andreandersson/ng-mocks that referenced this issue Apr 12, 2024
ViewContainerRef actually returns the wrong length in case of
Angular 17 Control Flow. By checking that the returned value is
not null we should be safe.

Solves help-me-mom#7216
andreandersson pushed a commit to andreandersson/ng-mocks that referenced this issue Apr 12, 2024
ViewContainerRef actually returns the wrong length in case of
Angular 17 Control Flow. By checking that the returned value is
not null we should be safe.

Solves help-me-mom#7216
@satanTime
Copy link
Member

v14.12.2 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.

@LukasMachetanz
Copy link

@satanTime, thank you very much. I can confirm that the issue is fixed for us as well. We highly appreciate the effort.

@satanTime
Copy link
Member

All creds go to @andreandersson.

Thank you for the contribution!

mchaitanya pushed a commit to mchaitanya/experimental-angular that referenced this issue Dec 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working released v14.12.2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants