fix: unable to bind ngIf in angular 17+

[sabrina-10kc]

When component templates contain child templates, they are replaced with a mock template that contains a *ngIf this causes a failure when the component doesn't have NgIf imported.

Though this problem likely exists in some pre-angular 17 situations, it has been causing new failures to appear as people upgrade since 3rd party angular components are starting to use the new control flow syntax and no longer import CommonModule / NgIf.

Instead the mock template now uses @if syntax if angular is 17 or higher.

Fixes #8884

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (9ef4a65) to head (0ba55d0).
Report is 15 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff            @@
##            master     #9314   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          227       227           
  Lines         4935      4946   +11     
  Branches      1147      1148    +1     
=========================================
+ Hits          4935      4946   +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[sabrina-10kc]

@satanTime Is there anything I need to change in this PR to help it get in & released ? (other than rebasing of course)

This issue is currently preventing use of errorOnUnknownElements for me

[satanTime]

Hi @sabrina-10kc,

Thank you for your contribution.
Next week I'll be working on the rep and take a look at the PR.

[donroyco]

Any update on this PR, @satanTime ? It would be really helpful to have this fixed.

[mikeysteele]

It would be great to get this merged. This can cause otherwise good tests to fail depending on when the error is thrown.

[clara-belair]

Hi @sabrina-10kc,

Thank you for this PR. I've tested it locally and it fix the problem in my projet.

@satanTime Can you tell us if you have the time to merge this PR and release it, please ?

[JsantosDK]

Hello @satanTime when can you approve this?
It's almost 2 months now since your last comment on it.

[satanTime]

I wish it was so easy.
The thing there is that a line of code which was covered by tests now is simply ignored.
Because of that I need to investigate whether this is the only way to fix the issue, because the line is covered well currently in a17 and a18.

[JsantosDK]

@satanTime after reading this comment, please re-consider moving the version to the approach of angular and the current approach that the library has.

It is a problem for people who want to use it to have to wait for months for something to be fix, because you need to be sure it works on all versions.
It is a problem for people who want to contribute, since they need to ensure that it works on all versions, make the fix so easy that you can understand and then wait for god knows how long until before it is merged.

The angular team just gave a speech a few days ago saying that they realize by time.
Everybody that uses things in the angular ecosystem is used to this way of release management.

Me and others want to contribute, but the current approach means everything will always fall on you.
You are blocking other from helping because you need to be 100% sure of everything, at the same time you don't have time.

Yesterday, I installed the lib on a new machine to contribute with the specs and it took over 1h. The approach to develop the lib already takes 6GB in docker to be run, this is not a normal amount of space for a library that like you say, is just a wrapper for making life easier. Will this keep going until the machine reaches it's limit?

People want to help, but it seems you don't want to be help, you don't really want others to collaborate with you.

What would it take to help?
Not just on this issue, but on the lib itself?
Or do you really want to keep going like, until eventually the lib faces so many issues that it dies out due to your lack of time?

I'm working on the schematics, but if there is anything I can help, tell.
And please, consider the change in versioning since the consumers of the lib don't agree with you and it doesn't match the way the angular ecosystem moves.

[satanTime]

Not sure I understand your point. Old syntax is support in all angular versions it wasn't removed from a17 or a18. So I'm not sure how release change can improve that. Still both syntax should be supported for a18 and a17.

[JsantosDK]

My main issue is that this PR has been opened for a long time without any feedback from anybody.

Really seems like you don't have the time to support the lib alone, but don't allow much help.

[satanTime]

Feel free to help, a test which would cover both beaches instead of ignoring one would help.

[JsantosDK]

Ok, will try to get it done between today and tomorrow.

[satanTime]

Great. Thanks. Is that branch cannot be covered at all - that’s also fine. I just need to be sure that that’s exactly like that.

[JsantosDK]

@satanTime I reviewed this with the focus on reaching the other part of the branch.
Only way to do so, was modify the specs so that it has 2 branches, one for angular 16 and below, and another for angular 17 and above.

When I did this, I got the following results:

Angular 17+ branch:

• Original specs pass;
• Added spec passes (expected, due to the if statement exiting before the test is reached);

Angular 16 and below:

• Original specs pass(expected, due to the if statement exiting before the test is reached);
• Added spec fails with the same error as originally: NG0303: Can't bind to 'ngIf' since it isn't a known property of 'ng-template' (used in the 'TargetComponent' component template);

From what I can tell, I'm worried that if this PR is merged, and a new version is released, in an Angular 16 or below app, it will break it. I may try to make a build and tested this myself with an actual use case, but would like your input on this before proceeding.

Note: other that using this testing logic, I don't think there is a way to reach the other branch statement.

In example you can find the spec file I used:

import {
  Component,
  ContentChild,
  NgModule,
  TemplateRef,
  VERSION,
} from '@angular/core';

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

// @see https://github.com/help-me-mom/ng-mocks/issues/8884
describe('issue-8884', () => {
  ngMocks.throwOnConsole();

  describe('with a16 and below (added by me)', () => {
    if (Number.parseInt(VERSION.major, 10) > 16) {
      it('needs a16 or below', () => {
        expect(true).toBeTruthy();
      });

      return;
    }

    @Component({
      selector: 'app-target',
      template: `<ng-template [ngTemplateOutlet]="content" />`,
    })
    class TargetComponent {
      @ContentChild('content', {} as never)
      content?: TemplateRef<any>;
    }

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

    beforeEach(() => MockBuilder(null, TargetModule));

    it('should create', () => {
      MockRender(`<app-target>Test content</app-target>`);

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

  describe('with a17+', () => {
    if (Number.parseInt(VERSION.major, 10) < 17) {
      it('needs a17+', () => {
        expect(true).toBeTruthy();
      });

      return;
    }

    describe('when standalone component does not import NgIf', () => {
      @Component({
        selector: 'app-standalone',
        standalone: true,
        template: `<ng-template [ngTemplateOutlet]="content" />`,
      })
      class StandaloneComponent {
        @ContentChild('content', {} as never)
        content?: TemplateRef<any>;
      }

      beforeEach(() => MockBuilder(null, StandaloneComponent));

      it('should create', () => {
        MockRender(`<app-standalone>Test content</app-standalone>`);

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

    describe('when NgIf is not available to a component in a module', () => {
      @Component({
        selector: 'app-target',
        template: `<ng-template [ngTemplateOutlet]="content" />`,
      })
      class TargetComponent {
        @ContentChild('content', {} as never)
        content?: TemplateRef<any>;
      }

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

      beforeEach(() => MockBuilder(null, TargetModule));

      it('should create', () => {
        MockRender(`<app-target>Test content</app-target>`);

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

[satanTime]

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