🔧 Update build process to use references #44
Conversation
| @@ -75,9 +95,6 @@ jobs: | |||
| - name: Install Packages | |||
| run: yarn install --frozen-lockfile | |||
|
|
|||
| - name: Test | |||
| run: yarn test | |||
There was a problem hiding this comment.
Found that we missed this as this stage relies on it already.
There was a problem hiding this comment.
yeah, I'm still iffy on trusting another machine with different installs potentially passed the build and using that as justification here that we're good. probably enough of an edge case to justify removing this (could use frozen-lockfile in master/push condition also, potentially).
.github/workflows/main.yml
Outdated
| uses: bahmutov/npm-install@v1 | ||
|
|
||
| - name: Run ESLint and TSC type check | ||
| run: yarn build |
There was a problem hiding this comment.
Stop this from happening at merge time.
| @@ -9,7 +9,7 @@ | |||
| "directory": "packages/blocks" | |||
| }, | |||
| "scripts": { | |||
| "tsc": "tsc -p ./tsconfig.build.json", | |||
| "tsc": "tsc -b ./tsconfig.build.json", | |||
There was a problem hiding this comment.
-b is for incremental builds https://www.typescriptlang.org/docs/handbook/project-references.html
| }, | ||
| "references": [ | ||
| { | ||
| "path": "../jest-mock-web-client/tsconfig.build.json" |
There was a problem hiding this comment.
Important to reference the .build here
| @@ -2,6 +2,4 @@ import * as events from './events'; | |||
| import fields from './fields'; | |||
| import ServerlessTester from './serverless-tester'; | |||
|
|
|||
| // Plans to add more fixtures like `web` for responses | |||
| // eslint-disable-next-line import/prefer-default-export | |||
barlock
force-pushed
the
build-with-references
branch
from
0dc7c12 to
2271db5
Compare
* converge on a single workflow with jobs * ✨ add a github actions badge and link to the readme
barlock
force-pushed
the
build-with-references
branch
from
2271db5 to
16140fb
Compare
| @@ -75,9 +95,6 @@ jobs: | |||
| - name: Install Packages | |||
| run: yarn install --frozen-lockfile | |||
|
|
|||
| - name: Test | |||
| run: yarn test | |||
There was a problem hiding this comment.
yeah, I'm still iffy on trusting another machine with different installs potentially passed the build and using that as justification here that we're good. probably enough of an edge case to justify removing this (could use frozen-lockfile in master/push condition also, potentially).
| @@ -4,11 +4,13 @@ | |||
| "scripts": { | |||
| "test": "yarn lint && yarn lerna run test", | |||
| "lint": "yarn eslint && yarn type", | |||
| "eslint": "eslint . --ext .js,.ts --max-warnings 0", | |||
| "eslint": "eslint . --ext .js,.ts --max-warnings 0 --report-unused-disable-directives", | |||
There was a problem hiding this comment.
surprised this isn't more obvious/a best practice. why haven't I been using this?
tsconfig.build.json
Outdated
| "strict": true, | ||
| "preserveSymlinks": true, | ||
| "allowJs": true, | ||
| "noLib": false, |
There was a problem hiding this comment.
this is the default for noLib, I think? https://www.typescriptlang.org/docs/handbook/compiler-options.html
| @@ -23,6 +23,26 @@ jobs: | |||
|
|
|||
| - uses: pre-commit/action@v1.0.1 | |||
|
|
|||
| build: | |||
There was a problem hiding this comment.
do we want this to happen on PR? I guess this way we know explicitly if a PR breaks our build, which I like.
if so, we'll need it to be a required check after merging
Related Issue
Supports #42
Related PRs
This PR is not dependent on any other PR
What does this PR do?
Fixes the broken tsc compiler so we can release things.
Description of Changes
referencesto project tsconfigs. Interestingly, you can't have references and paths play nicely together, so, well, you'll see.--report-unused-disable-directivesto eslint and fixed the one error.Inspiration and description in implementation: https://medium.com/@NiGhTTraX/how-to-set-up-a-typescript-monorepo-with-lerna-c6acda7d4559
Future implementation: microsoft/TypeScript#25376
What gif most accurately describes how I feel towards this PR?