woocommerce / woocommerce

A customizable, open-source ecommerce platform built on WordPress. Build any commerce solution you can imagine.
https://woocommerce.com
9.24k stars 10.73k forks source link

Warn about TS errors and transpile packages that rely on it for compilation #45535

Open samueljseay opened 4 months ago

samueljseay commented 4 months ago

Right now we have a critical issue trying to update Gutenberg dependencies across the monorepo. A slight change in a dependency causes chains of TS errors, and makes it impossible to upgrade anything.

In the past we discussed turning off tsc and I'd like to reintroduce that idea here with some specific steps to take.We're not removing TS from the codebase, I'll just start there to ensure no one worries about that. We also have a goal of getting back to type-correctness so we should be able, some time in the future, to re-enable tsc compilation.

The ideal situation to reduce friction is to use tsc to typecheck packages, emit errors as warnings but not block building the code base.

There are 3 kinds of situations with TS in the codebase right now:

  1. The js packages, they are compiled with tsc so ts is typechecking them and transpiling them to JavaScript
  2. TS is used within a webpack build to provide type checking, I think in most cases this does block the build if TS errors are encountered
  3. There is one other case, the blocks TypeScript where we do TS type checking but we don't block building to JS if it fails

1 is the top priority to fix, because most of the TS errors that we deal with when trying to update dependencies come out of these packages.

To solve 1, I propose we convert the tsc compilation we do now to a 2 step process:

  1. Run tsc with noEmit, swallow error codes but emit all errors as warnings
  2. After that we run esbuild or babel over the code to transpile it to the same formats we support now. i had been thinking esbuild would be much faster, but it may be safer to use babel for now because we're familiar with it and already have presets and such in place that we could use.

To solve 2, firstly I would like to see if 1 is enough to allow us to update the majority of packages but if its not, we should adjust the TS / Webpack setup to just emit type errors but not block building the JS. We can use the approach suggested here: https://github.com/TypeStrong/ts-loader?tab=readme-ov-file#faster-builds which I've used in the past and it works fine.

The next thing we need to solve after that is to track introduced TS errors and report on them in CI. We should consider whether we have some kind of threshold for what's considered a failure (ie introduced x or x% new errors) and block merging when that happens. I think though that the baseline of what errors exist should be established after we update WP packages to latest and also remove any dependency on @types packages for WP.

samueljseay commented 2 months ago

I think I was naive when I wrote the suggested approach, because all the library packages will still need to generate type defs.

Updated suggestion of approach here: https://github.com/woocommerce/woocommerce/pull/47486/files#r1600825869