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 TS 5.8-beta #61116

Open
1 task done
shicks opened this issue Feb 4, 2025 · 1 comment
Open
1 task done

Google feedback on TS 5.8-beta #61116

shicks opened this issue Feb 4, 2025 · 1 comment
Labels
Discussion Issues which may not have code impact

Comments

@shicks
Copy link
Contributor

shicks commented Feb 4, 2025

Acknowledgement

  • I acknowledge that issues using this template may be closed without further explanation at the maintainer's discretion.

Comment

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

Executive summary

  • We do not expect to have significant difficulty in upgrading Google to TS 5.8.
  • Some changes to our TypeScript code are required to fix compilation.
    • Most of the new errors are clearly related to announced changes.
    • Detail sections below explain the changes we expect to make to unblock the upgrade.

Impact summary

Change description Announced Libraries affected
Conditional expression checking Yes ~0.007%
lib.d.ts Changes Yes ~0.005%

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

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

Announced Changes

This section reviews all announced changes from the TS5.8-beta announcement, whether we support these changes at Google, how we will resolve pre-existing issues for these changes (if applicable), and other thoughts.

Checked Returns for Conditional and Indexed Access Types

We support this change. It improves ergonomics and type checking accuracy and uncovers real unintentional runtime behavior.

The PR mentions a breaking change to conditional expression checking, which might be worth calling out explicitly in the release notes. We found that this caused errors in ~0.007% of our libraries, generally due to one branch being typed as any, causing the other branch to now be checked.

Most of the errors caused this way were legitimate, though some were arguably spurious (e.g. this uncovered an instance of #17002), while a few sat somewhere in between. We will work through most of these by just suppressing the error with // @ts-ignore and a note advising owners to revisit their code and fix the underlying problems.

These were essentially all instances of TS2322.

The --erasableSyntaxOnly Option

We support this change, though we will not be able to take advantage of it any time soon. In particular, we're interested in seeing something like #60790, which actually aligns quite well with the Closure-style enums that are very common in our codebase.

Preserved Computed Property Names in Declaration Files

We support this change, and will leverage it immediately to better handle symbol properties under isolated declarations.

lib.d.ts changes

We support these changes.

The changed declarations cause a few problems we will need to work around. In particular, the change to the Window.location setter broke ~0.004% of our targets, primarily tests mocking out the Location object with a FakeLocation. We will silence these with // @ts-ignore. Additionally, a handful of new declarations conflict with older versions in DefinitelyTyped and we'll need to adjust these dependencies.

Other changes

The following changes are largely irrelevant in our codebase and we don't have anything to add about them:

  • Support for require() of ECMAScript Modules in --module nodenext
  • --module node18
  • The --libReplacement Flag
  • Optimizations on Program Loads and Updates
  • Restrictions on Import Assertions Under --module nodenext

Unannounced Changes

As mentioned above, the change in type checking of conditional expressions is a notable breaking change. While it was technically announced via details in the PR linked in the announcement, it's easy to miss and could stand to be called out more explicitly.

@RyanCavanaugh RyanCavanaugh added the Discussion Issues which may not have code impact label Feb 4, 2025
@jakebailey
Copy link
Member

Preserved Computed Property Names in Declaration Files

We support this change, and will leverage it immediately to better handle symbol properties under isolated declarations.

Do note that these are still not legal under isolatedDeclarations, pending changes to that flag and its implementation.

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