Use TypeScript project references #962

ismail-syed · 2019-09-05T20:42:09Z

Description

Closes #857

This PR adds Typescript Project References to quilt. In doing so we aim to see faster CI builds and faster local rebuilds.

Faster CI builds

Previously ~19mins CI runs
Currently in this branch I'm getting ~14mins

Faster local rebuilds

packages/address-consts/package.json

marutypes

A few questions and comments, mostly I want the extraneous files burned and the ts-ignores gone, but we should also at the least create tasks for making sure the new boilerplate doesn't get out of synch in the future.

marutypes · 2019-09-16T14:22:12Z

packages/tsconfig.json

+    {"path": "./semaphore"},
+    {"path": "./sewing-kit-koa"},
+    {"path": "./useful-types"},
+    {"path": "./with-env"}


It sucks that we have to manually maintain a list like this. It doesn't have to be part of this PR but at the least we should add a task to include adding an entry here in the new package generator, or at least have a separate script that gets run after like we do with adding to the main readme.

I've added a test to cover for now.

marutypes · 2019-09-16T14:23:33Z

packages/react-tracking-pixel/tsconfig.json

+    "./src/**/*.tsx"
+  ],
+  "exclude": ["**/*.test.ts", "**/*.test.tsx"],
+  "references": [{"path": "../react-html"}]


We already have issues with remembering to add dependencies to package.json when they are local packages. We are going to want to have some way of statically enforcing that dependencies are tracked in the tsconfig properly, whether through a linter, a custom script, a repo-level test, or whatever.

The compiler does an ok-ish good job at checking this. If a project is missing an explicit reference here then the build fails. That said, I've seen cases of it passing without an explicit reference. I'll open up an issue.

script.js

packages/react-async/src/Prefetcher.tsx

packages/react-i18n-universal-provider/src/I18nUniversalProvider.tsx

packages/react-self-serializers/src/I18n.tsx

marutypes · 2019-09-16T14:26:22Z

packages/graphql-testing/src/links/mocks.ts

@@ -7,7 +7,7 @@ export class MockLink extends ApolloLink {
    super();
  }

