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 simultaneous support of jest v26 & v27 for transformer #548

Merged
merged 7 commits into from
Sep 3, 2021
Merged

Conversation

cbmd
Copy link
Contributor

@cbmd cbmd commented Aug 9, 2021

No description provided.

@piglovesyou
Copy link
Owner

Thank you for making it solid and compatible, @cbmd. Could you take a look at the build results?

Comment on lines 28 to 30
input,
filePath,
__compatJestConfig,

Choose a reason for hiding this comment

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

jest and babel-jest use arg names of (sourceText, sourcePath, options) for both process and getCacheKey, it would be cool to get aligned with them: https://jestjs.io/docs/next/code-transformation#writing-custom-transformers

@cbmd
Copy link
Contributor Author

cbmd commented Aug 27, 2021

Sure, i'll take a look next week @piglovesyou!

@cbmd
Copy link
Contributor Author

cbmd commented Aug 30, 2021

@piglovesyou it's ready to re-review.

Copy link
Owner

@piglovesyou piglovesyou left a comment

Choose a reason for hiding this comment

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

So nice. Thank you @cbmd!!

I left one comment. Could you consider and share what you think about it?

src/jestTransformer.test.ts Show resolved Hide resolved
Copy link
Contributor Author

@cbmd cbmd left a comment

Choose a reason for hiding this comment

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

LGTM

@piglovesyou piglovesyou merged commit caa137d into piglovesyou:main Sep 3, 2021
@cbmd
Copy link
Contributor Author

cbmd commented Sep 8, 2021

@piglovesyou can this one be released to npm? :)

@piglovesyou
Copy link
Owner

I'll do it soon! Sorry, I've forgotten it after my tasks..

@piglovesyou
Copy link
Owner

v0.18.5 includes your improvement. Thank you, @cbmd!

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