chore(jest): add directions for adding new jest support#4952
chore(jest): add directions for adding new jest support#4952rwaskiewicz merged 1 commit intomainfrom
Conversation
|
| Path | Error Count |
|---|---|
| src/dev-server/index.ts | 37 |
| src/mock-doc/serialize-node.ts | 36 |
| src/dev-server/server-process.ts | 32 |
| src/compiler/build/build-stats.ts | 27 |
| src/compiler/output-targets/dist-lazy/generate-lazy-module.ts | 25 |
| src/compiler/style/test/optimize-css.spec.ts | 23 |
| src/testing/puppeteer/puppeteer-element.ts | 23 |
| src/compiler/prerender/prerender-main.ts | 22 |
| src/runtime/vdom/vdom-render.ts | 20 |
| src/runtime/client-hydrate.ts | 19 |
| src/screenshot/connector-base.ts | 19 |
| src/compiler/config/test/validate-paths.spec.ts | 16 |
| src/dev-server/request-handler.ts | 15 |
| src/compiler/prerender/prerender-optimize.ts | 14 |
| src/compiler/sys/stencil-sys.ts | 14 |
| src/compiler/transpile/transpile-module.ts | 14 |
| src/runtime/vdom/vdom-annotations.ts | 14 |
| src/sys/node/node-sys.ts | 14 |
| src/compiler/build/build-finish.ts | 13 |
| src/compiler/prerender/prerender-queue.ts | 13 |
Our most common errors
| Typescript Error Code | Count |
|---|---|
| TS2345 | 418 |
| TS2322 | 398 |
| TS18048 | 310 |
| TS18047 | 100 |
| TS2722 | 38 |
| TS2532 | 34 |
| TS2531 | 23 |
| TS2454 | 14 |
| TS2352 | 13 |
| TS2769 | 10 |
| TS2790 | 10 |
| TS2538 | 8 |
| TS2344 | 5 |
| TS2416 | 4 |
| TS2493 | 3 |
| TS18046 | 2 |
| TS2684 | 1 |
| TS2488 | 1 |
| TS2430 | 1 |
Unused exports report
There are 12 unused exports on this PR. That's the same number of errors on main, so at least we're not creating new ones!
Unused exports
| File | Line | Identifier |
|---|---|---|
| src/runtime/bootstrap-lazy.ts | 21 | setNonce |
| src/screenshot/screenshot-fs.ts | 18 | readScreenshotData |
| src/testing/testing-utils.ts | 198 | withSilentWarn |
| src/utils/index.ts | 145 | CUSTOM |
| src/compiler/app-core/app-data.ts | 25 | BUILD |
| src/compiler/app-core/app-data.ts | 115 | Env |
| src/compiler/app-core/app-data.ts | 117 | NAMESPACE |
| src/compiler/fs-watch/fs-watch-rebuild.ts | 123 | updateCacheFromRebuild |
| src/compiler/types/validate-primary-package-output-target.ts | 62 | satisfies |
| src/compiler/types/validate-primary-package-output-target.ts | 62 | Record |
| src/testing/puppeteer/puppeteer-declarations.ts | 485 | WaitForEventOptions |
| src/compiler/sys/fetch/write-fetch-success.ts | 7 | writeFetchSuccessSync |
tanner-reits
left a comment
There was a problem hiding this comment.
Couple small things
src/testing/jest/README.md
Outdated
| └── src/testing/jest/ | ||
| └── jest-27-and-under | ||
| ``` | ||
| Adding support for Jest v28 would begin with making a copy of `jest-27-and-under/` (the latest supported version) and naming it `jest-28/`. |
There was a problem hiding this comment.
Can we somehow make the "latest supported version" callout a bit more prominent? I think we all have enough intuition to use the latest version's adapter in practice, but better safe than sorry. Only calling this out so we don't end up cloning the jest-27-and-under directory as the starting point each time since that has some special cases in files like the preprocessor that have to make additional version checks.
There was a problem hiding this comment.
👍 I tried to do this in ac9c04a - I added Jest 28 into the example as if it already existed and the example goes through adding Jest 29. Hopefully that and some bolding of certain areas brings out the intent more
There was a problem hiding this comment.
Yep, think its pretty clear now. Changing the examples to be a migration from Jest 28 to 29 was a good move 👍
alicewriteswrongs
left a comment
There was a problem hiding this comment.
a few questions / comments
src/testing/jest/README.md
Outdated
| 1. Update the name of the package in the newly created directory to reflect the version of Jest it supports. | ||
| Do so by replacing the one instance of the package name in `package.json` and two instances in `package-lock-json`. | ||
| Following the example above, `@stencil/jest-28` would be replaced with `@stencil/jest-29`. |
There was a problem hiding this comment.
would it make sense to update the package name before installing the new dependencies? just thinking then the package-lock.json would be updated already. also do we maybe want to delete that (the package-lock.json) when starting a new adapter?
There was a problem hiding this comment.
Yeah, that's probably the better thing to do - I tested it out locally and everything worked as expected. Doc updated in 09129f4
src/testing/jest/README.md
Outdated
| 1. It's time to get the project to compile. | ||
| Build Stencil and resolve any typing errors that have arisen from adding new versions of Jest. | ||
| 1. Once Stencil compiles, generate a version that can be installed in a Stencil component starter project (either a tarball or a dev build) and install it into the component starter. | ||
| Attempting to run the tests, working through any initial errors that you may run into. |
There was a problem hiding this comment.
I think maybe
| Attempting to run the tests, working through any initial errors that you may run into. | |
| Attempt to run the tests, working through any initial errors that you may run into. |
create a README file that instructs folks how to add support for a new version of jest
09129f4 to
3bb89e4
Compare
What is the current behavior?
GitHub Issue Number: N/A
What is the new behavior?
create a README file that instructs folks how to add support for a new version of jest
Does this introduce a breaking change?
Testing
n/a - readme only (although these steps are being followed/created as we add support for newer versions of jest, so I suppose technically we're testing it)
Other information
I wanted to push this up & get it out of my Jest 28+ branch. I'll mark it ready for review once Jest 28 is ready for review.