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

Support TypeScript 4.1 #39571

Open
wants to merge 10 commits into
base: master
from
Open

Support TypeScript 4.1 #39571

wants to merge 10 commits into from

Conversation

@clydin
Copy link
Member

@clydin clydin commented Nov 5, 2020

This change updates the framework to support TypeScript 4.1.

@google-cla google-cla bot added the cla: yes label Nov 5, 2020
@clydin clydin force-pushed the clydin:ts-4.1 branch 2 times, most recently from 2b39f9f to a40cfd7 Nov 5, 2020
@google-cla

This comment has been hidden.

@google-cla google-cla bot added cla: no and removed cla: yes labels Nov 5, 2020
@JoostK

This comment has been hidden.

@google-cla google-cla bot added cla: yes and removed cla: no labels Nov 5, 2020
@ngbot ngbot bot added this to the needsTriage milestone Nov 6, 2020
@@ -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];

This comment has been minimized.

@JoostK

JoostK Nov 6, 2020
Member

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.

Copy link
Contributor

@alan-agius4 alan-agius4 left a comment

@clydin clydin force-pushed the clydin:ts-4.1 branch from 25ee0df to cfbfcf7 Nov 20, 2020
@clydin clydin added target: minor and removed state: blocked labels Nov 20, 2020
@clydin clydin force-pushed the clydin:ts-4.1 branch from cfbfcf7 to 8ed654b Nov 20, 2020
@clydin clydin marked this pull request as ready for review Nov 20, 2020
@pullapprove pullapprove bot requested review from alxhub, gkalpak, IgorMinar and JiaLiPassion Nov 20, 2020
Copy link
Member

@gkalpak gkalpak left a comment

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

aio/package.json Outdated Show resolved Hide resolved
setTempProgramHandlerForTest(program => {
structureReuse = tsStructureIsReused(program);
});
compile();

This comment has been minimized.

@gkalpak

gkalpak Nov 20, 2020
Member

Wouldn't it be safer to call resetTempProgramHandlerForTest() right after compile()?

This comment has been minimized.

@JoostK

JoostK Nov 20, 2020
Member

That would work, but we'd have to be sure that it is exception safe so the afterEach is still nice to have IMO (or wrap it in try-finally)

clydin and others added 4 commits Nov 5, 2020
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.
JoostK and others added 5 commits Nov 5, 2020
This change adds a typings test identical to the TypeScript 4.0 test but using TypeScript 4.1 instead.
@clydin clydin force-pushed the clydin:ts-4.1 branch from 877fb35 to 72ae869 Nov 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.