Type check test files #248
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
spalladino
force-pushed
the
palla/rename-tsconfig-dest
branch
from
88c54b9 to
fe95fff
Compare
ludamad
reviewed
Apr 12, 2023
yarn-project/archiver/package.json
Outdated
| }, | ||
| "scripts": { | ||
| "prepare": "node ../yarn-project-base/scripts/update_build_manifest.mjs package.json", | ||
| "prepare:check": "node ../yarn-project-base/scripts/update_build_manifest.mjs package.json --check", | ||
| "build": "yarn clean && tsc -b tsconfig.dest.json", | ||
| "build:dev": "tsc -b tsconfig.dest.json --watch", | ||
| "build": "yarn clean && tsc -b tsconfig.json", |
There was a problem hiding this comment.
We can do this in a followup PR but because tsconfig.json is the default name, we can actually drop all these -b and p flags
There was a problem hiding this comment.
Or I can change it really quick with a yarn prepare!
ludamad
reviewed
Apr 12, 2023
| "clean": "rm -rf ./dest .tsbuildinfo", | ||
| "formatting": "run -T prettier --check ./src && run -T eslint ./src", | ||
| "formatting:fix": "run -T prettier -w ./src", | ||
| "test": "NODE_NO_WARNINGS=1 node --experimental-vm-modules $(yarn bin jest) --passWithNoTests", | ||
| "start": "node ./dest", | ||
| "start:dev": "tsc-watch -p tsconfig.dest.json --onSuccess 'yarn start'", | ||
| "start:dev": "tsc-watch -p tsconfig.json --onSuccess 'yarn start'", |
There was a problem hiding this comment.
nit: -p tsconfig.json can also be simplified
ludamad
reviewed
Apr 12, 2023
| @@ -6,20 +6,20 @@ | |||
| "typedoc": { | |||
| "entryPoint": "./src/index.ts", | |||
| "displayName": "Aztec.js", | |||
| "tsconfig": "./tsconfig.dest.json" | |||
| "tsconfig": "./tsconfig.json" | |||
There was a problem hiding this comment.
Eventually if we care, also can be removed
ludamad
approved these changes
Apr 12, 2023
ludamad
pushed a commit
that referenced
this pull request
Jul 14, 2023
* adding adrians new transcript classes * tests added for transcript and new manifest concept --------- Co-authored-by: codygunton <[email protected]>
codygunton
added a commit
that referenced
this pull request
Jan 23, 2024
* adding adrians new transcript classes * tests added for transcript and new manifest concept --------- Co-authored-by: codygunton <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR includes test files in tsconfig, so they are type checked. Now, any type error in a test file is surfaced when building (eg when running build:dev at the project root). This makes refactors that impact test files much easier to handle, since type errors are caught by the compiler and don't need running the tests.
To avoid bundling the tests with the published packages, every package.json now has a
filesfield that defines what gets included, and explicitly removes tests. We are including:Note that the first is the only one actually required for consuming the package. The second is highly recommended to be included, so ts consumers benefit from type definitions. The last two are pretty much optional, and assist clients with debugging, but we could drop them if we think package size becomes an issue.
Adding tests to build surfaced an error with our jest types. Apparently we hit an error with jest type definitions, which is being fixed here. I added a temporary (let's see for how long it survives in the repo!) patch to the type definition to fix it.
This PR changes a lot of files, but each commit should be pretty self-contained, so I suggest reviewing by each individual commit.
Fixes #219