feat(core): support TypeScript 4.7 #45749

crisbeto · 2022-04-25T07:43:58Z

Adds support for TypeScript 4.7. Changes include:

Bumping the TS version as well as some Bazel dependencies to include fix(concatjs): resolve error with TypeScript 4.7 bazelbuild/rules_nodejs#3420.
Adding a backwards-compatibility layer for calls to updateTypeParameterDeclaration.
Making LView generic in order to make it easier to type the context based on the usage. Currently the context can be 4 different types which coupled with stricter type checking would've required a lot of extra casting all over core.
Fixing a bunch of miscellaneous type errors.
Removing assertions of ReferenceEntry.isDefinition in a few of the language service tests. The field isn't returned by TS anymore and we weren't using it for anything.
Resolving an error in the language service that was caused by TS attempting to parse HTML files when we try to open them. Previous TS was silently setting them as ScriptKind.Unknown and ignoring the errors, but now it throws. I've worked around it by setting them as ScriptKind.JSX.

JoostK

I am not sure if the changes to LView are necessary/desirable, but left some comments for your consideration.

packages/compiler-cli/ngcc/test/packages/configuration_spec.ts

packages/compiler-cli/src/ngtsc/typecheck/src/ts_util.ts

packages/core/src/render3/component_ref.ts

packages/core/src/render3/instructions/change_detection.ts

JoostK · 2022-04-26T20:05:30Z

packages/core/src/render3/interfaces/view.ts

@@ -78,7 +78,7 @@ export interface OpaqueViewState {
 * Keeping separate state for each view facilities view insertion / deletion, so we
 * don't have to edit the data array based on which views are present.
 */
-export interface LView extends Array<any> {
+export interface LView<T = unknown> extends Array<any> {


Can you expand on the motivation for these changes, e.g. in the commit message? It's unclear why this has been changed and it has quite a knock-on effect throughout the codebase.

It's because LView[CONTEXT] can have 4 different types which made it difficult to deal with when combined with some of the new type checking changes. This saves us some as unknown as <something> casts across the codebase.

JoostK · 2022-04-26T20:06:56Z

packages/core/src/render3/state.ts

@@ -266,8 +266,8 @@ export function ɵɵdisableBindings(): void {
 /**
 * Return the current `LView`.
 */
-export function getLView(): LView {
-  return instructionState.lFrame.lView;
+export function getLView<T>(): LView<T> {


It's generally a bad idea to have a generic type in an output position only, as it is guaranteed to be unsafe (i.e. the caller can specify any type argument without type-checking against some concrete value). I realise this isn't the first usage of such pattern (there's lots actually!) but it makes the reason why LView has a generic type less effective IMHO.

packages/language-service/test/references_and_rename_spec.ts

packages/language-service/testing/src/project.ts

atscott

reviewed-for: language-service

crisbeto · 2022-04-27T16:30:36Z

I've addressed the feedback and expanded the PR description with more info on the changes.

josephperrott

LGTM overall though I share some of @JoostK's concerns about the unsafe usage of generics with LView.

An additional question is if we should be landing this with ts as beta or wait until its at least in rc.

crisbeto · 2022-04-27T18:04:40Z

Due to the feature freeze, we have to land it as a beta and then update to RC during our RC period. The final version should be out before the v14 final release.

AndrewKushnir · 2022-04-28T02:30:44Z

packages/core/src/render3/interfaces/view.ts

@@ -78,7 +78,7 @@ export interface OpaqueViewState {
 * Keeping separate state for each view facilities view insertion / deletion, so we
 * don't have to edit the data array based on which views are present.
 */
-export interface LView extends Array<any> {
+export interface LView<T = unknown> extends Array<any> {


I'd propose adding more descriptive generic name:

Suggested change

export interface LView<T = unknown> extends Array<any> {

export interface LView<Context = unknown> extends Array<any> {

I don't mind making the change, but it goes against the convention we have everywhere else where we use single-character names for generics.

I personally feel clarity is better than convention in this case.

JoostK

I continue to feel uncomfortable with the generic type arg for LView, but I haven't seen what the alternative would look like so can't compare which is better/worse.

package.json

packages/core/src/render3/interfaces/view.ts

crisbeto · 2022-04-28T20:20:41Z

I've addressed the latest feedback. Regarding making LView generic, I did it because I found my self having to cast foo as unknown as Something in a bunch of places so having it be generic made sense. We should also keep in mind that this is an internal API so we can change it at any time if we want to.

AndrewKushnir

Reviewed-for: public-api

jessicajaniuk

reviewed-for: public-api

jessicajaniuk · 2022-04-29T10:16:07Z

packages/core/src/render3/interfaces/view.ts

@@ -78,7 +78,7 @@ export interface OpaqueViewState {
 * Keeping separate state for each view facilities view insertion / deletion, so we
 * don't have to edit the data array based on which views are present.
 */
-export interface LView extends Array<any> {
+export interface LView<T = unknown> extends Array<any> {


I personally feel clarity is better than convention in this case.

Adds support for TypeScript 4.7. Changes include: * Bumping the TS version as well as some Bazel dependencies to include bazelbuild/rules_nodejs#3420. * Adding a backwards-compatibility layer for calls to `updateTypeParameterDeclaration`. * Making `LView` generic in order to make it easier to type the context based on the usage. Currently the context can be 4 different types which coupled with stricter type checking would required a lot of extra casting all over `core`. * Fixing a bunch of miscellaneous type errors. * Removing assertions of `ReferenceEntry.isDefinition` in a few of the language service tests. The field isn't returned by TS anymore and we weren't using it for anything. * Resolving in error in the language service that was caused by TS attempting to parse HTML files when we try to open them. Previous TS was silently setting them as `ScriptKind.Unknown` and ignoring the errors, but now it throws. I've worked around it by setting them as `ScriptKind.JSX`.

crisbeto · 2022-04-29T12:41:03Z

I've pushed an extra integration test which tries to import all our types using the new module resolution option.

jelbourn

LGTM

Reviewed-for: public-api

dylhunn · 2022-04-29T16:19:47Z

This PR was merged into the repository by commit 29039fc.

- [Angular supports TypeScript 4.7 until v14](angular/angular#45749)

angular-automatic-lock-bot · 2022-05-30T16:06:06Z

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.}

crisbeto force-pushed the ts-4.7 branch 8 times, most recently from bea8850 to 091beed Compare April 26, 2022 08:16

crisbeto added action: review The PR is still awaiting reviews from at least one requested reviewer area: core Issues related to the framework runtime target: major This PR is targeted for the next major release labels Apr 26, 2022

ngbot bot modified the milestone: Backlog Apr 26, 2022

crisbeto added this to the v14-candidates milestone Apr 26, 2022

crisbeto marked this pull request as ready for review April 26, 2022 16:31

pullapprove bot requested review from alxhub, atscott, JiaLiPassion and josephperrott April 26, 2022 16:31

JoostK reviewed Apr 26, 2022

View reviewed changes

atscott approved these changes Apr 26, 2022

View reviewed changes

crisbeto force-pushed the ts-4.7 branch from 091beed to f985862 Compare April 27, 2022 16:29

crisbeto requested review from AndrewKushnir and removed request for alxhub April 27, 2022 16:31

josephperrott approved these changes Apr 27, 2022

View reviewed changes

JiaLiPassion approved these changes Apr 27, 2022

View reviewed changes

AndrewKushnir reviewed Apr 28, 2022

View reviewed changes

crisbeto force-pushed the ts-4.7 branch from f985862 to 877ffb5 Compare April 28, 2022 17:50

JoostK reviewed Apr 28, 2022

View reviewed changes

package.json Show resolved Hide resolved

packages/core/src/render3/interfaces/view.ts Outdated Show resolved Hide resolved

crisbeto force-pushed the ts-4.7 branch from 877ffb5 to ee395b8 Compare April 28, 2022 20:18

JoostK approved these changes Apr 28, 2022

View reviewed changes

AndrewKushnir approved these changes Apr 29, 2022

View reviewed changes

pullapprove bot requested review from AndrewKushnir, atscott, dylhunn and jessicajaniuk April 29, 2022 04:25

AndrewKushnir approved these changes Apr 29, 2022

View reviewed changes

pullapprove bot requested a review from alxhub April 29, 2022 04:45

jessicajaniuk approved these changes Apr 29, 2022

View reviewed changes

crisbeto force-pushed the ts-4.7 branch from ee395b8 to d44c90a Compare April 29, 2022 10:42

crisbeto force-pushed the ts-4.7 branch from d44c90a to 010f3b3 Compare April 29, 2022 12:39

jelbourn approved these changes Apr 29, 2022

View reviewed changes

crisbeto removed request for atscott, dylhunn and alxhub April 29, 2022 14:38

crisbeto added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Apr 29, 2022

dylhunn closed this in 29039fc Apr 29, 2022

akunzai added a commit to akunzai/angular-boilerplate that referenced this pull request May 25, 2022

Ignore to renovate typescript

b12206d

- [Angular supports TypeScript 4.7 until v14](angular/angular#45749)

akunzai mentioned this pull request May 25, 2022

Ignore to renovate typescript akunzai/angular-boilerplate#523

Merged

angular-automatic-lock-bot bot locked and limited conversation to collaborators May 30, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(core): support TypeScript 4.7 #45749

feat(core): support TypeScript 4.7 #45749

crisbeto commented Apr 25, 2022 •

edited

JoostK left a comment

JoostK Apr 26, 2022

crisbeto Apr 26, 2022

JoostK Apr 26, 2022

atscott left a comment

crisbeto commented Apr 27, 2022

josephperrott left a comment

crisbeto commented Apr 27, 2022

AndrewKushnir Apr 28, 2022

crisbeto Apr 28, 2022

jessicajaniuk Apr 29, 2022

JoostK left a comment

crisbeto commented Apr 28, 2022

AndrewKushnir left a comment

jessicajaniuk left a comment

jessicajaniuk Apr 29, 2022

crisbeto commented Apr 29, 2022

jelbourn left a comment

dylhunn commented Apr 29, 2022

angular-automatic-lock-bot bot commented May 30, 2022

	export interface LView<T = unknown> extends Array<any> {
	export interface LView<Context = unknown> extends Array<any> {

feat(core): support TypeScript 4.7 #45749

feat(core): support TypeScript 4.7 #45749

Conversation

crisbeto commented Apr 25, 2022 • edited

JoostK left a comment

Choose a reason for hiding this comment

JoostK Apr 26, 2022

Choose a reason for hiding this comment

crisbeto Apr 26, 2022

Choose a reason for hiding this comment

JoostK Apr 26, 2022

Choose a reason for hiding this comment

atscott left a comment

Choose a reason for hiding this comment

crisbeto commented Apr 27, 2022

josephperrott left a comment

Choose a reason for hiding this comment

crisbeto commented Apr 27, 2022

AndrewKushnir Apr 28, 2022

Choose a reason for hiding this comment

crisbeto Apr 28, 2022

Choose a reason for hiding this comment

jessicajaniuk Apr 29, 2022

Choose a reason for hiding this comment

JoostK left a comment

Choose a reason for hiding this comment

crisbeto commented Apr 28, 2022

AndrewKushnir left a comment

Choose a reason for hiding this comment

jessicajaniuk left a comment

Choose a reason for hiding this comment

jessicajaniuk Apr 29, 2022

Choose a reason for hiding this comment

crisbeto commented Apr 29, 2022

jelbourn left a comment

Choose a reason for hiding this comment

dylhunn commented Apr 29, 2022

angular-automatic-lock-bot bot commented May 30, 2022

crisbeto commented Apr 25, 2022 •

edited