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

Bloomberg TS5.5-beta feedback [part two] #58587

Open
1 task done
acutmore opened this issue May 20, 2024 · 1 comment
Open
1 task done

Bloomberg TS5.5-beta feedback [part two] #58587

acutmore opened this issue May 20, 2024 · 1 comment
Labels
Discussion Issues which may not have code impact

Comments

@acutmore
Copy link
Contributor

Acknowledgement

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

Comment

I was delayed starting my half of Bloomberg's TypeScript beta analysis. Part one here: #58523


We are in the process of evaluating the impact of TS 5.5 beta on Bloomberg code. Below are preliminary findings.

Overall this looks to be a very positive release.

Change Affects Release notes Packages affected
Cleaner .d.ts emit Declaration Yes Over 20%
Inferred type predicates Type Checking Yes 4
Control flow narrowing for constant indexed access Type Checking Yes 1
Stricter module interpretation Type Checking & Emit Yes 8
noCheck builds

Cleaner `.d.ts` emit

https://devblogs.microsoft.com/typescript/announcing-typescript-5-5-beta/#isolated-declarations

Over 10% of our packages stopped erroring compared to using TypeScript 5.4 due to now emitting .d.ts that pass our post-build declaration validation steps; a primary check is that the .d.ts are only using valid imports. This is most likely due to the reduction of inferred dynamic type imports and instead using imports from the original source:

+ import type { Foo } from "./local-file";
- export declare const thing: (arg: Utility<import("../path/to/something").Value>) => void;
+ export declare const thing: (arg: Foo) => void;

We also noticed cases where jsdoc started to be preserved in the .d.ts emit, which was great.

 declare const Vec3d: {
     create: {
+        /**
+         * Useful documentation here
+         */
         (): Vec3d;
     }

Inferred type predicates

https://devblogs.microsoft.com/typescript/announcing-typescript-5-5-beta/#inferred-type-predicates

Four packages started to error due to inferred type predicates. The code changes required to fix these errors were straight forward.

Control flow narrowing for constant indexed access

https://devblogs.microsoft.com/typescript/announcing-typescript-5-5-beta/#control-flow-narrowing-for-constant-indexed-accesses

Only one package started to error due to this change. The code changes required to fix the error was straight forward.

Stricter module interpretation

We've had a mixture of issues here. For context on the projects we saw errors: while there is a package.json in a parent directory, usually above the tsconfig.json directory, this is not for the TypeScript project. The package.json is there for the repo-wide tooling (e.g. eslint). The TypeScript project within the repo is not a Node package.

For the teams that have specified "type": "module" in this package.json we started to get build errors for projects that were relying on some of the previously allowed CJS compatibility.

Two projects were using export = in a .d.ts file and started to get:

error TS1203: Export assignment cannot be used when targeting ECMAScript modules

Four projects were incorrectly using the synthetic default export:

error TS1192: Module has no default export.

One project had "type": "commonjs" in their package.json which meant that it started to emit CJS, even though the tsconfig.json 'target' was set to esnext. To fix this we will likely need to update the repo to remove the "type": "commonjs".

`noCheck` builds

While not publicly exposed I used a locally patched version of typescript.js to analyze the potential impact of noCheck: false on ts.transpileModule and #50699.
The results were very promising, a 4 second build dropped down to 2.2 seconds.

@DanielRosenwasser DanielRosenwasser added the Discussion Issues which may not have code impact label May 24, 2024
@DanielRosenwasser
Copy link
Member

As always, thank you for these updates!

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

2 participants