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

After running the control flow migration strict template checking is enforced. #52969

Closed
penfold opened this issue Nov 16, 2023 · 9 comments
Closed
Assignees
Labels
area: core Issues related to the framework runtime bug core: control flow Issues related to the built-in control flow (@if, @for, @switch) state: has PR
Milestone

Comments

@penfold
Copy link

penfold commented Nov 16, 2023

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

core

Is this a regression?

No

Description

I have a legacy app which doesn't have strictTemplates enabled.

I updated to ng 17 and then built the project without error.

I then ran the preview migration script to convert the project to the new flow control.

I'm now getting errors consistent with strictTemplates. (These aren't issues with the migration going wrong.)

e.g.
error TS2339: Property 'YYYYYYY' does not exist on type 'XXXXXX'.

Can I force strictTemplates off again? (my tsconfig is unchanged between the 2 builds).

Please provide a link to a minimal reproduction of the bug

No response

Please provide the exception or error you saw

No response

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

Angular CLI: 17.0.1
Node: 18.17.1
Package Manager: npm 9.8.1
OS: win32 x64

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

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1700.1
@angular-devkit/build-angular   17.0.1
@angular-devkit/core            17.0.1
@angular-devkit/schematics      17.0.1
@angular/cli                    17.0.1
@angular/ssr                    17.0.1
@angular/youtube-player         17.0.0
@schematics/angular             17.0.1
rxjs                            7.8.1
typescript                      5.2.2
zone.js                         0.14.2

Anything else?

No response

@penfold
Copy link
Author

penfold commented Nov 16, 2023

To limit this down...

As soon as the new flow control is used in that template, then strictTemplates are enforced. Other 'non-strict' templates using the old flow control don't trigger strict errors.

@jessicajaniuk jessicajaniuk added area: core Issues related to the framework runtime core: control flow Issues related to the built-in control flow (@if, @for, @switch) labels Nov 16, 2023
@ngbot ngbot bot added this to the needsTriage milestone Nov 16, 2023
@JoostK
Copy link
Member

JoostK commented Nov 16, 2023

This was discussed in the team today, and the new control flow will not adopt a mode where it disables type-narrowing capabilities and type-checking of its template block. If this poses a problem for your project then continuing to use ngIf is the way to go, until type-check errors have been resolved. Documentation on this matter will be improved to surface this aspect as a consideration for the control-flow migration.

@penfold
Copy link
Author

penfold commented Nov 17, 2023

Ouch! I can see your perspective (and I assume there are some technical details hidden away in your choice). But this is going to bite a lot of people and take the shine off things.

I do have some follow-up questions/suggestions?

  • If a project (or nested tsconfig) has strictTemplates:false then add a warning/checkpoint to the migration script.
  • Have an option to not migrate subfolders who's tsconfig is set to strictTemplates:false
  • In the future (sorry, star gazing), do you envisage a point where if just one component uses the new control flow then all of the project will have the strictTemplate checks active - if so, this will be nail in the coffin for my project (running 7 years from NG 1.08 with all the highs and lows along the way).

I do love the new flow control and strictTemplates! It's just that we have been on a journey from a place where these didn't exist.

@alxhub
Copy link
Member

alxhub commented Dec 6, 2023

Another option we're considering is to have the migration add $any casts when strictTemplates isn't enabled:

@if ($any(cond)) {
  ...
}

This is how you would opt out of strictTemplates for a particular if block.

In the future (sorry, star gazing), do you envisage a point where if just one component uses the new control flow then all of the project will have the strictTemplate checks active

Control flow will function fine with strictTemplates off, it's just more type-safe than *ngIf which might result in type errors being reported where they weren't before. You can always use a $any cast for a problematic check if the code in question is not ready for such checks.

Eventually we do plan to make strictTemplates the only mode of building Angular (essentially deprecating and removing the strictTemplates: false mode).

@Harpush
Copy link

Harpush commented Dec 7, 2023

This is quite a problem especially as we don't have a good solution for #49161

@melkorCBA
Copy link

