Skip to content

Support TypeScript 4.1#39571

Closed
clydin wants to merge 9 commits intoangular:masterfrom
clydin:ts-4.1
Closed

Support TypeScript 4.1#39571
clydin wants to merge 9 commits intoangular:masterfrom
clydin:ts-4.1

Conversation

@clydin
Copy link
Member

@clydin clydin commented Nov 5, 2020 •
edited
Loading

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 ts-4.1 branch 2 times, most recently from 2b39f9f to a40cfd7 Compare November 5, 2020 15:15
@google-cla

This comment has been minimized.

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

This comment has been minimized.

Sign in to view
@google-cla google-cla bot added cla: yes and removed cla: no labels Nov 5, 2020
@josephperrott josephperrott added the area: build & ci Related the build and CI infrastructure of the project label Nov 6, 2020
@ngbot ngbot bot added this to the needsTriage milestone Nov 6, 2020
JoostK
JoostK reviewed Nov 6, 2020
View reviewed changes
packages/compiler-cli/ngcc/src/host/commonjs_umd_utils.ts Outdated
Copy link
Member

@JoostK JoostK Nov 6, 2020

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.

alan-agius4
alan-agius4 reviewed Nov 9, 2020
View reviewed changes
Copy link
Contributor

@alan-agius4 alan-agius4 left a 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

@alan-agius4 alan-agius4 mentioned this pull request Nov 20, 2020
Closed
@clydin clydin added target: minor This PR is targeted for the next minor release and removed state: blocked labels Nov 20, 2020
@clydin clydin mentioned this pull request Nov 20, 2020
Merged
@clydin clydin marked this pull request as ready for review November 20, 2020 20:49
gkalpak
gkalpak approved these changes Nov 20, 2020
View reviewed changes
Copy link
Member

@gkalpak gkalpak left a 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

clydin and others added 4 commits November 20, 2020 17:14
@clydin
feat(compiler-cli): add support for using TypeScript 4.1
e9d1ded 
This change enables projects to be built with TypeScript 4.1.  Support for TypeScript 4.0 is also retained.
@clydin
build: support building with TypeScript 4.1
310a054 
TypeScript 4.1 is now used to build and test within the repository.
@clydin
fixup! build: support building with TypeScript 4.1
a0cb33c
@JoostK @clydin
fixup! build: support building with TypeScript 4.1
6de5ef0
gkalpak
gkalpak reviewed Nov 21, 2020
View reviewed changes
@jpike88
Copy link

jpike88 commented Nov 23, 2020

Just checking... I got the error Internal Error: The structure of the program changed during codegen when attempting to use typescript 4.1.2 with angular cli 11.0.2. Is this expected, and covered in the PR's fixes? I can't seem to find anyone mentioning that kind of error, and not sure what to make of it.

@JoostK
Copy link
Member

JoostK commented Nov 23, 2020

Just checking... I got the error Internal Error: The structure of the program changed during codegen when attempting to use typescript 4.1.2 with angular cli 11.0.2. Is this expected, and covered in the PR's fixes? I can't seem to find anyone mentioning that kind of error, and not sure what to make of it.

Yes that is expected for ViewEngine compilations, but not Ivy. And yes that is covered in this PR.

JiaLiPassion
JiaLiPassion approved these changes Nov 23, 2020
View reviewed changes
Copy link
Contributor

@JiaLiPassion JiaLiPassion left a 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

IgorMinar
IgorMinar approved these changes Nov 25, 2020
View reviewed changes
Copy link
Contributor

@IgorMinar IgorMinar left a 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

@IgorMinar IgorMinar added the action: merge The PR is ready for merge by the caretaker label Nov 25, 2020
jessicajaniuk pushed a commit that referenced this pull request Nov 25, 2020
@clydin @thePunderWoman
build: support building with TypeScript 4.1 (#39571)
318255a 
TypeScript 4.1 is now used to build and test within the repository.

PR Close #39571
jessicajaniuk pushed a commit that referenced this pull request Nov 25, 2020
@clydin @thePunderWoman
test(compiler-cli): add TypeScript 4.1 typings integration test (#39571)
4def19f 
This change adds a typings test identical to the TypeScript 4.0 test but using TypeScript 4.1 instead.

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

angular-automatic-lock-bot bot commented Dec 26, 2020

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 Dec 26, 2020
@pullapprove pullapprove bot removed the area: build & ci Related the build and CI infrastructure of the project label Dec 26, 2020
@ngbot ngbot bot removed this from the needsTriage milestone Dec 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@JoostK JoostK JoostK left review comments

@alan-agius4 alan-agius4 alan-agius4 left review comments

@alxhub alxhub alxhub approved these changes

+3 more reviewers

@gkalpak gkalpak gkalpak approved these changes

@IgorMinar IgorMinar IgorMinar approved these changes

@JiaLiPassion JiaLiPassion JiaLiPassion approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

action: merge The PR is ready for merge by the caretaker cla: yes target: minor This PR is targeted for the next minor release

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@clydin @JoostK @jpike88 @IgorMinar @JiaLiPassion @alxhub @gkalpak @alan-agius4 @josephperrott