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

Support different parsers in the test utils #332

Merged
merged 2 commits into from Oct 29, 2019

Conversation

@skovy
Copy link
Contributor

skovy commented Jul 14, 2019

Motivation

jscodeshift now supports TypeScript (and other parsers) via the CLI. The react-codemod package can now support different React codebases.

Changes

In order to use the defineTest helper in testUtils.js with different parsers the defineTest function needs to allow dynamic parsers and look at different extensions (for TypeScript: .ts, .tsx).

The approach used in this PR was to add another parameter testOptions that allows passing in an object with a parser key. The extension then can be inferred from the file being parsed.

I didn't feel great about adding another parameter so I opted for an object so if there are additional config options they can be added to this object rather than adding more arguments and passing many nulls to get to the proper position. I also considered folding testFilePrefix into this object:

{
  testFilePrefix?: string;
  parser?: "flow" | "ts" | "tsx" | ...;
}

However, this would be a breaking change for those using the testUtils. I'm happy to make that change but opted for avoiding the breaking change.

I also originally experiment with reusing the second existing options parameter but that's what get's passed to the transformer and it felt like mixing two unrelated option configs. Open to feedback on this approach. See the proposed pull request in react-codemod for how this helper would be used and the overall motivation for this change: reactjs/react-codemod#228

Testing

  • See the new TypeScript sample added (in the sample directory)
@Daniel15

This comment has been minimized.

Copy link
Member

Daniel15 commented Oct 21, 2019

Seems like a reasonable idea. Could you please rebase?

@skovy skovy force-pushed the skovy:skovy/support-parsers-in-test-utils branch from b825f3e to 1201750 Oct 23, 2019
@skovy

This comment has been minimized.

Copy link
Contributor Author

skovy commented Oct 23, 2019

Thanks for taking a look @Daniel15! This should be ready for another look 👌

@Daniel15 Daniel15 merged commit 08b8608 into facebook:master Oct 29, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.