Skip to content

build: remove typescript 3.6 and 3.7 support - #36329

Closed
atscott wants to merge 2 commits into
angular:masterfrom
atscott:ts3637
Closed

atscott wants to merge 2 commits into
angular:masterfrom
atscott:ts3637

Conversation

@atscott

@atscott atscott commented Mar 30, 2020

Copy link
Copy Markdown
Contributor

BREAKING CHANGE: typescript 3.6 and 3.7 are no longer supported, please
update to typescript 3.8

@atscott atscott added area: build & ci Related the build and CI infrastructure of the project target: major This PR is targeted for the next major release labels Mar 30, 2020
@atscott atscott added this to the v10-candidates milestone Mar 30, 2020
@atscott
atscott force-pushed the ts3637 branch 3 times, most recently from e192eb0 to 1aa6349 Compare March 30, 2020 23:24
@atscott atscott changed the title build: remove tyescript 3.6 and 3.7 support build: remove typescript 3.6 and 3.7 support Mar 30, 2020
@atscott
atscott force-pushed the ts3637 branch 2 times, most recently from a0d37e0 to 84a3bc0 Compare March 30, 2020 23:42
@atscott
atscott force-pushed the ts3637 branch 6 times, most recently from 9305651 to 25f8d86 Compare March 31, 2020 21:21
@atscott
atscott marked this pull request as ready for review March 31, 2020 21:54

@filipesilva filipesilva left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is the next master a minor or a major?

If it's a minor, this change raises discoverability problems. A user won't know to update TS when updating Angular. They can only know after they get an error for the AOT compiler. For setups that skip the TS version check, they might not even get that error.

If it's a major, then ng update should have a migration for TS. Users are instructed to run ng update between majors but not between minors.

@atscott

atscott commented Apr 1, 2020 •

Copy link
Copy Markdown
Contributor Author

@IgorMinar can correct me if I'm wrong, but I believe the next release on master is 10.0.0-next.0. Either way, I'm going to split this PR up into two commits and we can drop the second one if it's not the right time for it:

  1. updating the dependency versions for angular to 9.1 and typescript to 3.8.3
  2. dropping the 3.6 and 3.7 support

@atscott

atscott commented Apr 1, 2020

Copy link
Copy Markdown
Contributor Author

@filipesilva

Copy link
Copy Markdown
Contributor

@atscott I think @clydin also noticed something similar, with regards to Build Optimizer. Correct me if I'm wrong though.

@clydin

clydin commented Apr 1, 2020

Copy link
Copy Markdown
Member

The issue I'm currently investigating relates to a runtime error due to incorrect import eliding. It appears that TS 3.8 when used within build optimizer is incorrectly marking certain imports as unreferenced. However, this only appears to occur within the very specific build optimizer usage.
In regards to memory, I haven't noticed any issues.

From the logs, the errors appear to be occurring during the ngcc parallel worker phase (one of them looks to be with a standalone ngcc execution). Do these errors occur with 9.1 when using TS 3.7? It looks like ngcc will limit the number of process workers to 8 but that may be too much for that CI instance (4 GB of memory). CPU counts on CI systems tend to be inaccurate due to the number of underlying virtual CPUs being quite high (16/32/128/etc.) so even though it's a 2 CPU instance the count will most likely not be 2.

@atscott

atscott commented Apr 1, 2020

Copy link
Copy Markdown
Contributor Author

@clydin - based on my very unscientific testing in #36380, it does seem like the issue happens with the update to 9.1, not the TS 3.8 upgrade: https://app.circleci.com/pipelines/github/angular/angular/12538/workflows/5f75318c-510a-4de9-aa86-8862a0efcc30/jobs/668882

@JoostK

JoostK commented Apr 1, 2020

Copy link
Copy Markdown
Member

FYI, @gkalpak has been investigating ngcc worker crashes, which have also been observed on ngcc-validation CI since 9.1. There's FW-2008 to track that work.

@clydin

clydin commented Apr 1, 2020

Copy link
Copy Markdown
Member

From dealing with similar issues within the CLI's build pipeline, the minimum of 8 workers may be too large in relation to the memory usage profile of the ngcc workers.

