-
Notifications
You must be signed in to change notification settings - Fork 781
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor(jest): narrow getJestPreset typings #5053
Conversation
this commit is a follow up to #5031 (5df16e6). it narrows the return type of the `JestFacade#getJestPreset` function from `any` to an alias to Jest's `Config.InitialOptions`. prior to #5031, the return type of `getJestPreset` would be inferred by the TypeScript compiler, and a dynamic import to an internal Jest type declaration file would be generated for the typing of the function: ```ts // a snippet of what was generated in `jest-stencil-connector.d.ts export declare const getJestPreset: () => Partial<{ coverageReporters: import("@jest/types/build/Config").CoverageReporters; // other fields omitted }>; ``` however, stencil cannot/should not make assumptions about the location of this file. by placing an alias, the dyanmic import is removed from the output `.d.ts` file (having the same effect as #5031). pr #5031 eliminated this by using `any` as an explicit return type, which would generate: ```ts export declare const getJestPreset: () => any; ``` with this commit, we now generate: ```ts export declare const getJestPreset: () => JestPresetConfig; ``` which no longer inlines dynamic imports for typings. STENCIL-1003 Dynamic Import Type Resolution Fails for Jest 28, 29
|
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/client-hydrate.ts | 19 |
src/screenshot/connector-base.ts | 19 |
src/runtime/vdom/vdom-render.ts | 18 |
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 | 417 |
TS2322 | 394 |
TS18048 | 310 |
TS18047 | 101 |
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 15 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/utils/index.ts | 251 | resolve |
src/utils/index.ts | 269 | normalize |
src/utils/index.ts | 7 | escapeRegExpSpecialCharacters |
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested out locally and works great 👍
What is the current behavior?
We have a small piece of technical debt introduced in #5031 where we use an
any
type for the Jest infrastructure. This PR fixes it!GitHub Issue Number: N/A
What is the new behavior?
this commit is a follow up to #5031 (5df16e6). it narrows the return type of the
JestFacade#getJestPreset
function fromany
to an alias to Jest'sConfig.InitialOptions
.prior to #5031, the return type of
getJestPreset
would be inferred by the TypeScript compiler, and a dynamic import to an internal Jest type declaration file would be generated for the typing of the function:however, stencil cannot/should not make assumptions about the location of this file. by placing an alias, the dyanmic import is removed from the output
.d.ts
file (having the same effect as #5031).pr #5031 eliminated this by using
any
as an explicit return type, which would generate:with this commit, we now generate:
which no longer inlines dynamic imports for typings.
Does this introduce a breaking change?
Testing
yarn
to install dependenciesyarn build
to build the project, observe the erroryarn add @stencil/core@4.7.1-dev.1699565413.1847940
yarn build
to build the project, observe no error occursOther information
STENCIL-1003 Dynamic Import Type Resolution Fails for Jest 28, 29