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 known limitation #251

Merged
merged 2 commits into from
Jun 19, 2017
Merged

added known limitation #251

merged 2 commits into from
Jun 19, 2017

Conversation

masters3d
Copy link
Contributor

@kulshekhar
Copy link
Owner

I'm not sure this is a limitation.

Catching, processing and responding to tsc errors should be part of the build/live development step.

The purpose of ts-jest is to help write tests using Jest use in Typescript projects.

@masters3d
Copy link
Contributor Author

masters3d commented Jun 18, 2017

Don't have to call it a limitation but it is something I believe should be documented.
How about:

Out of scope for this project:

  • TS-Jest as a transformer is not able to show compiler errors in Jest
    A workout is to call the ts compiler before calling Jest tsc --noEmit -p . && jest

@kulshekhar
Copy link
Owner

I might be wrong but I don't see the need to mention something that's out of scope.

@GeeWee @bcruddy thoughts?

@masters3d
Copy link
Contributor Author

I added more background. Feel free to close this if you think it is not needed. This is more of a ts compiler limitation. microsoft/TypeScript#4864

@kulshekhar
Copy link
Owner

@masters3d I've shared my thoughts but let's keep this open and see what the others think about it. If it's helpful and doesn't add any confusion, we might merge this in :)

Thanks for adding the additional details. If it goes in, that would be useful contextual information.

@GeeWee
Copy link
Collaborator

GeeWee commented Jun 18, 2017

If this is a known tsc issue with transpileModule, I think this should go in. It's reasonably valuable information.

README.md Outdated
@@ -263,6 +263,10 @@ By default Jest ignores everything in `node_modules`. This setting prevents Jest
"moduleDirectories": ["node_modules", "<path_to_your_sources>"]
}
```
### TS compiler && error reporting
- Only syntactic errors are reported by the [tsc `transpileModule`](https://github.com/Microsoft/TypeScript/issues/4864#issuecomment-141567247)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is a little too technical. Perhaps the first line could be 'ts-jest only returns syntax errors from TSC' or something like that?

README.md Outdated
### TS compiler && error reporting
- Only syntactic errors are reported by the [tsc `transpileModule`](https://github.com/Microsoft/TypeScript/issues/4864#issuecomment-141567247)
- Non syntactic errors do not show up in [Jest](https://github.com/facebook/jest/issues/2168)
- A workaround is to call the ts compiler before calling Jest `tsc --noEmit -p . && jest`
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think in a lot of cases you want to run the tests even though tsc reports errors. Maybe last line could be 'If you only want to run jest if tsc does not output any errors, a workaround is ...' ?

@GeeWee
Copy link
Collaborator

GeeWee commented Jun 18, 2017

I am okay with this PR, and I don't think we should necessarily seek to overcome this limitation. The ability to run type-unsafe code while doing e.g. TDD is valuable.

@kulshekhar
Copy link
Owner

I think this should go in. It's reasonably valuable information.

My concern is that this would add no value and could cause confusion.

I'm not sure devs expect their testing framework to tell them about language errors. Editors/IDEs and build processes are responsible for that, no?

If you still think this adds value, feel free to merge it in

@GeeWee
Copy link
Collaborator

GeeWee commented Jun 19, 2017

I imagine that some people might use ts-jest in a CI environment, and would expect it to fail on tsc warnings.

@GeeWee GeeWee merged commit 04e13c0 into kulshekhar:master Jun 19, 2017
@masters3d masters3d deleted the patch-1 branch June 19, 2017 07:18
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.

3 participants