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

Added Typescript to the jest config #26

Merged
merged 2 commits into from
Jan 6, 2018

Conversation

mateuscb
Copy link
Contributor

@mateuscb mateuscb commented Jan 5, 2018

What:
This PR addresses issue #25

The idea was to minor Typescript options to the jest config so tests written typescript of projects using kcd-scripts don't have to overwrite too much.

Unfortunately, one still needs to add the given transform. I felt this would have been too specific to add to kcd-scripts src, but added it to the readme so it makes it a bit easier for new-comers using kcd-scripts with typescript.

Why:
Makes it easier when using kcd-scripts in Typescripts project

How:
Added Typescript file extensions to the jest config

Checklist:

  • Documentation
  • Tests N/A
  • Ready to be merged
  • Added myself to contributors table

@codecov
Copy link

codecov bot commented Jan 5, 2018

Codecov Report

Merging #26 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #26   +/-   ##
=======================================
  Coverage   67.06%   67.06%           
=======================================
  Files          22       22           
  Lines         337      337           
  Branches       80       80           
=======================================
  Hits          226      226           
  Misses         83       83           
  Partials       28       28
Impacted Files Coverage Δ
src/config/jest.config.js 100% <ø> (ø) ⬆️

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 709c49d...7062213. Read the comment docs.

@@ -102,6 +102,11 @@ Or, for `jest`:
const {jest: jestConfig} = require('kcd-scripts/config')
module.exports = Object.assign(jestConfig, {
// your overrides here

// for test written in Typescript, add:
Copy link
Contributor Author

@mateuscb mateuscb Jan 5, 2018

Choose a reason for hiding this comment

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

Since kcd-scripts won't work out of the box for Typescript tests because I felt the transform was too specific to put into kcd-script; I thought it would be nice to document what was needed to do so. What do you think?

Copy link
Owner

Choose a reason for hiding this comment

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

Looks good 👍

Copy link
Owner

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

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

Thanks! A few comments.

@@ -102,6 +102,11 @@ Or, for `jest`:
const {jest: jestConfig} = require('kcd-scripts/config')
module.exports = Object.assign(jestConfig, {
// your overrides here

// for test written in Typescript, add:
Copy link
Owner

Choose a reason for hiding this comment

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

Looks good 👍

collectCoverageFrom: ['src/**/*.js'],
testMatch: ['**/__tests__/**/*.js'],
moduleFileExtensions: ['js', 'jsx', 'json', 'ts', 'tsx'],
collectCoverageFrom: ['src/**/*.js', 'src/**/*.js'],
Copy link
Owner

Choose a reason for hiding this comment

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

Should this be:

collectCoverageFrom: ['src/**/*.+(js|jsx|ts|tsx)'],

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also a good question. I got this from a couple projects I work on, which came from else where, and not sure why jsx wasn't added. Will get to the bottom of this. Thanks!

testMatch: ['**/__tests__/**/*.js'],
moduleFileExtensions: ['js', 'jsx', 'json', 'ts', 'tsx'],
collectCoverageFrom: ['src/**/*.js', 'src/**/*.js'],
testRegex: '/__tests__/.*\\.(ts|tsx|js)$',
Copy link
Owner

Choose a reason for hiding this comment

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

Could this be:

testMatch: ['**/__tests__/**/*.+(js|jsx|ts|tsx)'],

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting, I will test this. For some reason I thought testRegex had to be used for regex, and testMatch only accepted list of type. But I'll test and verify.

@mateuscb
Copy link
Contributor Author

mateuscb commented Jan 5, 2018

@kentcdodds I didn't understand that those values accepted glob patterns. That makes a lot more sense. Thanks for catching that!

@kentcdodds kentcdodds merged commit a312574 into kentcdodds:master Jan 6, 2018
@kentcdodds
Copy link
Owner

Thanks! :)

@mateuscb
Copy link
Contributor Author

mateuscb commented Jan 6, 2018

Thank you! :)

@mateuscb mateuscb deleted the jest-with-ts branch January 6, 2018 21:58
layershifter pushed a commit to layershifter/kcd-scripts that referenced this pull request Mar 29, 2021
The value of content should not be converted based on the flow direction
of the document, as it will usually not be the intended result since
`right` is not the RLT equvalent of `left` in the context of `content`.

The equvalent of `left` would be `اليسار` or `يسار` in Arabic and
`שמאל` or `שמאלה` in Hebrew.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants