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

Google Feedback on TypeScript 5.0-beta #52913

Closed
h-joo opened this issue Feb 22, 2023 · 2 comments
Closed

Google Feedback on TypeScript 5.0-beta #52913

h-joo opened this issue Feb 22, 2023 · 2 comments
Labels
Discussion Issues which may not have code impact

Comments

@h-joo
Copy link
Contributor

h-joo commented Feb 22, 2023

This GitHub issue contains feedback on the TS 5.0-beta release from the team that is responsible for keeping Google's internal software working with the latest version of TypeScript.

Executive summary

  • Small changes to our TypeScript code are required to make it compile with TS 5.0.
  • Detail sections below explain the changes to our code we expect to make to unblock the upgrade.

Impact summary

Change description Announced Libraries affected
All enums are union enums yes 0.056%
Forbidden Implicit Coercions in Relational Operators yes 0.037%
lib/d.ts changes yes 0.011%
Type inference for declared enum member assignment no 0.004%
Others - 0.004%

The Announced column indicates whether we were able to connect the observed change with a section in the TS5.0-beta announcement.

The following sections give more detailed explanations of the changes listed above.

Changes which were announced

All enums are union enums

This was part of the release announcement, and the majority of this failure was coming from test codes that was passing in ill-typed value and was caught by the new change.

We expect to add // @ts-ignore to get around with this issue.

Forbidden Implicit Coercions in Relational Operators

This was part of the release announcement, and uncovered many code that were doing implicit coercions.

To be safe to have no implication on the runtime, we expect to ts-ignore this warning for existing code.

lib/d.ts changes

We support the typing improvements. Generally it's clear what's changing and the changes seems minimal this time. The majority of the change is coming from the change in TSC API, that modifier property is now gone from Node type.

For the TSC changes, we expect to fix our codebase to comply to the new API and for other typing changes, we expect to // @ts-ignore to silence the errors.

Type inference for declared enum member assignment

We saw the same issue that was reported in this bug report, but it already seemed to be fixed.

Others

There was a runtime failure in our internal tooling caused by the change in the behaviour of ts.getJSDocTags. Before, it was blindly traversing up in the AST to find a valid JSDoc, but now it first checks whether a node can have a JSDoc,
otherwise returns an empty array.

This is the change that caused our failure, and we have a fix which now queries JSDoc only on specific nodes.

There were other small failures that all looked reasonable such as disallowing ill-formed type annotation (using '?' or '*') in catch statements. (playground in 4.9, playground in 5.0-beta)

We also noticed some errors around inappropriately narrowing types to never in case of instanceof operations on Function types, but was also fixed

@RyanCavanaugh RyanCavanaugh added the Discussion Issues which may not have code impact label Feb 22, 2023
@DanielRosenwasser
Copy link
Member

Thanks for following up here! It sounds like these are mostly reasonable. What sorts of things on our API are you planning to // @ts-ignore to get through?

@h-joo
Copy link
Contributor Author

h-joo commented Feb 23, 2023

I apologize for making it a bit confusing. I mixed up the d.ts changes as a whole(regular typing changes which involves lib.d.ts changes + typescript.d.ts changes caused by API changes in the TypeScript compiler itself) so it was a bit confusing when I read it again. We don't intend to ts-ignore any TypeScript Compiler API changes (in our TypeScript tooling) as it'll most likely cause runtime failures on the compiler side, but we only will ts-ignore the build failures coming from lib.d.ts changes and depend on the code owners of our codebase to fix for themselves correctly with accordance to the new type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion Issues which may not have code impact
Projects
None yet
Development

No branches or pull requests

3 participants