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

feat(core): support TypeScript 4.3 #42022

Closed

Conversation

devversion
Copy link
Member

@devversion devversion commented May 10, 2021

Switches the repository to TypeScript 4.3 and the latest version of tslib. This involves updating the peer dependency ranges on typescript for the compiler CLI and for the Bazel package. Tests for new TypeScript features have been added to ensure compatibility with Angular's ngtsc compiler.

@google-cla google-cla bot added the cla: yes label May 10, 2021
@devversion devversion added area: core Issues related to the framework runtime state: WIP labels May 10, 2021
@ngbot ngbot bot modified the milestone: Backlog May 10, 2021
@devversion devversion force-pushed the refactor/support-typescript-4.3 branch 3 times, most recently from 468786c to c93273a Compare May 17, 2021 13:58
@devversion devversion added action: review The PR is still awaiting reviews from at least one requested reviewer and removed state: WIP labels May 17, 2021
@devversion
Copy link
Member Author

This is ready for review; but not ready for merge (given TS 4.3 not being released yet). It would be great to get a early review from @angular/fw-compiler and @angular/fw-ngcc, as well as @kyliau for the language-service. Please have a look 😄

@devversion devversion requested review from kyliau, a team and josephperrott May 17, 2021 17:33
aio/aio-builds-setup/dockerbuild/scripts-js/package.json Outdated Show resolved Hide resolved
packages/animations/package.json Show resolved Hide resolved
packages/compiler-cli/src/ngtsc/typecheck/api/symbols.ts Outdated Show resolved Hide resolved
@@ -368,6 +368,36 @@ export declare class AnimationEvent {
expect(diags.length).toBe(0);
});

// https://devblogs.microsoft.com/typescript/announcing-typescript-4-3-beta/#separate-write-types-on-properties
it('should support separate write types on inputs', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably include a note in the documentation about input coercion that TS 4.3 onwards supports it natively.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, let's do that as a follow-up. I'd like to change the section in AIO template-typecheck.md and I'm not sure yet how that would look like.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM 👍

Copy link
Member

@josephperrott josephperrott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing seems to be an issue from my persective. google3 presubmit also started.

Copy link
Member

@petebacondarwin petebacondarwin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good!

@devversion devversion force-pushed the refactor/support-typescript-4.3 branch from c93273a to 2d2826a Compare May 25, 2021 14:55
@devversion devversion added the target: minor This PR is targeted for the next minor release label May 25, 2021
Copy link
Contributor

@kyliau kyliau 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 language service

Switches the repository to TypeScript 4.3 and the latest
version of tslib. This involves updating the peer dependency
ranges on `typescript` for the compiler CLI and for the Bazel
package. Tests for new TypeScript features have been added to
ensure compatibility with Angular's ngtsc compiler.
…t regression

Updates the compiler-cli compliance goldens. The golden updates are
required due to a regression in TypeScript 4.3 that causes the emitter
to not incorrectly preserve lines when emitting a node list.

This can be reverted once microsoft/TypeScript#44070
is available.
@devversion devversion force-pushed the refactor/support-typescript-4.3 branch from 2d2826a to 59a0810 Compare May 26, 2021 19:44
@devversion devversion changed the title [WIP] feat(core): support TypeScript 4.3 feat(core): support TypeScript 4.3 May 26, 2021
@devversion devversion marked this pull request as ready for review May 26, 2021 20:31
@IgorMinar IgorMinar added the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label May 26, 2021
@devversion devversion removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label May 28, 2021
@petebacondarwin petebacondarwin removed the action: review The PR is still awaiting reviews from at least one requested reviewer label May 28, 2021
@devversion devversion added the action: merge The PR is ready for merge by the caretaker label May 31, 2021
@ngbot
Copy link

ngbot bot commented May 31, 2021

I see that you just added the action: merge label, but the following checks are still failing:
    failure status "google3" is failing

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken master, please try rebasing to master and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

@devversion devversion removed the action: merge The PR is ready for merge by the caretaker label May 31, 2021
@IgorMinar
Copy link
Contributor

I reran the g3 presubmit - there is just one failure and it seems unrelated, so I'm almost certain that we'll be fine in g3.

@devversion devversion added action: merge The PR is ready for merge by the caretaker action: presubmit The PR is in need of a google3 presubmit labels Jun 3, 2021
@ngbot

This comment has been minimized.

@AndrewKushnir AndrewKushnir removed the action: presubmit The PR is in need of a google3 presubmit label Jun 3, 2021
@AndrewKushnir
Copy link
Contributor

FYI presubmits look good and since this change is also compatible with TS 4.2 it should be safe to land. Thank you.

@jessicajaniuk jessicajaniuk added action: merge The PR is ready for merge by the caretaker and removed action: merge The PR is ready for merge by the caretaker labels Jun 4, 2021
jessicajaniuk pushed a commit that referenced this pull request Jun 4, 2021
…t regression (#42022)

Updates the compiler-cli compliance goldens. The golden updates are
required due to a regression in TypeScript 4.3 that causes the emitter
to not incorrectly preserve lines when emitting a node list.

This can be reverted once microsoft/TypeScript#44070
is available.

PR Close #42022
devversion added a commit to devversion/angular that referenced this pull request Jun 18, 2021
dylhunn pushed a commit that referenced this pull request Jun 21, 2021
@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 Jul 5, 2021
TeriGlover pushed a commit to TeriGlover/angular that referenced this pull request Jul 21, 2021
…t regression (angular#42022)

Updates the compiler-cli compliance goldens. The golden updates are
required due to a regression in TypeScript 4.3 that causes the emitter
to not incorrectly preserve lines when emitting a node list.

This can be reverted once microsoft/TypeScript#44070
is available.

PR Close angular#42022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: core Issues related to the framework runtime cla: yes target: minor This PR is targeted for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants