Skip to content
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

feat: add async support to babel-jest #11192

Merged
merged 11 commits into from
Mar 14, 2021
Merged

Conversation

SimenB
Copy link
Member

@SimenB SimenB commented Mar 13, 2021

Summary

Why not 🙂 Only transformer we ship in Jest.

/cc @nicolo-ribaudo

Test plan

Not sure... I've verified via some inserted console.logs that it works. Not sure how to write a test. (marked as draft due to this)

EDIT: copied the synchronous test 🤷 Also added an e2e test

@netlify
Copy link

netlify bot commented Mar 13, 2021

Deploy preview for jestjs ready!

Built without sensitive environment variables with commit 6e4c1c7

https://deploy-preview-11192--jestjs.netlify.app

@netlify
Copy link

netlify bot commented Mar 13, 2021

Deploy preview for jestjs ready!

Built without sensitive environment variables with commit 809380d

https://deploy-preview-11192--jestjs.netlify.app

filename: Config.Path,
transformOptions: JestTransformOptions,
): PartialConfig {
const {cwd} = transformOptions.config;
// `cwd` first to allow incoming options to override it
const babelConfig = loadPartialConfig({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is also the async version of this function, and I think it allows using await in config files.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I assumed there wasn't as it's not in the types 😅

image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh it's because the types are not maintained by us.

We are slowly converting our codebase to TS, we'll release official type definitions when we finish.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can send a PR to DT 👍

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@codecov-io
Copy link

Codecov Report

Merging #11192 (d052f81) into master (dd13096) will decrease coverage by 0.07%.
The diff coverage is 23.68%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #11192      +/-   ##
==========================================
- Coverage   64.28%   64.21%   -0.08%     
==========================================
  Files         307      308       +1     
  Lines       13458    13480      +22     
  Branches     3285     3286       +1     
==========================================
+ Hits         8652     8656       +4     
- Misses       4100     4117      +17     
- Partials      706      707       +1     
Impacted Files Coverage Δ
packages/babel-jest/src/loadBabelConfig.ts 0.00% <0.00%> (ø)
packages/babel-jest/src/index.ts 44.82% <24.32%> (-9.23%) ⬇️
packages/expect/src/utils.ts 94.83% <0.00%> (-1.30%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dd13096...d052f81. Read the comment docs.

@SimenB SimenB marked this pull request as ready for review March 14, 2021 10:16
import babelJest from 'babel-jest';

export default {
...babelJest.default.createTransformer({
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not too happy about this one 🙁 We should output ESM from Jest 28 or something

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think adding export { createTransformer } to babel-jest would be enough.

Copy link
Member Author

@SimenB SimenB Mar 14, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep that'd work, but "real" ESM should use a single default export of an object, so the issue is a "fake" esm module imported directly in native ESM. Edge casey enough that I don't think it's worth changing before just using native ESM

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, and just to be explicit - the .default change is not because of this PR, but the change made in #11193

@github-actions
Copy link

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.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants