Optimize how TypeScript-authored packages are built

[jsnajdr]

This PR solves two problems:

1. I noticed that the Search and Language Picker packages are built with a combination of TypeScript Compiler and Babel: the types are generated with tsc, but the JS files are generated by Babel (by virtue of the transpile script from calypso-build). That looks inefficient: why not build both with tsc, in one step? Follows up to a discussion with @saramarcondes in Extract the search component into @automattic/search #46424 (comment)

2. To build Calypso The App, we only need the files in dist/esm. Building the dist/cjs folder is needed only when publishing a package to NPM and the time and CPU to build it is wasted in all other cases. This PR follows the lead in Do not transpile JS packages twice #44824 and builds only the dist/esm folder in the prepare step. dist/cjs is done only in the prepack step, during NPM publishing.

To produce only the dist/esm or dist/cjs folders, I had to patch the transpile and copy-assets scripts in calypso-build. They now accept --esm and --cjs options to override the default that processes both module types.

Booting up the TypeScript compiler and compiling a handful of files takes ~3.5s on my machine, which is a lot. These changes should speed up the build-packages step significantly.

If this PR goes through, there are still plenty of other packages that should be optimized the same way. For example, the wpcom.js package's prepare script builds CJS, ESM and also a webpack library bundle intended to be loaded as <script> tag. Only the ESM build is needed locally.

How to test:
Verify that prepare-ing the @automattic/search and @automattic/language-picker produces a correct dist/esm folder with everything that should be there (types, compiled JS files, copied SCSS files) and that the dist/cjs folder is created only
on yarn run prepack.

[matticbot]

Test live: https://calypso.live/?branch=optimize/ts-packages-build

[matticbot]

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

App Entrypoints (~47 bytes removed 📉 [gzipped])

name                 parsed_size           gzip_size
entry-gutenboarding        -87 B  (-0.0%)      -47 B  (-0.0%)

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

[sirreal]

For the packages that have transitioned from the transpile command to use tsc, we'll want to change the tsconfig. They were taken from packages that transpile the code and generate the types as separate processes. "Just generate the types" has different concerns and isn't interested in the code that tsc skips generating.

We should use a tsconfig that is intended for generating code and not just types. I believe the data-stores tsconfig is the most up-to-date example and can likely be copied.

Notably, the target changes which results in different output:
https://github.com/Automattic/wp-calypso/blob/0dcfe7ae0122cf4819c5c0e0ff5ea311657bf2b9/packages/search/tsconfig.json#L3

wp-calypso/packages/data-stores/tsconfig.json

Line 3 in d2613b4

"target": "ES5",

// Input
const a = { a: 'a' }, b = { b: 'b' }, c = { c: 'c' }
const abc = { ...a, ...b, ...c }

// target: es2018 
const a = { a: 'a' }, b = { b: 'b' }, c = { c: 'c' };
const abc = { ...a, ...b, ...c };

// target: es5
import { __assign } from "tslib";
var a = { a: 'a' }, b = { b: 'b' }, c = { c: 'c' };
var abc = __assign(__assign(__assign({}, a), b), c);

That's not expected at the moment, our published packages are intended to be published with ES5 syntax (with versions for esmodules or commonjs modules).

For Calypso as a consumer, if we transpile these after the fact then we can compile to modern syntax + type definitions which will likely give us the best output, but that would need another configuration on top of the existing es5 targets.

We'll also need to add tslib as a dependency of the package if we use that configuration.

---

☝️ This bit is important and should be resolved
👇 I'll continue to share ideas but they aren't critical for this PR and might take more work

---

Improvements

The TypeScript setup in Calypso isn't ideal. I don't have a lot of time or focus to dedicate to it right now but I will help with reviews and guidance if someone would like to take on the task.

Better config sharing

Common tsconfig should be centralized and extended. Don't repeat and allow for confusing examples that may be copied from more or less ideal sources. tsc-compiled and type-only configurations would likely be lifted to top-level and extended.

Smarter building

This PR touches on the redundancy of cjs builds for Calypso. It's an improvement, but the following suggests further improvement. For Calypso where cjs is redundant, we should make Calypso client a composite typescript project. This is achieved by adding an of the tsc-compiled packages it requires, e.g.:

// client/tsconfig.build.json
{
  // Don't actually compile anything from Calypso
  "files": [],
  // Calypso requires all of these packages to be built
  "references": [
    { "path": "../packages/data-stores" },
    { "path": "../packages/search" },
    // … add all the tsc-built monorepo dependencies
  ]
}

All of the tsc-built packages we have could replace their prepare scripts with prepack and skip work on initial boot. The Calypso build/dev scripts would need to run tsc --build client/tsconfig.build.json and we'd get watch mode trivially with tsc --build client/tsconfig.build.json --watch.

[jsnajdr]

@sirreal I improved the transpilation config in 46cb58d

I used the data-stores config, with a few exception that I think should be backported to data-stores, too:

• I didn't include ScriptHost in the lib array. It's a native Windows API for Windows Script Host, nothing to do with browsers or Node.js, so why does data-stores use it?
• instead of ES2015 in lib, I put ES2020 there. With ES2015, the compiler didn't let me use Array.prototype.includes. That method was added only in 2016.
• specifying both noEmitHelpers: true and importHelpers: true is not necessary. importHelpers is sufficient to add imports from tslib instead of inlining the helpers.

This PR touches on the redundancy of cjs builds for Calypso. It's an improvement, but the following suggests further improvement. For Calypso where cjs is redundant, we should make Calypso client a composite typescript project.

I believe this is blocked by the conflict over main field's value. To make the composite project work, the main field would need to point to src/index.ts so that imports from a package work. But main field also needs to be dist/cjs/index.js, to make the published NPM package work in CommonJS mode.

For JS packages consumed only by webpack or Node.js, not by tsc, we solved that in #44824 by adding the calypso:src field and using it as the source of untranspiled code. But tsc can't be easily configured to use a custom main field.

This PR touches on the redundancy of cjs builds for Calypso.

I would be even more radical here and say that the cjs build is redundant. 🙂 I don't think there's even a single consumer that would use the published CJS build. It's all ESM modules consumed by webpack.

Ideally, we would publish untranspiled sources (ES2020 + JSX, or TypeScript) and let the consumer do the transpilations they need. I believe all consumers are already easily configured for that, we'll avoid complexity related to the builds, and the end users will get bundles optimized for their platform without unneccessary ES5.

[sirreal]

@sirreal I improved the transpilation config in 46cb58d

👍 I scanned the configs and they look good.

I used the data-stores config, with a few exception that I think should be backported to data-stores, too:

• I didn't include ScriptHost in the lib array. It's a native Windows API for Windows Script Host, nothing to do with browsers or Node.js, so why does data-stores use it?

That was likely because the ES5 target implied a lib value of [ "ES5", "DOM", "ScriptHost" ]. I'm fine with dropping it as you've done and backporting.

• instead of ES2015 in lib, I put ES2020 there. With ES2015, the compiler didn't let me use Array.prototype.includes. That method was added only in 2016.

If we're expecting consumers to polyfill anyways, should it just be esnext? Reference here.

• specifying both noEmitHelpers: true and importHelpers: true is not necessary. importHelpers is sufficient to add imports from tslib instead of inlining the helpers.

👍

This PR touches on the redundancy of cjs builds for Calypso. It's an improvement, but the following suggests further improvement. For Calypso where cjs is redundant, we should make Calypso client a composite typescript project.

I believe this is blocked by the conflict over main field's value. To make the composite project work, the main field would need to point to src/index.ts so that imports from a package work. But main field also needs to be dist/cjs/index.js, to make the published NPM package work in CommonJS mode.

For JS packages consumed only by webpack or Node.js, not by tsc, we solved that in #44824 by adding the calypso:src field and using it as the source of untranspiled code. But tsc can't be easily configured to use a custom main field.

🤔 Let's back up and forget references and composite projects for a moment. The Calypso webpack build is already configured to consume TypeScript source files. None of the Calypso client is actually built by tsc. There's no true tsc consumer, it's all webpack and it should tolerate TypeScript source if we add calypso:src fields to the TypeScript packages just like was done in #44824. Is there a reason we can't do that for these packages and stop "pre-compiling" TypeScript for Calypso to use?

This PR touches on the redundancy of cjs builds for Calypso.

I would be even more radical here and say that the cjs build is redundant. 🙂 I don't think there's even a single consumer that would use the published CJS build. It's all ESM modules consumed by webpack.

Ideally, we would publish untranspiled sources (ES2020 + JSX, or TypeScript) and let the consumer do the transpilations they need. I believe all consumers are already easily configured for that, we'll avoid complexity related to the builds, and the end users will get bundles optimized for their platform without unneccessary ES5.

I haven't thought much about this and don't feel strongly either way. I'd say it's material we can leave for another PR 🙂

[jsnajdr]

If we're expecting consumers to polyfill anyways, should it just be esnext?

Good point, I changed the lib to ESNext in cb36622. By the time the conservative TypeScript compiler adds support for the latest ES standard, the Babel and core-js environments usually have supported it for a very long time.

Let's back up and forget references and composite projects for a moment. The Calypso webpack build is already configured to consume TypeScript source files. None of the Calypso client is actually built by tsc.

That's true. Where tsc really failed me when trying to introduce calypso:src in #43679 was not the webpack build, but the tsc invocations in the typecheck task: #43679 (comment)

And it was the types field, pointing to dist/types, that failed to resolve: when there is no build, the dist/types are not generated.

That fails the typecheck task, and likely also any package's tsc build if it references another package that hasn't been built yet.

But I admit I'm already pretty confused here and what I write can be wrong. I'll need to repeat the experiments I did 4 months ago to get new results and to refresh my memory 🙂

[sirreal]

That's true. Where tsc really failed me when trying to introduce calypso:src in #43679 was not the webpack build, but the tsc invocations in the typecheck task: #43679 (comment)

And it was the types field, pointing to dist/types, that failed to resolve: when there is no build, the dist/types are not generated.

That fails the typecheck task, and likely also any package's tsc build if it references another package that hasn't been built yet.

We removed typecheck completely I believe. typecheck-strict was still relevant but I'm not sure whether it's still run. I don't know where to find it in teamcity.

These checks and editor integrations are where project references and the composite project would be beneficial. They allow tsc to build TypeScript dependencies in order and disallow circular dependencies. TypeScript doesn't need to build or bundle JavaScript in order typecheck and be informed by type declarations which it will be able to find (and build on-demand) if we provide project references correctly.

[sirreal]

I tested the instructions with the search and language-picker packages and this works.

We've discussed some future improvements that would be good avenues to continue this work, but this is a good incremental step.

Thanks!

[scinos]

Looks like this was autofixed by prettier wrong, those should be *.md.js

[jsnajdr]

Nice catch, fixing in #47179.