SimpleChanges type definition is incorrest

[yohny]

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

core

Is this a regression?

No

Description

Currently the SimpleChanges type is defined as:

export declare interface SimpleChanges {
    [propName: string]: SimpleChange;
}

which means that whatever string index property you use, it will be defined and have value of SimpleChange.
This is however not true as only the changed component properties will be present, the rest will be undefined, so type definition should be:

export declare interface SimpleChanges {
    [propName: string]: SimpleChange | undefined;
}

Common implementation of ngOnChanges check if the property of interest have changed, like so:

ngOnChanges(changes: SimpleChanges): void {
    if (changes['myPropertyOfInterest']) {
      // do stuff with it
    }
}

but with current typing linters/code analysers correctly complain that such confition is always true.

Please provide a link to a minimal reproduction of the bug

https://typescript-eslint.io/play/#ts=4.9.3&sourceType=module&code=CYUwxgNghgTiAEBLAdgFxDAZlMCDKiAtgA4QgDCAFlMgOYgDO8A3gLABQ88A2sTAPbEAclEIgAXPAaoYKWgF1JBEmSo16Abg4BfDh1CRYCQwybLSFanQRtO8PiABuifgFcGANSgRXE+DQBPLTswVxg4NC8fP0DgrkxEGGk1a0kAI35+Mho4+DB+ZGkYVzBUfhgACgdnN09vX0lAgBo8sIjUKIb-ZACWhKTUFPp0zOzkAEpcxAYAMUTkq3oK8ZGskBydPXZMV2RSl2R4OgB5ZCHGCrBFxiUiC3OGFfhHfkRgFg4uREx4S+uGbgAckIAQACgJiBhUAFjpgAJJoDCMVCA+TjD52LjwAD02PgwH4UlQrkwPwA7ohUJQkKhPvBdOxtEA&eslintrc=N4KABGBEBOCuA2BTAzpAXGUEKQAIBcBPABxQGNoBLY-AWhXkoDt8B6Jge1tiacTJTIAhtEK0yHJgBNK+SpPRgA2pETRoHaJAC64MAF8Q+oA&tsconfig=N4KABGBEDGD2C2AHAlgGwKYCcDyiAuysAdgM6QBcYoEEkJemy0eAcgK6qoDCAFutAGsylBm3TgwAXxCSgA

Please provide the exception or error you saw

No response

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

Angular CLI: 15.2.1        
Node: 16.17.1
Package Manager: npm 8.15.0
OS: win32 x64

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

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1502.1
@angular-devkit/build-angular   15.2.1
@angular-devkit/core            15.2.1
@angular-devkit/schematics      15.2.1
@schematics/angular             15.2.1
ng-packagr                      15.2.2
rxjs                            7.8.0
typescript                      4.9.5

Anything else?

No response

[JeanMeche]

Hi, this has already been reported by #48752 !

[JoostK]

Yep, this is a TypeScript quirk and a duplicate of #48752.

[yohny]

I agree that it is a duplicate (I was not able to find #48752 when creating this one, sry), but the orignal argument for closing should be reconsidered.
The orignal argument was that it is a language quirk as stated here microsoft/TypeScript#13778. However even in that discussion there is a suggestion that in library you can include undefined in index signature typing if it makes sense. They just didnt want to to make it global default with strictNullCheck as it would make working with normal arrays cumbersome. I believe that in SimpleChanges index signature adding undefined is justified as it better reflects the reality.

[uap-universe]

I want to add that noUncheckedIndexAccess is actually not a good way to fix this, because that also affects index access for arrays and not only hash tables. For example this simple code:

return (0 <= i && i < myarray.length) ? myarray[i].somevalue  :  somedefault;

would produce an error, even though the index access is obviously checked (in some sense). That makes noUncheckedIndexAccess practically unusable as an ad-hoc switch for large code bases. It is only useful for new projects that adapt to a certain way of accessing arrays or hashtables right from the start.

[JeanMeche]

@uap-universe That's an cumbersome way to check array bounds when you have nowadays optional chaining and null coalescing.

[uap-universe]

@JeanMeche yes, but I prefer not only to think about myself and my own problems but also what it might mean for the millions of other people out there, that are not so "smart" and up to date as I am.

Look, I am just saying, that it is a bad suggestion for people to say "enable this compiler option" effectively generating more problems for them than they had before. This only encourages people to disable linter and modern compiler features altogether.

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}