-  request(operation: Operation) {
+  request(operation: Operation): Observable<any> {


is any right here?

I spent a bit of time trying to address these. The changes were significant in some cases. I'd prefer having those changes in a subsequent PR if that's ok with you. I can open up an issue for now

packages/react-import-remote/src/hooks.ts

yarn.lock

GoodForOneFare · 2019-09-16T19:05:49Z

package.json

    "test": "NODE_ICU_DATA=node_modules/full-icu jest --maxWorkers=3 --coverage=false",
    "check": "lerna run check",
    "release": "lerna publish",
-    "clean": "rimraf './packages/*/dist/**/*.{js,d.ts}'",
+    "clean": "rimraf './packages/*/dist/**/*.{js,d.ts}' './packages/*/*.tsbuildinfo' './packages/*.tsbuildinfo'",


I know dist/src is annoying, but it's unavoidable for most projects. So I'd prefer standardising on that, even for projects that currently use dist.

More importantly, how are you planning to have tsbuildinfo files cached in CI? I'm still at the top of the PR, so I may be missing the magic that makes it happen.

So I'd prefer standardising on that, even for projects that currently use dist

I've updated all the monorepo tsconfigs to reflect this as sewing-kit does. ('.' as the baseUrl and rootDir). Also changed the number of imports from ./dist/foo to ./dist/src/foo.

how are you planning to have tsbuildinfo files cached in CI?

I tried getting Travis to only cache the tsbuildinfo files, but I couldn't find any docs or working examples of caching files. Only directories. How does sewing-kit cache tsbuildinfo files?

In Travis, what if we move all the tsbuildinfos to a tmp_build_cache directory of some sort after a build and dump them back into their respective packages/* paths before a subsequent build starts.

GoodForOneFare · 2019-09-16T19:12:11Z

packages/useful-types/tsconfig.json

@@ -0,0 +1,15 @@
+{
+  "extends": "../tsconfig_base.json",
+  "compileOnSave": false,


compileOnSave should be in the base config, and deleted from all others.

GoodForOneFare · 2019-09-16T19:13:55Z

packages/useful-types/tsconfig.json

+    "rootDir": "./src"
+  },
+  "include": [
+    "../../config/typescript/**/*",


What is this pulling in?

Doesn't seem like anything. I wrote a script that converted all tsconfig.build.json => tsconfig.json and updated the extended field. Looking at the history of this file this must have been included from the generators.

GoodForOneFare · 2019-09-16T19:14:04Z

packages/useful-types/tsconfig.json

+  "include": [
+    "../../config/typescript/**/*",
+    "./src/**/*.ts",
+    "./src/**/*.tsx"


Why does this need to process tsx files?

GoodForOneFare · 2019-09-16T19:14:21Z

packages/useful-types/tsconfig.json

+    "./src/**/*.ts",
+    "./src/**/*.tsx"
+  ],
+  "exclude": ["**/*.test.ts", "**/*.test.tsx"]


There are no test files, so 🔥

GoodForOneFare · 2019-09-16T19:15:20Z

packages/with-env/tsconfig.json

+    "rootDir": "./src"
+  },
+  "include": [
+    "../../config/typescript/**/*",


Same question about pulling in config and tsx files as above.

GoodForOneFare · 2019-09-16T19:16:12Z

packages/tsconfig_base.json

  },
-  "include": ["./**/*.ts", "./**/*.tsx"],
-  "exclude": ["./node_modules"]
+  "include": ["../config/typescript/**/*.d.ts"]


Does it make sense to have an include here? I thought all projects should specify their own includes?

This should be removed

ismail-syed · 2019-09-16T20:06:32Z

Thanks for all the feedback folks. I'll get back around to addressing these comments sometime this week. Metrics tracking works is higher on my list of but I'll be been pushing this in-between cycles.

…uilds

ismail-syed force-pushed the ts-project-references branch 5 times, most recently from f906ee4 to 8429568 Compare September 11, 2019 05:34

ismail-syed force-pushed the ts-project-references branch 13 times, most recently from 05fa209 to 8437ba2 Compare September 16, 2019 02:44

ismail-syed commented Sep 16, 2019

View reviewed changes

packages/address-consts/package.json Show resolved Hide resolved

ismail-syed changed the title ~~[WIP] Migrate codebase to use TS project references & incremental builds~~ Migrate codebase to use TS project references & incremental builds Sep 16, 2019

ismail-syed force-pushed the ts-project-references branch from df054dd to 11207eb Compare September 16, 2019 14:07

ismail-syed changed the title ~~Migrate codebase to use TS project references & incremental builds~~ Use TypeScript project references with incremental builds Sep 16, 2019

ismail-syed marked this pull request as ready for review September 16, 2019 14:17

ismail-syed requested review from GoodForOneFare, cartogram and marutypes September 16, 2019 14:17

marutypes suggested changes Sep 16, 2019

View reviewed changes

GoodForOneFare reviewed Sep 16, 2019

View reviewed changes

yarn.lock Show resolved Hide resolved

GoodForOneFare suggested changes Sep 16, 2019

View reviewed changes

GoodForOneFare approved these changes Nov 15, 2019

View reviewed changes

ismail-syed added 10 commits November 15, 2019 13:46

Migrate codebase to use Typescript project references & incremental b…

641a0c4

…uilds

Remove some ts-ignores

c824b0b

Remove some lingering ts-ignores

ac6d607

Add ExtendedWindow to useful-types

c23b539

Remove script

0b1a407

PR fixes

922b0e2

Fix bad rebase

99bfddd

Cleanup & lint fixes

7ff62fe

PR fixes

ae0a46b

Add tests for project reference things

5ec4a47

ismail-syed force-pushed the ts-project-references branch from d1c8ac9 to 5ec4a47 Compare November 15, 2019 18:46

ismail-syed changed the title ~~Use TypeScript project references with incremental builds~~ Use TypeScript project references Nov 15, 2019

yarn.lock fix

6b4364e

This was referenced Nov 15, 2019

Improving TypeScript project reference automation #1181

Closed

Improve typings on Observable types #1182

Closed

ismail-syed merged commit d3720fd into master Nov 15, 2019

ismail-syed requested a deployment to production November 15, 2019 20:46 Abandoned

michenly requested a deployment to michenly-beta November 15, 2019 20:53 Abandoned

michenly requested a deployment to michenly-beta November 15, 2019 21:19 Abandoned

michenly requested a deployment to michenly-beta November 15, 2019 21:31 Abandoned

michenly requested a deployment to michenly-beta November 15, 2019 21:47 Abandoned

michenly requested a deployment to michenly-beta November 15, 2019 22:24 Abandoned

ismail-syed requested a deployment to backfill November 15, 2019 22:35 Abandoned

ismail-syed deployed to backfill November 15, 2019 22:54 Active

michenly had a problem deploying to production November 19, 2019 16:36 Failure

michenly temporarily deployed to production November 21, 2019 17:49 Inactive

michenly temporarily deployed to michenly-beta November 21, 2019 17:54 Inactive

theodoretan temporarily deployed to gem December 10, 2019 21:19 Inactive

BPScott deleted the ts-project-references branch May 22, 2021 00:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use TypeScript project references #962

Use TypeScript project references #962

ismail-syed commented Sep 5, 2019 •

edited

Loading

marutypes left a comment

marutypes Sep 16, 2019

ismail-syed Nov 14, 2019

marutypes Sep 16, 2019 •

edited

Loading

ismail-syed Nov 14, 2019

marutypes Sep 16, 2019

ismail-syed Nov 14, 2019 •

edited

Loading

GoodForOneFare Sep 16, 2019

GoodForOneFare Sep 16, 2019

ismail-syed Nov 14, 2019

ismail-syed Nov 14, 2019

GoodForOneFare Sep 16, 2019

GoodForOneFare Sep 16, 2019

ismail-syed Sep 26, 2019

GoodForOneFare Sep 16, 2019

GoodForOneFare Sep 16, 2019

GoodForOneFare Sep 16, 2019

GoodForOneFare Sep 16, 2019

ismail-syed Sep 26, 2019

ismail-syed commented Sep 16, 2019

Use TypeScript project references #962

Use TypeScript project references #962

Conversation

ismail-syed commented Sep 5, 2019 • edited Loading

Description

Faster CI builds

Faster local rebuilds

marutypes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marutypes Sep 16, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ismail-syed Nov 14, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ismail-syed commented Sep 16, 2019

ismail-syed commented Sep 5, 2019 •

edited

Loading

marutypes Sep 16, 2019 •

edited

Loading

ismail-syed Nov 14, 2019 •

edited

Loading