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.

Sign up for GitHub

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

Jump to bottom

fix(jest-config): add mts and cts to default moduleFileExtensions config #14369

Merged
merged 10 commits into from
Sep 20, 2023
Merged

fix(jest-config): add mts and cts to default moduleFileExtensions config #14369

merged 10 commits into from
Sep 20, 2023

Conversation

yinm
Copy link
Contributor

@yinm yinm commented Jul 28, 2023 •
edited by SimenB
Loading

Summary

fixes #14149

This PR will only fix

mts and mts are missing from moduleFileExtensions.

Test plan

test locally

# /jest/e2e/test-match-ts
$ node ../../packages/jest-cli/bin/jest.js
 PASS  __tests__/sample-suite2.cts
 PASS  __tests__/sample-suite.mts

Test Suites: 2 passed, 2 total
Tests:       2 passed, 2 total
Snapshots:   0 total
Time:        0.165 s, estimated 1 s
Ran all test suites.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jul 28, 2023 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

@netlify
Copy link

netlify bot commented Jul 28, 2023 •
edited
Loading

✅ Deploy Preview for jestjs ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 1283494
🔍 Latest deploy log https://app.netlify.com/sites/jestjs/deploys/650ad99f4da9e9000858a1f6
😎 Deploy Preview https://deploy-preview-14369--jestjs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

mrazauskas
mrazauskas suggested changes Jul 28, 2023
View reviewed changes
Copy link
Contributor

@mrazauskas mrazauskas left a comment

Choose a reason for hiding this comment

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

Thanks. I left a couple of minor comments.

docs/Configuration.md Outdated
@@ -1177,6 +1177,7 @@ export default config;
```

We hope to support Prettier v3 seamlessly out of the box in a future version of Jest. See [this](https://github.com/jestjs/jest/issues/14305) tracking issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you split this change into separate PR, please?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. I'll do it later.

@mrazauskas mrazauskas added this to the Jest 30 milestone Jul 28, 2023
@yinm yinm mentioned this pull request Jul 28, 2023
Closed
mrazauskas
mrazauskas approved these changes Jul 28, 2023
View reviewed changes
Copy link
Contributor

@mrazauskas mrazauskas left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks!

@yinm yinm marked this pull request as ready for review July 28, 2023 11:32
SimenB
SimenB reviewed Sep 20, 2023
View reviewed changes
docs/Configuration.md
@@ -92,7 +92,7 @@ const {defaults} = require('jest-config');

/** @type {import('jest').Config} */
const config = {
moduleFileExtensions: [...defaults.moduleFileExtensions, 'mts', 'cts'],
moduleDirectories: [...defaults.moduleDirectories, 'bower_components'],
Copy link
Member

Choose a reason for hiding this comment

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

why is this changed to moduleDirectories?

Copy link
Contributor

Choose a reason for hiding this comment

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

I had the same question. My guess: if 'mts' and 'cts' get included in the default, the code in the example does nothing useful for the user. So @yinm was simply trying to find a way to keep the example useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, @mrazauskas's guess is right. I was trying to find a way to keep the example useful at 0f3e261.

Copy link
Member

Choose a reason for hiding this comment

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

aha! makes sense 🙂

@SimenB SimenB merged commit e7b1e75 into jestjs:main Sep 20, 2023
60 checks passed
@yinm yinm deleted the add-mts-and-cts-to-moduleFileExtensions branch September 20, 2023 14:13
@yinm yinm mentioned this pull request Sep 20, 2023
Closed
@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 Oct 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mrazauskas mrazauskas mrazauskas approved these changes

@SimenB SimenB SimenB approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Jest 30
Development

Successfully merging this pull request may close these issues.

[Bug]: Support {m,c}{j,t}s file extensions for tests and modules
3 participants
@yinm @SimenB @mrazauskas