-
Notifications
You must be signed in to change notification settings - Fork 24.8k
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
Support TypeScript 4.1 #39571
Support TypeScript 4.1 #39571
Conversation
2b39f9f
to
a40cfd7
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@@ -78,7 +78,7 @@ export function findNamespaceOfIdentifier(id: ts.Identifier): ts.Identifier|null | |||
export function findRequireCallReference(id: ts.Identifier, checker: ts.TypeChecker): RequireCall| | |||
null { | |||
const symbol = checker.getSymbolAtLocation(id) || null; | |||
const declaration = symbol && symbol.valueDeclaration; | |||
const declaration = symbol?.valueDeclaration ?? symbol?.declarations?.[0]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change seems to have been due to microsoft/TypeScript#39770, where the variable declaration is now treated as an alias declaration which no longer has a value declaration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add an integration tests like https://github.com/angular/angular/tree/master/integration/typings_test_ts40
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM for dev-infra
, docs-infra
, fw-upgrade
.
From a quick look, I would expect the paths
without baseUrl
breaking change to affect ngcc
, because we iirc we assume baseUrl
will be present. For example, ngcc
seems to ignore paths
if there is no baseUrl
.
Reviewed-for: dev-infra, docs-infra, fw-upgrade
This change enables projects to be built with TypeScript 4.1. Support for TypeScript 4.0 is also retained.
TypeScript 4.1 is now used to build and test within the repository.
This change adds a typings test identical to the TypeScript 4.0 test but using TypeScript 4.1 instead.
Just checking... I got the error |
Yes that is expected for ViewEngine compilations, but not Ivy. And yes that is covered in this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Reviewed-for: zone-js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thank you!
Reviewed-for: global-approvers
TypeScript 4.1 is now used to build and test within the repository. PR Close #39571
This change adds a typings test identical to the TypeScript 4.0 test but using TypeScript 4.1 instead. PR Close #39571
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
This change updates the framework to support TypeScript 4.1.