build: add support for typescript to monorepo #7274
Conversation
mcous
requested review from
a team and
IanLondon
and removed request for
a team
Codecov Report
@@ Coverage Diff @@
## edge #7274 +/- ##
==========================================
- Coverage 80.90% 80.64% -0.27%
==========================================
Files 267 277 +10
Lines 19265 20029 +764
==========================================
+ Hits 15587 16153 +566
- Misses 3678 3876 +198
Continue to review full report at Codecov.
|
mcous
added
the
robot-svcs
| @@ -62,20 +67,17 @@ setup-py: | |||
| $(MAKE) -C $(SHARED_DATA_DIR) setup-py | |||
|
|
|||
| # front-end dependecies handled by yarn | |||
| # TODO(mc, 2021-02-12): add `$(MAKE) build-ts` after `yarn` when we have TS | |||
There was a problem hiding this comment.
#7309 removes this comment and adds build-ts as a dependency to check-js, instead
| @@ -211,11 +224,6 @@ circular-dependencies-js: | |||
| madge $(and $(CI),--no-spinner --no-color) --circular labware-library/src/index.js | |||
| madge $(and $(CI),--no-spinner --no-color) --circular app/src/index.js | |||
|
|
|||
| # upload coverage reports | |||
| .PHONY: coverage | |||
| coverage: | |||
There was a problem hiding this comment.
This has been replaced by codecov/codecov-action. This target was CI-only, so removing it mostly because I'm already touching this file
| @@ -8,6 +8,7 @@ include ./scripts/python.mk | |||
| SHELL := bash | |||
|
|
|||
| # add node_modules/.bin to PATH | |||
| # TODO(mc, 2021-02-12): remove in favor of `yarn run` directly | |||
There was a problem hiding this comment.
Per old thread in Slack. Based on that conversation and testing, there doesn't appear to be any remaining downsides to this
|
|
||
| extends: [ | ||
| 'standard', | ||
| 'standard-with-typescript', |
There was a problem hiding this comment.
This is a very nice config from the standard folks that includes all the normal standard stuff, and enables a bunch of other rules conditionally if the file being checked is TS
I definitely prefer this approach over having to decide and maintain a bunch of our own rules
| @@ -35,6 +35,8 @@ untyped-type-import=error | |||
| [options] | |||
| module.name_mapper.extension='css' -> '@opentrons/components/interfaces/CSSModule.js' | |||
| module.ignore_non_literal_requires=true | |||
| module.system.node.main_field=flow:main | |||
There was a problem hiding this comment.
This allows a module to specific a "flow:main" field in it's package.json for flow to check instead of main. This way, main can point at compiled JS, types can point at some *.d.ts declaration file for TS, and Flow can be shunted over to a shim that covers whatever type definitions we need while the conversion is in progress.
Technically, we could try simply placing a index.flow.js sibling to whatever main index.js file, and Flow would automatically look at that without any main_field setting, but Flow would then blow up if index.js didn't exist. Since our main files are compiled source (that may be cleaned), using main_field seemed like the more developer-friendly option
| "strict": true, | ||
| "allowSyntheticDefaultImports": true, | ||
| "esModuleInterop": true, | ||
| "resolveJsonModule": true |
There was a problem hiding this comment.
This is a fun config that gives us full literal typing on .json imports
Overview
As discussed in 2021's inaugural JS Guild meeting, this PR enables TypeScript in our monorepo in advance of an effort to move off of Flow.
See #4087 for the original RFC and reasoning (which we still agree with!).
Changelog
tscis our least disruptive option, given our existing Babel setupStill To Do
Review requests
The best way to test this PR is to start a conversion branch (or several) on top of this branch and make sure linting, typechecking, etc. all works!
I'll keep this PR in draft form until we have one or more of those going to verify.
Risk assessment
TS additions are pretty carefully scoped to ignore anything that doesn't have a
.ts(x)extension, but the babel toolchain upgrades always carry some risk. Smoke tests (along with automated E2E tests) are prudent here.