feat: pass ESM options to transformers #9597
Conversation
Codecov Report
@@ Coverage Diff @@
## master #9597 +/- ##
==========================================
+ Coverage 65.09% 65.11% +0.01%
==========================================
Files 287 287
Lines 12145 12151 +6
Branches 3009 3015 +6
==========================================
+ Hits 7906 7912 +6
Misses 3604 3604
Partials 635 635
Continue to review full report at Codecov.
|
| @@ -90,6 +90,8 @@ export default class ScriptTransformer { | |||
| fileData: string, | |||
| filename: Config.Path, | |||
| instrument: boolean, | |||
| supportsDynamicImport: boolean, | |||
There was a problem hiding this comment.
I made them non-optional for all private methods and default false for the public ones
There was a problem hiding this comment.
should ideally use options objects, as 2-3 boolean flag inputs are quite confusing
There was a problem hiding this comment.
Generally agree, but IIRC functions with more arguments are optimized better than the ones with object arguments, and this is kinda hot path
There was a problem hiding this comment.
potentially we could add separate functions? the es module path can be async (or rather, always is) as well, so that might tie nicely into #9504. Then we could ditch the supportsDynamicImport option since that works for both esm and cjs (and can be async) and just vary on the static version
| @@ -90,6 +90,8 @@ export default class ScriptTransformer { | |||
| fileData: string, | |||
| filename: Config.Path, | |||
| instrument: boolean, | |||
| supportsDynamicImport: boolean, | |||
There was a problem hiding this comment.
Generally agree, but IIRC functions with more arguments are optimized better than the ones with object arguments, and this is kinda hot path
SimenB
force-pushed
the
pass-esm-options-to-transformers
branch
from
5f6eac4
to
a7fac13
Compare
SimenB
force-pushed
the
pass-esm-options-to-transformers
branch
from
a7fac13
to
6aa31e9
Compare
SimenB
force-pushed
the
pass-esm-options-to-transformers
branch
from
6aa31e9
to
17285a9
Compare
|
Extracted the |
packages/jest-runtime/src/index.ts
Outdated
| @@ -498,6 +498,8 @@ class Runtime { | |||
| return { | |||
| ...options, | |||
| ...this._coverageOptions, | |||
| supportsDynamicImport: false, | |||
There was a problem hiding this comment.
should have been in #9766, meh
| @@ -350,12 +380,16 @@ export default class ScriptTransformer { | |||
|
|
|||
| private _transformAndBuildScript( | |||
| filename: Config.Path, | |||
| options: Options | null, | |||
There was a problem hiding this comment.
seems to have been a bug, it was never passed null 🤷♂
SimenB
force-pushed
the
pass-esm-options-to-transformers
branch
from
17285a9
to
75fd400
Compare
SimenB
force-pushed
the
pass-esm-options-to-transformers
branch
from
75fd400
to
2f163d9
Compare
…pshots * upstream/master: (225 commits) docs: add CLA link to contributing docs (jestjs#9789) chore: roll new version of docs v25.3.0 chore: update changelog for release chore(jest-types): correct type testRegex for ProjectConfig (jestjs#9780) feat(circus): enable writing async test event handlers (jestjs#9397) feat: enable all babel syntax plugins (jestjs#9774) chore: add helper for getting Jest's config in e2e tests (jestjs#9770) feat: pass ESM options to transformers (jestjs#9597) chore: replace `any`s with `unknown`s (jestjs#9626) feat: pass ESM options to Babel (jestjs#9766) chore(website): add copy button the code blocks (jestjs#9750) chore: bump istanbul-reports for new uncovered lines design (jestjs#9758) chore: correct CHANGELOG.md (jestjs#9763) chore(jest-types): expose type `CacheKeyOptions` for `getCacheK… (jestjs#9762) docs: Fix simple typo, seperated -> separated (jestjs#9760) v25.2.7 chore: update changelog for release fix: drop getters and setters when diffing objects for error (jestjs#9757) chore(jest-types): correct return type of shouldRunTestSuite fo… (jestjs#9753) ...
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
When
jest-runtimesupports ESM, then babel can leave those statements alone. While not usable yet, this helps reduce the eventual diff for ESM.Test plan
Added some unit tests to some funky merging code, otherwisethis is not really testable until we land more ESM stuff. While it's all added as optional to avoid a breaking change,jest-runtimeexplicitly opts out if it, so no real way of triggering this.