This was discussed in the team today, and the new control flow will not adopt a mode where it disables type-narrowing capabilities and type-checking of its template block. If this poses a problem for your project then continuing to use ngIf is the way to go, until type-check errors have been resolved. Documentation on this matter will be improved to surface this aspect as a consideration for the control-flow migration.

So Help me understand this, will you? 😊.

In the basic mode where strictTemplates is not enabled, it doesn't check embedded views, such as *ngIf, *ngFor, other <ng-template>. But for new control flows, it does.

@JoostK
Copy link
Member

JoostK commented Dec 21, 2023

So Help me understand this, will you? 😊.

In the basic mode where strictTemplates is not enabled, it doesn't check embedded views, such as *ngIf, *ngFor, other <ng-template>. But for new control flows, it does.

Indeed, this is correct.

@pkozlowski-opensource
Copy link
Member

After re-discussing it internally with @alxhub and @JoostK we've decided to address this bug. While we really want to push people towards adopting strict template checks, we don't want to block adoption of the new control flow.

@crisbeto crisbeto self-assigned this Apr 16, 2024
crisbeto added a commit to crisbeto/angular that referenced this issue Apr 16, 2024
Angular only checks the contents of template nodes in full type checking mode. After v17, the new control flow always had its body checked, even in basic mode, which started revealing compilation errors for apps that were using the schematic to automatically switch to the new syntax.

These changes mimic the old behavior by not checking the bodies of `if`, `switch` and `for` blocks in basic mode. Note that the expressions of the blocks are still going to be checked.

Fixes angular#52969.
crisbeto added a commit to crisbeto/angular that referenced this issue Apr 16, 2024
Angular only checks the contents of template nodes in full type checking mode. After v17, the new control flow always had its body checked, even in basic mode, which started revealing compilation errors for apps that were using the schematic to automatically switch to the new syntax.

These changes mimic the old behavior by not checking the bodies of `if`, `switch` and `for` blocks in basic mode. Note that the expressions of the blocks are still going to be checked.

Fixes angular#52969.
crisbeto added a commit to crisbeto/angular that referenced this issue Apr 16, 2024
…in basic mode

Angular only checks the contents of template nodes in full type checking mode. After v17, the new control flow always had its body checked, even in basic mode, which started revealing compilation errors for apps that were using the schematic to automatically switch to the new syntax.

These changes mimic the old behavior by not checking the bodies of `if`, `switch` and `for` blocks in basic mode. Note that the expressions of the blocks are still going to be checked.

Fixes angular#52969.
crisbeto added a commit to crisbeto/angular that referenced this issue Apr 18, 2024
…in basic mode

Angular only checks the contents of template nodes in full type checking mode. After v17, the new control flow always had its body checked, even in basic mode, which started revealing compilation errors for apps that were using the schematic to automatically switch to the new syntax.

These changes mimic the old behavior by not checking the bodies of `if`, `switch` and `for` blocks in basic mode. Note that the expressions of the blocks are still going to be checked.

Fixes angular#52969.
@alxhub alxhub closed this as completed in 7a16d7e Apr 19, 2024
crisbeto added a commit to crisbeto/angular that referenced this issue Apr 26, 2024
…in basic mode

Angular only checks the contents of template nodes in full type checking mode. After v17, the new control flow always had its body checked, even in basic mode, which started revealing compilation errors for apps that were using the schematic to automatically switch to the new syntax.

These changes mimic the old behavior by not checking the bodies of `if`, `switch` and `for` blocks in basic mode. Note that the expressions of the blocks are still going to be checked.

Fixes angular#52969.
AndrewKushnir pushed a commit that referenced this issue Apr 26, 2024
…in basic mode (#55558)

Angular only checks the contents of template nodes in full type checking mode. After v17, the new control flow always had its body checked, even in basic mode, which started revealing compilation errors for apps that were using the schematic to automatically switch to the new syntax.

These changes mimic the old behavior by not checking the bodies of `if`, `switch` and `for` blocks in basic mode. Note that the expressions of the blocks are still going to be checked.

Fixes #52969.

PR Close #55558
@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators May 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: core Issues related to the framework runtime bug core: control flow Issues related to the built-in control flow (@if, @for, @switch) state: has PR
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants