Refactor TypeScript Build Configuration

[ObliviousHarmony]

Prerequisites (mark completed items with an [x]):

• I have checked that my issue type is not listed here https://github.com/woocommerce/woocommerce/issues/new/choose
• My issue is not a security issue, support request, bug report, enhancement or feature request (Please use the link above if it is).

Issue Description:

In order to make our TypeScript configuration a bit easier to maintain, we should clean up and centralize our configuration. While this will ultimately have an impact on the "target" for various projects, they should still all remain compatible given our supported Node version. The goal of this task is to make appropriate use of "extends" and centralize common configuration where possible.

[ObliviousHarmony]

While working on this it became pretty clear that using a base config comes with some caveats. First I investigated creating a new @woocommerce/internal-config package that could contain common config intended to be shared in the monorepo. This felt like a good approach, however, it's important to note how "extends" works with TSConfig files.

Any relative paths are relative to the source TSConfig file. This means that a reference like 'outDir': 'build' in packages/js/internal-config/tsconfig.base.json would reference packages/js/internal-config/build/. Even when the file is being referenced by "extends" this remains true, so, each file would still need to declare similar configuration.

My solution to this was to have a tsconfig.base.json file that is copied to all of the project directories via a script. My plan was:

• Create @woocommerce/internal-monorepo-config package in packages/js/ that contains a common TSConfig. We can also look at putting other useful configurations in here like ESLint and the like. This will be good because it lets us remove the root-level configuration files that might imply that you use these tools from the root.
• Add a command to our monorepo tool
  • Read the pnpm-workspace.yaml file and use the globs to find package.json files.
  • Read the package.json files for typescript references.
  • For those that have TypeScript support, copy the tsconfig.base.json file from @woocommerce/internal-monorepo-config.
• Add tsconfig.base.json files to .gitignore with exceptions where appropriate.
• We might need to look at E2E packages and make sure that they don't have any configuration this might squash that's shipped to downstream consumers.

The end result would be that we could have really robust base configuration featuring all of the relevant paths without worrying about maintenance burdens. The script could be ran on pnpm install and so nobody would ever be in a non-functional state.

[ObliviousHarmony]

I also developed the following base tsconfig.json file:

{
	"compilerOptions": {
		/* Standardize ES2019 and ESM Usage */
		"target": "es2019",
		"module": "esnext",
		"moduleResolution": "node",

		/* Configured Type Checks */
		"strict": true,
		"skipLibCheck": true,
		"noUnusedLocals": true,
		"noUnusedParameters": true,
		"noImplicitReturns": true,
		"noFallthroughCasesInSwitch": true,

		/* Support JavaScript Transpilation */
		"allowJs": true,

		/* @wordpress/element Provides Our JSX Runtime */
		"jsx": "react",
		"jsxFactory": "createElement",
		"jsxFragmentFactory": "Fragment",

		/* Generate Types */
		"declaration": true,
		"declarationMap": true,

		/* Support Single File Transpilation */
		"isolatedModules": true,

		/* Support JSON File Imports */
		"resolveJsonModule": true,

		/* Require Isolated Package Roots */
		"typeRoots": []
	},
	"include": [
		"**/src/**/*"
	],
	"exclude": [
		"**/test/*",
		"**/*.spec.*",
		"**/*.test.*"
	]
}

The "include" and "exclude" might need to be adjusted for compatibility, but, the idea was that all projects should be consistent in the first place.

[samueljseay]

For my own reference just linking the issue discussing tsconfig behaviour here because I found it very surprising 😂 microsoft/TypeScript#29172

[samueljseay]

@ObliviousHarmony took your idea and added my own spin, keen to hear your thoughts: #37875

[samueljseay]

I don't agree with the approach suggested here:

• postinstall steps that copy paste tsconfigs is worse imo and easier to get into a broken dev state (imo) or a state where git clean accidentally messed up a user environment. postinstall is actually a PITA in a monorepo because it can stall a pnpm i when something goes wrong and that halts productivity when perhaps the issue is not related to a package you're working in.
• I don’t think that the downsides of my approach are that relevant, more based on opinion about what solution is cleanest.
• I don’t care to argue over it, simply I am not happy with introducing the alternate solution to the mono repo, it feels like low value, for the added complexity since the existing TS configs are already shared and working fine as they are.

[ObliviousHarmony]

Thanks for your feedback @samueljseay!

This issue mostly came about when I started poking at removing CJS and ESM builds where they aren't necessary. Our TypeScript configuration ended up being a bit of an inconsistent mess, and as a result, it wasn't as easy to make those changes as it felt like it should be.

The primary goal of this issue was to settle on a single consistent base configuration for all TypeScript projects. Right now we have "target"s of es6, es2019, es5, and esnext scattered about. We have different compiler options enabled in different places for, as far as I can tell, absolutely no reason. When I started making changes here I found that every tsconfig.json file in the monorepo was essentially identical, and in cases where a mistake was made and it wasn't, it broke in odd ways in different environments.

Regardless of whether or not we do anything with the duplication caused by the tsconfig.base.json path resolution, what do you think of that specific point?

[samueljseay]

@ObliviousHarmony yes fair points!

and in cases where a mistake was made and it wasn't, it broke in odd ways in different environments.

Is it possible you could you elaborate on this one? I just wasn't aware there were situations where it was broken 😃

[ObliviousHarmony]

Yes @samueljseay, this instance ended up causing some trouble during the contributor day. The typeRoots are zeroed out in the packages/js/tsconfig.json file to try and avoid this kind of mistake.

While most (not all, actually!) of the packages inherit from packages/js/tsconfig.json, the plugins, tools, and actually a few packages (@woocommerce/api for instance) don't. Since typeRoots use relative paths and need to be copied, it's really easy to make a mistake that won't break in every environment and isn't easily debugged by people who aren't already familiar with the potential problem.

To be honest, I don't particularly care about solving the relative path problem, but, we should come up with a single base tsconfig.json file for every project in the repository at least. With respect to that, I see two paths:

• Roottsconfig.base.json File: I don't like this one much since it breaks the isolation principle of our packages. Up to this point, each package can more or less be copied out of the monorepo and just work. There's some problems, of course, but generally, this is true.
• @woocommerce/tsconfig Package: This is a bit nicer. We can encapsulate the typescript dependency and tsconfig.base.json file which can then be extended in each package's tsconfig.json file.

Honestly, you might be right about the value of a @woocommerce/internal-build package that encapsulates some of this. We have a @woocommerce/internal-style-build package already, so, maybe we should bring together this common build functionality? There's style builds, bundler configuration, and TypeScript builds all floating around with mild inconsistencies that don't necessarily need to exist. This would solve version differences with build tools.

With that said, going that far might still be unnecessary.

[samueljseay]

Thanks @ObliviousHarmony

Maybe internal-build would be worth introducing at some point, but I think I agree that at least for now it's too far.

@woocommerce/tsconfig Package: This is a bit nicer

Yeah maybe, for the relative path problem we could just add the paths in each package that extends the base config for now? We can leave all path related options out of the shared config and just set them in the project's consuming the package? Its a bit repetitive, but, things I like:

• no postinstall
• less complexity
• less hiding of ✨ magic ✨ behaviours

[ObliviousHarmony]

Yep @samueljseay,

I'm only talking about the shared configuration. The empty typeRoots will catch breakage there if it is extending properly. We can leave all of the relative paths in the children and revisit the rest later if it's ever a concern.

[samueljseay]

@ObliviousHarmony sounds great, thanks for the discussion to come to a solution there 🎉