build: typescript 3.7 support#33717
build: typescript 3.7 support#33717andrius-pra wants to merge 14 commits intoangular:masterfrom andrius-pra:typescript-3.7
Conversation
|
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
There was a problem hiding this comment.
Fixes error in this line line
asyncPromises.push(callbackResult);
~~~~~~~~~~~~~~
Argument of type 'void' is not assignable to parameter of type 'Promise<any>'.
|
@filipesilva, I have added you as co-author because of this PR contains changes from #32962. |
There was a problem hiding this comment.
The typeArguments property has been removed from the TypeReference interface in Typescipt 3.7. we should use the getTypeArguments function on TypeChecker instances. See #32962 (comment)
gkalpak
left a comment
gkalpak
left a comment
There was a problem hiding this comment.
BTW, I can see several more .typeArguments accesses through-out the codebase.
Shouldn't all these occurrences be converted to use checker.getTypeArguments() if available?
|
|
|
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) ℹ️ Googlers: Go here for more info. |
|
hi @andrius-pra, thanks for working on this PR - I just want to set the expectations right that we won't be merging this PR until we work on 9.1. I estimate that we will come back to this PR and be able to merge it in December, but unlikely sooner than that. I'm grateful for your help, and we could use this PR in the future to get typescript 3.7 support into 9.1, but that will require cooperation with the CLI team and extensive testing that we no longer have time in 9.0 release cycle. |
|
@IgorMinar I wanted to chime in and say I think that message you wrote to @andrius-pra was very kind and professional! 😊 In both how it mentioned release timings / priorities / logistics but also gratitude for the contribution and time put into Andrius work. This is a big project with a lot of moving parts, and being graceful like that was a nice thing to see. Look forward to seeing TypeScript 3.7 support in Angular! |
|
@IgorMinar no 3.7 in 9.0 then? |
With TS 3.7, these examples were running into the error below (e.g. on https://circleci.com/gh/angular/angular/574906#tests/containers/0): ``` ============== AIO example output for: /home/circleci/ng/aio/content/examples/observables/ running: yarn tsc --project ./ $ /home/circleci/ng/aio/content/examples/observables/node_modules/.bin/tsc --project ./ ../../../tools/examples/shared/node_modules/protractor/built/index.d.ts(5,10): error TS2440: Import declaration conflicts with local declaration of 'PluginConfig'. ../../../tools/examples/shared/node_modules/protractor/built/index.d.ts(5,24): error TS2440: Import declaration conflicts with local declaration of 'ProtractorPlugin'. error Command failed with exit code 2. info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command. completed: yarn tsc --project ./ ``` This happened because of angular/protractor#5348. It's unclear why this typings problem does not affect `ng e2e` runs, and only affects `tsc` runs. For now it seems sensible to alter the tests to compile only the app and not the e2e, since the intent of 2cc954d was never to verify the correctness of the e2e in the first place. We still need a release of protractor that supports TS 3.7 though, but at least it doesn't seem to block our update proper. PR Close #33717
Updates the commit SHA the `material-unit-tests` CircleCI job runs against. We need to include a commit that makes the node module installation more determinisitc (i.e. ensuring that `tsickle` is always hoisted at the node module root). angular/components@31a50f7 PR Close #33717
The components repository has a Yarn resolution to ensure that dgeni-packages uses a specific TypeScript version. This resolution causes the specified TS version to be considered as candidate for the `@angular/bazel` peer dependency. Ultimately, Yarn decides to use the TypeScript version from the resolution for `@angular/bazel`, and builds will fail due to a version mismatch. This is because `tsickle` will use the hoisted top-level TS version (set to `3.7.4` ), while `@angular/bazel` uses the version from the resolution (at the time of writing: v3.6.4) PR Close #33717
Just to be consistent. PR Close #33717
This PR updates TypeScript version to 3.7 while retaining compatibility with TS3.6. PR Close #33717
This is the tsickle version that supports TypeScript 3.7. PR Close #33717
With TS 3.7, these examples were running into the error below (e.g. on https://circleci.com/gh/angular/angular/574906#tests/containers/0): ``` ============== AIO example output for: /home/circleci/ng/aio/content/examples/observables/ running: yarn tsc --project ./ $ /home/circleci/ng/aio/content/examples/observables/node_modules/.bin/tsc --project ./ ../../../tools/examples/shared/node_modules/protractor/built/index.d.ts(5,10): error TS2440: Import declaration conflicts with local declaration of 'PluginConfig'. ../../../tools/examples/shared/node_modules/protractor/built/index.d.ts(5,24): error TS2440: Import declaration conflicts with local declaration of 'ProtractorPlugin'. error Command failed with exit code 2. info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command. completed: yarn tsc --project ./ ``` This happened because of angular/protractor#5348. It's unclear why this typings problem does not affect `ng e2e` runs, and only affects `tsc` runs. For now it seems sensible to alter the tests to compile only the app and not the e2e, since the intent of 2cc954d was never to verify the correctness of the e2e in the first place. We still need a release of protractor that supports TS 3.7 though, but at least it doesn't seem to block our update proper. PR Close #33717
Updates the commit SHA the `material-unit-tests` CircleCI job runs against. We need to include a commit that makes the node module installation more determinisitc (i.e. ensuring that `tsickle` is always hoisted at the node module root). angular/components@31a50f7 PR Close #33717
The components repository has a Yarn resolution to ensure that dgeni-packages uses a specific TypeScript version. This resolution causes the specified TS version to be considered as candidate for the `@angular/bazel` peer dependency. Ultimately, Yarn decides to use the TypeScript version from the resolution for `@angular/bazel`, and builds will fail due to a version mismatch. This is because `tsickle` will use the hoisted top-level TS version (set to `3.7.4` ), while `@angular/bazel` uses the version from the resolution (at the time of writing: v3.6.4) PR Close #33717
Just to be consistent. PR Close #33717
angular/angular#33717 upgraded Angular monorepo to TypeScript 3.7.4, so we could now upgrade to ts 3.7 This would fix the error with "no config file found for *.html" if no TypeScript file is open. PR closes angular#546
angular/angular#33717 upgraded Angular monorepo to TypeScript 3.7.4, so we could now upgrade to ts 3.7 This would fix the error with "no config file found for *.html" if no TypeScript file is open. PR closes #546
| count: number; | ||
| readonly even: boolean; | ||
| readonly first: boolean; | ||
| get even(): boolean; |
There was a problem hiding this comment.
Why did you switch from readonly to get?
- curious angular developer
There was a problem hiding this comment.
We didn't, at least intentionally.
These .d.ts files in tools/public_api_guard/common are snapshots of the types for the Angular API created by ts-api-guardian. The purpose of that tools is to tell us when the public TS api changed. This is useful to detect cases where we did some code change that accidentally changed the API, in which case we revert the change.
In this case however, TS started emitting different types so we had to update the snapshots. TS does not follow semver and can be expected to emit different type output between minors, so it is ok to update the snapshots.
|
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. |
12 participants
This PR updates TypeScript version to 3.7 while retaining compatibility with TS3.6.
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #33596
What is the new behavior?
Does this PR introduce a breaking change?
Other information