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

handle typescript by default #7533

Merged
merged 3 commits into from
Dec 19, 2018
Merged

handle typescript by default #7533

merged 3 commits into from
Dec 19, 2018

Conversation

SimenB
Copy link
Member

@SimenB SimenB commented Dec 18, 2018

Summary

Now that we've landed Babel 7, I think we should handle TypeScript by default, next to JS, JSX and Flow.

See https://2018.stateofjs.com/javascript-flavors/overview/ for why I think this should be #0C

Test plan

We have an integration test with Babel and typescript, so I just removed its config.

@@ -15,18 +15,5 @@
},
"scripts": {
"test": "jest"
},
"jest": {
Copy link
Member Author

@SimenB SimenB Dec 18, 2018

Choose a reason for hiding this comment

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

this is my "test plan". The tests still run (and pass)

@@ -148,6 +152,17 @@ const setupBabelJest = (options: InitialOptions) => {
babelJest = customJSTransformer;
}
}

if (customTSPattern) {
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 sure about this one. Is this correct?

Copy link
Collaborator

@thymikee thymikee Dec 18, 2018

Choose a reason for hiding this comment

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

couldn't we just extend the customJSPattern check?

const customPattern = Object.keys(transform).find(pattern => {
  const regex = new RegExp(pattern);
  return regex.test('a.js') || regex.test('a.jsx') || regex.test('a.ts') || regex.test('a.tsx');
});

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, that doesn't make sense

Copy link
Member Author

Choose a reason for hiding this comment

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

what if there are different patterns for js and ts, such as ts-jest?

@SimenB
Copy link
Member Author

SimenB commented Dec 18, 2018

/cc @kulshekhar @huafu @GeeWee. I don't think ts-jest will need to change, but I've been known to be terribly mistaken before 🙂

Copy link
Collaborator

@thymikee thymikee left a comment

Choose a reason for hiding this comment

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

LGTM

@GeeWee
Copy link

GeeWee commented Dec 19, 2018

Just want to say that I am excited as fuck for this.

I think we might have to change some docs on the ts-jest side, but I don't think it'll be an issue for us. I think the others will have a better idea however.

@SimenB
Copy link
Member Author

SimenB commented Dec 19, 2018

Awesome! I think docs and maybe the preset (as it won't have to set moduleFileExtensions etc). But it shouldn't break anything as you'll just overwrite our defaults, it's just unnecessary.

@GeeWee
Copy link

GeeWee commented Dec 19, 2018

Yeah that's what I'm thinking as well.

README.md Outdated Show resolved Hide resolved
@rubennorte rubennorte self-requested a review December 19, 2018 12:18
docs/GettingStarted.md Outdated Show resolved Hide resolved
Co-Authored-By: SimenB <sbekkhus91@gmail.com>
@codecov-io
Copy link

Codecov Report

Merging #7533 into master will decrease coverage by 0.01%.
The diff coverage is 83.33%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #7533      +/-   ##
=========================================
- Coverage   67.71%   67.7%   -0.02%     
=========================================
  Files         247     247              
  Lines        9501    9504       +3     
  Branches        5       5              
=========================================
+ Hits         6434    6435       +1     
- Misses       3065    3067       +2     
  Partials        2       2
Impacted Files Coverage Δ
packages/jest-cli/src/lib/init/questions.js 100% <ø> (ø) ⬆️
packages/jest-cli/src/lib/init/index.js 87.8% <ø> (-1.09%) ⬇️
packages/jest-config/src/ValidConfig.js 100% <ø> (ø) ⬆️
packages/jest-config/src/Defaults.js 100% <ø> (ø) ⬆️
packages/jest-config/src/constants.js 100% <100%> (ø) ⬆️
...ages/jest-cli/src/lib/init/generate_config_file.js 100% <100%> (ø) ⬆️
packages/jest-config/src/normalize.js 83.01% <80%> (-0.13%) ⬇️

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 873c641...ba2328c. Read the comment docs.

@SimenB SimenB merged commit c7caa7b into jestjs:master Dec 19, 2018
@SimenB SimenB deleted the typescript-tests branch December 19, 2018 13:34
@SimenB
Copy link
Member Author

SimenB commented Dec 20, 2018

For people coming to this thread, it has been released in 24.0.0-alpha.9, tagged as jest@beta. Feedback very much welcome 🙂

@sebald
Copy link

sebald commented Dec 20, 2018

I know this is very early, but does this patch include support for TS features like path mapping? (Not sure if this is covered by Babel) 🙂

@thymikee
Copy link
Collaborator

We rely on Babel for our default TS support. If you need something more advanced, then ts-jest is a way to go like it always was.

@SimenB
Copy link
Member Author

SimenB commented Dec 20, 2018

This is using purely babel, no tsconfig config is picked up. You'll want to use https://jestjs.io/docs/en/configuration.html#modulenamemapper-object-string-string. Or ts-jest if you want deeper typescript integration.

EDIT: Heh, what he said

@jeffryang24
Copy link

This means that if I want to use different tsconfig.json for the test, I should use ts-jest? 🤔

@SimenB
Copy link
Member Author

SimenB commented Dec 20, 2018

if you want to use a tsconfig at all you need to use ts-jest

@sebald
Copy link

sebald commented Dec 20, 2018

@thymikee @SimenB thanks for clarification. I was just checking if I can get rid of ts-jest 🙂

@milesj
Copy link

milesj commented Dec 20, 2018

As someone who only writes TS, love this. <3

Edit: I just tested the beta with this PR and everything worked great with TS.

captain-yossarian pushed a commit to captain-yossarian/jest that referenced this pull request Jul 18, 2019
@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 12, 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.

10 participants