@atscott
atscott force-pushed the ts3637 branch 2 times, most recently from ee1193d to ef97ccb Compare April 30, 2020 23:39
@atscott

atscott commented May 1, 2020

Copy link
Copy Markdown
Contributor Author

Hi @filipesilva and @IgorMinar - PTAL. This PR has been rebased with master since FW-2008 was resolved. I was also having issues with the main yarn.lock so @josephperrott recommended updating the keys to v7 in config.yml.

@gregmagolan

Copy link
Copy Markdown
Contributor

Hi @filipesilva and @IgorMinar - PTAL. This PR has been rebased with master since FW-2008 was resolved. I was also having issues with the main yarn.lock so @josephperrott recommended updating the keys to v7 in config.yml.

Glad that fixed it. Another +1 to remove the fallback cache. Having an incremental node_modules update on yarn.lock changes has broken things a few times now. @alan-agius4 did you have some measurements on fallback cache performance?

@IgorMinar IgorMinar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm still good with this change.

LGTM! Thank you!

Reviewed-for: global-approverse

@filipesilva

Copy link
Copy Markdown
Contributor

@alan-agius4 looked into the Angular CLI fallback cache in angular/angular-cli#17533 and removed it because it wasn't worth it.

@gkalpak

gkalpak commented May 1, 2020

Copy link
Copy Markdown
Member

In order to avoid potential CI flakes, this should not be merged before #36145 (which in turn is blocked on getting a released version that includes the fix from #36626).

@kara kara left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

atscott added 2 commits May 5, 2020 12:14
Update the typescript version to 3.8 as well as the Angular version to
9.1, which is the one which added TS 3.8 support.
Remove TypeScript 3.6 and 3.7 support from Angular along with tests that
ensure those TS versions work.

BREAKING CHANGE: typescript 3.6 and 3.7 are no longer supported, please
update to typescript 3.8
@atscott atscott removed state: blocked action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels May 5, 2020
@kara kara added the action: presubmit The PR is in need of a google3 presubmit label May 5, 2020
@atscott

atscott commented May 5, 2020

Copy link
Copy Markdown
Contributor Author

presubmit

@kara kara added the action: merge The PR is ready for merge by the caretaker label May 5, 2020
@atscott atscott removed the action: presubmit The PR is in need of a google3 presubmit label May 5, 2020
@alxhub alxhub closed this in 420c179 May 5, 2020
alxhub pushed a commit that referenced this pull request May 5, 2020
Remove TypeScript 3.6 and 3.7 support from Angular along with tests that
ensure those TS versions work.

BREAKING CHANGE: typescript 3.6 and 3.7 are no longer supported, please
update to typescript 3.8

PR Close #36329
gkalpak added a commit to gkalpak/ngcc-validation that referenced this pull request May 6, 2020
Support for TypeScript version <3.8 was dropped from Angular in
angular/angular#36329. In order to allow updates to the latest Angular
framework, this commit updates TypeScript to the latest 3.8.x version.
gkalpak added a commit to angular/ngcc-validation that referenced this pull request May 6, 2020
Support for TypeScript version <3.8 was dropped from Angular in
angular/angular#36329. In order to allow updates to the latest Angular
framework, this commit updates TypeScript to the latest 3.8.x version.
@angular-automatic-lock-bot

Copy link
Copy Markdown

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 Jun 5, 2020
profanis pushed a commit to profanis/angular that referenced this pull request Sep 5, 2020
…ular#36329)

Update the typescript version to 3.8 as well as the Angular version to
9.1, which is the one which added TS 3.8 support.

PR Close angular#36329
profanis pushed a commit to profanis/angular that referenced this pull request Sep 5, 2020
Remove TypeScript 3.6 and 3.7 support from Angular along with tests that
ensure those TS versions work.

BREAKING CHANGE: typescript 3.6 and 3.7 are no longer supported, please
update to typescript 3.8

PR Close angular#36329
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: build & ci Related the build and CI infrastructure of the project breaking changes cla: yes target: major This PR is targeted for the next major release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants