Build: Composite TypeScript setup #39786

sirreal · 2020-02-29T22:31:05Z

Changes proposed in this Pull Request

Fix client typechecking
Update gutenboarding typechecking to extend client
Setup "monorepo" composite packages

Testing instructions

Verify the typecheck jobs are working well on CI.

Closes #39659

matticbot · 2020-02-29T22:31:08Z

Test live: https://calypso.live/?branch=update/composite-typescript-setup

matticbot · 2020-02-29T23:08:23Z

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

ockham · 2020-03-01T15:51:37Z

package.json

@@ -130,7 +130,7 @@
 		"test-server:coverage": "npm run -s test-server -- --coverage",
 		"test-server:watch": "npm run -s test-server -- --watch",
 		"translate": "i18n-calypso --format pot --output-file ./calypso-strings.pot -k translate,__,_x,_n,_nx -e date '**/*.js' '**/*.jsx' '**/*.ts' '**/*.tsx' '!build/**' '!packages/**/dist/**' '!public/**' '!**/test/**' '!node_modules/**' '!apps/full-site-editing/**' '!**/node_modules/**'",
-		"typecheck": "tsc --noEmit",
+		"typecheck": "NODE_OPTIONS='--max-old-space-size=4096' tsc --project client",


This is used in the typecheck-strict check:

wp-calypso/.circleci/config.yml

Line 246 in b2a3ab3

command: npm run typecheck -- --project client/landing/gutenboarding

which thus now reads, expanded:

> NODE_OPTIONS='--max-old-space-size=4096' tsc --project client "--project" "client/landing/gutenboarding"

So we probably need to add a dedicated script to package.json and use it in the CCI config, or solve the problem there.

package.json

roo2 · 2020-03-02T01:21:42Z

client/lib/media/actions.js

@@ -18,7 +18,7 @@ import MediaListStore from './list-store';
 import MediaValidationStore from './validation-store';

 /**
- * @typedef MediaActions
+ * @typedef IMediaActions


Noice! this fixes the tsc error that was crashing tsc locally

Yes, this could land on its own to fix the client. The tsc error message was completely unhelpful and I'd speculated the problem was related to the client reorganization which I intended to address with these fixes, but along the way I found this was the issue 🙂

I believe this is a typings regression introduced in #39057 / #39048

Pulled this into #39868

roo2 · 2020-03-02T01:30:24Z

~~The circleCI Typecheck task raises errors and says it exits with status 1 but the CircleCi job still passes.~~ https://circleci.com/gh/Automattic/wp-calypso/621319?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

Edit: Ahh ok I see that's the intended behavior.

roo2 · 2020-03-02T01:37:42Z

.circleci/config.yml

@@ -243,7 +243,7 @@ jobs:
      - prepare
      - run:
          name: TypeScript strict typecheck of individual subprojects
-          command: npm run typecheck -- --project client/landing/gutenboarding
+          command: npm run tsc -- --project client/landing/gutenboarding


hmm are there other projects/directories that we should add here under typecheck-strict? it's currently just doing gutenboarding

Yeah, that was discussed when I originally added this section. My reasoning was that this enables us to gradually add other strictly-typechecked parts of Calypso -- I'm doing that e.g. in #39373 (which, granted, is just an experiment).

ockham

Not sure I understand all the nuance on this one, but it looks sane overall, and it's great to see the TypeScript check working again.

client/tsconfig.json

sirreal · 2020-03-04T09:06:08Z

.circleci/config.yml

  save-tsc-cache: &save-tsc-cache
    name: Save TypeScript cache
-    key: v{{ .Environment.GLOBAL_CACHE_PREFIX }}-v0-tsc-{{ .Branch }}-{{ .Revision }}
+    key: v{{ .Environment.GLOBAL_CACHE_PREFIX }}-v1-tsc-{{ .Branch }}-{{ .Revision }}
    paths:
      - ~/.tsc-cache


Interesting one here… for composite and incremental builds, I believe TypeScript skips outputting anything if the caches are up to date. We can add packages/*/dist to the cache to catch and tsc-build output.

Suggested change

- ~/.tsc-cache

- ~/.tsc-cache

- packages/*/dist

sirreal · 2020-03-04T09:16:38Z

Rebased

sirreal · 2020-03-04T12:41:38Z

I've rebase and cleaned this up. I'd love another round of reviews!

p-jackson

The CI job is succeeding and the config changes make sense.

A thought about the tsBuildInfoFile names: we're probably not going to have name collisions for gutenboarding and composite-checkout-wpcom, but components and client seem more common.

Should our pattern for choosing tsBuildInfoFile be more path based? e.g. packages-components, client-landing-gutenboarding, etc.?

I thought we could use package name from package.json since that should probably be unique, but not every tsconfig.json has a package.json e.g. gutenboarding

sirreal · 2020-03-06T09:04:42Z

A thought about the tsBuildInfoFile names: we're probably not going to have name collisions for gutenboarding and composite-checkout-wpcom, but components and client seem more common.

It's important to keep in mind that only projects that we're building here using the TypeScript compiler (tsc) will produce these files. We'll never be generating .tsbuildinfo or see them for external projects like packages from Gutenberg.

Should our pattern for choosing tsBuildInfoFile be more path based? e.g. packages-components, client-landing-gutenboarding, etc.?

That's a find idea although I believe we're unlikely to run into issues with collisions unless we blatantly reuse a name. I'll add this change.

sirreal · 2020-03-06T10:44:00Z

Rebased and applied suggestions. With several approvals, I'll plan to move ahead with this soon unless there's any opposition.

sirreal added the [Status] In Progress label Feb 29, 2020

sirreal self-assigned this Feb 29, 2020

sirreal marked this pull request as ready for review March 1, 2020 15:07

sirreal requested review from a team as code owners March 1, 2020 15:07

sirreal requested a review from a team March 1, 2020 15:07

sirreal added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Mar 1, 2020

sirreal requested a review from a team March 1, 2020 15:08

ockham reviewed Mar 1, 2020

View reviewed changes

roo2 reviewed Mar 2, 2020

View reviewed changes

package.json Show resolved Hide resolved

roo2 reviewed Mar 2, 2020

View reviewed changes

ockham approved these changes Mar 2, 2020

View reviewed changes

scinos reviewed Mar 2, 2020

View reviewed changes

client/tsconfig.json Outdated Show resolved Hide resolved

sirreal added [Status] Ready to Merge Build and removed [Status] In Progress [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Mar 2, 2020

sirreal mentioned this pull request Mar 2, 2020

wpcom-proxy-request: Migrate to TypeScript #39801

Open

roo2 approved these changes Mar 3, 2020

View reviewed changes

scinos mentioned this pull request Mar 4, 2020

Adds tsconfig.json to each package to speed up VSCode #39860

Merged

sirreal force-pushed the update/composite-typescript-setup branch from c3bab60 to 8b09672 Compare March 4, 2020 09:02

sirreal commented Mar 4, 2020

View reviewed changes

sirreal added [Status] In Progress and removed [Status] Ready to Merge labels Mar 4, 2020

sirreal mentioned this pull request Mar 4, 2020

CI: Disable typecheck job #39659

Closed

sirreal force-pushed the update/composite-typescript-setup branch 2 times, most recently from 4a1aa8f to 8da8761 Compare March 4, 2020 09:34

sirreal added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] In Progress labels Mar 4, 2020

sirreal added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Mar 4, 2020

p-jackson approved these changes Mar 5, 2020

View reviewed changes

sirreal added 2 commits March 6, 2020 11:42

Composite TypeScript setup

e68888b

Fix tsc breaking type error

77704d9

sirreal force-pushed the update/composite-typescript-setup branch from 89ff4ce to 77704d9 Compare March 6, 2020 10:42

sirreal added the [Status] Ready to Merge label Mar 6, 2020

sirreal added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Mar 6, 2020

sirreal merged commit 45d38e6 into master Mar 9, 2020

sirreal deleted the update/composite-typescript-setup branch March 9, 2020 10:32

matticbot removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Status] Ready to Merge labels Mar 9, 2020

This was referenced Mar 9, 2020

Build: Clean tsbuildinfo files on package clean #39960

Merged

CI: Remove tsc caching from circleci #39962

Merged

localize-currency-amount: Utility for locale-aware currency formatting #36758

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Build: Composite TypeScript setup #39786

Build: Composite TypeScript setup #39786

sirreal commented Feb 29, 2020 •

edited

Loading

matticbot commented Feb 29, 2020

matticbot commented Feb 29, 2020

ockham Mar 1, 2020 •

edited

Loading

roo2 Mar 2, 2020

sirreal Mar 2, 2020

sirreal Mar 4, 2020

roo2 commented Mar 2, 2020 •

edited

Loading

roo2 Mar 2, 2020

ockham Mar 2, 2020

ockham left a comment

sirreal Mar 4, 2020

sirreal commented Mar 4, 2020

sirreal commented Mar 4, 2020

p-jackson left a comment

sirreal commented Mar 6, 2020

sirreal commented Mar 6, 2020

Build: Composite TypeScript setup #39786

Build: Composite TypeScript setup #39786

Conversation

sirreal commented Feb 29, 2020 • edited Loading

Changes proposed in this Pull Request

Testing instructions

matticbot commented Feb 29, 2020

matticbot commented Feb 29, 2020

ockham Mar 1, 2020 • edited Loading

Choose a reason for hiding this comment

roo2 Mar 2, 2020

Choose a reason for hiding this comment

sirreal Mar 2, 2020

Choose a reason for hiding this comment

sirreal Mar 4, 2020

Choose a reason for hiding this comment

roo2 commented Mar 2, 2020 • edited Loading

roo2 Mar 2, 2020

Choose a reason for hiding this comment

ockham Mar 2, 2020

Choose a reason for hiding this comment

ockham left a comment

Choose a reason for hiding this comment

sirreal Mar 4, 2020

Choose a reason for hiding this comment

sirreal commented Mar 4, 2020

sirreal commented Mar 4, 2020

p-jackson left a comment

Choose a reason for hiding this comment

sirreal commented Mar 6, 2020

sirreal commented Mar 6, 2020

sirreal commented Feb 29, 2020 •

edited

Loading

ockham Mar 1, 2020 •

edited

Loading

roo2 commented Mar 2, 2020 •

edited

Loading