extended diagnostic for non-nullable optional is wrong

[tiberiuzuld]

Which @angular/* package(s) are the source of the bug?

core

Is this a regression?

No

Description

Having the case below produces the warning for optional chain operation. Because of ! it will always return a boolean to the input. Relates to the new feature https://github.com/angular/angular/pull/46686 https://github.com/angular/angular/issues/44870

Also can we have a angularCompilerOptions to disable this warnings?

First case:

optionalArrayElement: {a: 'value'}[] = [];

<button [disabled]="!optionalArrayElement[0]?.a"></button>

Second case:

enum MapKeys {
  A,
  B
}

 optionalMapElement: {[key: string]: {a: string}} = {[MapKeys.A] : {a: 'value'}};

<button [disabled]="!optionalMapElement[MapKeys.B]?.a"></button>

Please provide a link to a minimal reproduction of the bug

https://stackblitz.com/edit/angular-r3n3ec?file=src/app/app.component.ts

Please provide the exception or error you saw

warning NG8107: The left side of this optional chain operation does not include 'null' or 'undefined' in its type, therefore the '?.' operator can be replaced with the '.' operator.

Please provide the environment you discovered this bug in (run ng version)

Angular CLI: 14.1.0
Node: 16.15.1
Package Manager: npm 8.13.2
OS: win32 x64

Angular: 14.1.0
... animations, cli, common, compiler, compiler-cli, core, forms
... platform-browser, platform-browser-dynamic, router
... service-worker

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1401.0
@angular-devkit/build-angular   14.1.0
@angular-devkit/core            14.1.0
@angular-devkit/schematics      14.1.0
@schematics/angular             14.1.0
rxjs                            7.5.6
typescript                      4.7.4

Anything else?

No response

[pkozlowski-opensource]

@tiberiuzuld could you please share more details / reproduction scenario? We were briefly looking into it but the types and assigned values don't match thus making the problem confusing. Your type is written as any[] | null but the value being assigned ({b: null}) doesn't match this type. Did you mean [{b: null}]?

In short - the type, assigned value and expression don't seem to match / make sense together so I'm assuming there is some typo / missing info here.

[tiberiuzuld]

Hello @pkozlowski-opensource , Sorry yes, I missed typed the type of a in the description. I updated the initial description. I hope is more clear now.

[pkozlowski-opensource]

Thnx for the update @tiberiuzuld - yes indeed, it looks like a valid bug report, thnx!

[tiberiuzuld]

Hmm I was investigating the issue a bit some more. It seems our types are wrong since how I did in the stackblitz. The warning is being displayed only if we put the ? and the type of b is only as array (updated description).

Don't know if you want to include this as a bug.

[tiberiuzuld]

I found another case where I think the bug is valid. Second case with optionalArrayElement see stackblitz example in the main description.

[pkozlowski-opensource]

OK, so the first case is definitively not a bug and works as expected. The second optionalArrayElement: {a: 'value'}[] = []; case is more interesting.

[pkozlowski-opensource]

@tiberiuzuld could you please update the issue description to remove references to the first case (as this works as intended?)

[tiberiuzuld]

@pkozlowski-opensource Ok I will, also found same issue when you have a map optionalMapElement updated stackblitz. Will update the description also.

[tiberiuzuld]

Workaround possible with making a getter/method in the class ts:

get firstElement():  {a: 'value'} | undefined {
  return this.optionalArrayElement[0];
}

getElement(index: number):  {a: 'value'} | undefined {
  return this.optionalArrayElement[index];
}

<button [disabled]="firstElement?.a"></button>
<button [disabled]="getElement(0)?.a"></button>

[tiberiuzuld]

[sod]

we had a similar issue, see https://github.com/angular/angular/issues/44859#issuecomment-1024093837 - when array[1234] ?? 'fallback' threw a warning about supposedly array[1234] never being null

[BenRacicot]

I'd also like to turn off this warning.

[nunoarruda]

I also think that this NG8107 warning is broken. For example:

1. I have an optional templateForm property in my TS class of type UntypedFormGroup so its inferred types are UntypedFormGroup | undefined

2. Yet, Angular says that The left side of this optional chain operation does not include 'null' or 'undefined' in its type, therefore the '?.' operator can be replaced with the '.' operator. which is incorrect since an optional property can possibly be undefined.

[JoostK]

I also think that this NG8107 warning is broken. For example:

1. I have an optional templateForm property in my TS class of type UntypedFormGroup so its inferred types are UntypedFormGroup | undefined

2. Yet, Angular says that The left side of this optional chain operation does not include 'null' or 'undefined' in its type, therefore the '?.' operator can be replaced with the '.' operator. which is incorrect since an optional property can possibly be undefined.

@nunoarruda can you please share a runnable reproduction?

---

As for the indexed access false positive: this happens because TypeScript doesn't include undefined in the type of an indexed access expression (foo[index] syntax), so the optional chaining operator appears unnecessary based on the type system. I think it makes sense not to report this diagnostic for indexed accesses.

[nunoarruda]

@JoostK it looks like it is working fine in an isolated demo: Now the template is correctly assuming the type of the property as UntypedFormGroup | undefined. So I'm not sure what's going on with my project but I will let you folks know if I find anything along the way.

[danielehrhardt]

I'd also like to turn off this warning.

"angularCompilerOptions": {
    "strictNullChecks": false
  }

[JoostK]

I'd also like to turn off this warning.

"angularCompilerOptions": {

    "strictNullChecks": false

  }

Please do not do this. strictNullChecks is not an Angular compiler option, and disabling the TypeScript option is definitely not recommended.

This particular rule can be disabled like any other extended diagnostic, as documented here: https://angular.io/extended-diagnostics#configuration. The rule name is optionalChainNotNullable, which is currently not yet included in the docs.

[danielehrhardt]

optionalChainNotNullable does not work for me

[dcbroad3]

optionalChainNotNullable does not work for me

Can verify that this did fix it for me. Seems to require at least angular 14.1.

[robleka]

Hello. Got the same problem all over my application where things can clearly be null or undefined but I receive the warning regardless. Two examples bellow:

Angular CLI: 14.1.0 Node: 16.13.1 Package Manager: npm 8.1.2 OS: win32 x64

Angular: 14.1.0 ... animations, cdk, cli, common, compiler, compiler-cli, core ... forms, material, material-moment-adapter, platform-browser ... platform-browser-dynamic, router

Package Version

@angular-devkit/architect 0.1401.0 @angular-devkit/build-angular 14.1.0 @angular-devkit/core 14.1.0 @angular-devkit/schematics 14.1.0 @angular/flex-layout 14.0.0-beta.40 @schematics/angular 14.1.0 rxjs 7.5.6 typescript 4.7.4

[anisabboud]

@robleka the first case you mentioned is because you defined person!: (the ! is the non-null assertion operator). Replace the exclamation mark with a question mark (person?:) and the errors should go away.

(Regarding the second case, I think it's because item is defined as CommonTypes.Any, which I'm guessing maps to any.)

[robleka]

Thanks, that fixed the first issue. But the second one is still strange to me as "item" is optional. Yes, it can be of any type, but it still is optional, it might not exist or it may be null.

[nunoarruda]

Tip: watch out for optional properties inside *ngIf blocks. If you're checking if an optional property is truthy then references of that property inside that *ngIf block will always be truthy and thus the ?. operator is unnecessary. Example:

TS: user?: {firstName: string, lastName: string};

Template:

<div *ngIf="user">
    <div>{{user?.firstName}}</div> <!-- unnecessary -->
    <div>{{user.firstName}}</div> <!-- ok -->
</div>

[ghovmand]

Got the same warning:

Warning: src/main/webapp/skattekoersel/koersel/components/beregning/beregning-noegletal/beregning-noegletal.component.html:87:23 - warning NG8107: The left side of this optional chain operation does not include 'null' or 'undefined' in its type, therefore the '?.' operator can be replaced with the '.' operator.

87           {{ koersel?.noegletal?.antalKanIkkeFordeles | number }}
                         ~~~~~~~~~

After I "fixed" the warning it, got following error:

Error: src/main/webapp/skattekoersel/koersel/components/beregning/beregning-noegletal/beregning-noegletal.component.html:87:33 - error TS2532: Object is possibly 'undefined'.

87           {{ koersel?.noegletal.antalKanIkkeFordeles | number }}
                                   ~~~~~~~~~~~~~~~~~~~~

[anisabboud]

@ghovmand try koersel.noegletal?.antalKanIkkeFordeles

[JoostK]

It appears there is some confusion which optional chain is redundant. It's always the one to the left of the underlined property. If I recall correctly it wasn't possible to highlight just the operator, hence we now highlight the property name, but perhaps this can be improved as well. I'm currently on vacation though, so won't be able to look into this at the moment.

[ghovmand]

Thanks, that solved the issue for me; always look to the left :)

[e-oz]

An example of how to suppress this check right now (it's not obvious from previous comments):

  "angularCompilerOptions": {
    "extendedDiagnostics": {
      "checks": {
        "optionalChainNotNullable": "suppress"
      }
    }
  }

[Alex-Sessler]

I am seeing this warning quite a lot with this pattern:

class A {
    b: MyType
    
    constructor(private activatedRoute: ActivatedRoute) {
         b = activatedRoute.snapshot.data.b
    }
}

Which is quite common, as i tend to fetch the data in a resolver and assign it to the variable in the constructor. Now when i do a conditional check in a template like this:

<div *ngIf="b?.c"></div> It shows me the warning. I feel like angular should be able to detect that me declaring the variable without an initializer implies that i can actually be undefined and not force me to bloat my code by declaring my variable like this:

b: MyType | undefined

I definitely don't want to disable these checks, as they are definitely useful. But in this case it feels like angular should handle this case internally.

EDIT: I feel like the same should count for @ViewChild properties, as these are by definition undefined until after the template was initialized. These should also by default be recognized as possibly undefined.

[dcbroad3]

I am seeing this warning quite a lot with this pattern:

class A {
    b: MyType
    
    constructor(private activatedRoute: ActivatedRoute) {
         b = activatedRoute.snapshot.data.b
    }
}

This is effectively exactly the same as:

class A {
  b: MyType = this.activatedRoute.snapshot.data.b;
  
  constructor(private activatedRoute: ActivatedRoute) { }
}

In other words, in this class b is undefined only if the data from ActivatedRoute is undefined. In strict mode TypeScript will actually tell you, you need to add undefined to the type if the property is not assigned directly where it is declared or in the constructor.

There's no way to automatically mark things like @ViewChild as possibly undefined, that should be done manually and strict mode will require you to declare them as possibly undefined.