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

Add TypeScript Support #228

Closed
wants to merge 11 commits into from
Closed

Add TypeScript Support #228

wants to merge 11 commits into from

Conversation

skovy
Copy link
Contributor

@skovy skovy commented Jul 14, 2019

Note: this is a branch off #227 and it must be merged first.

Motivation

Now that jscodeshift supports TypeScript (added in v0.6.0) the react-codemod scripts can also support TypeScript without any custom logic.

Resolves #206.

Changes

  • Upgrade to the latest version of jscodeshift to get parser support for TypeScript
  • Update the defineTest helper in jscodeshift
    • The current helper doesn't allow a way to dynamically define the parser
    • The current helper assumes the file is always .js (not true for TypeScript)
  • Fix tests to define the parser for those that need it (some flow tests that now break with the newer version of jscodeshift)
  • Add basic tests for a TypeScript scenario for renaming deprecated lifecycle methods
  • Update the rename-unsafe-lifecycles to also replace ClassMethod for TypeScript classes

Testing

  • Tested against a TypeScript React public codebase: codesandbox/codesandbox-client#2159
  • #227 and facebook/jscodeshift#332 changes must first be merged, until then
    • Clone jscodeshift locally and checkout the branch with the necessary changes
    • In the jscodeshift directory yarn link
    • In this directory, yarn link jscodeshift
  • yarn test

@threepointone
Copy link
Contributor

@threepointone threepointone commented Jul 18, 2019

Sorry I missed this. I have another PR I'm trying to land today that updates jscodeshift as well #229 I'm trying to land this asap so we can do a react alpha release.

@skovy
Copy link
Contributor Author

@skovy skovy commented Jul 18, 2019

Hey @threepointone, thanks for the heads up 👍 let me know if there are any changes I can make to help add TS support. Looks like you mostly have it in that PR and maybe the only thing left in this one are the tests and the one addition to the transform for renaming unsafe lifecycles?

The tests I added to require a change to jscodeshift test utils to allow testing things other than js.

@threepointone
Copy link
Contributor

@threepointone threepointone commented Jul 27, 2019

I don't expect the jscodeshift PR to land any time soon, so I'd recommend using the Object.assign hack https://github.com/reactjs/react-codemod/blob/master/transforms/__tests__/rename-unsafe-lifecycles-test.js#L13-L17 I'm not sure of a workaround for the extension.

To simplify, I'm happy to accept a PR with just this

root
    .find(j.ClassMethod)
    .forEach(renameDeprecatedApis);

in rename-unsafe-lifecycles.js, since that seems to be the core change.

threepointone added a commit to threepointone/react-codemod that referenced this issue Jul 27, 2019
Thanks to @skovy in reactjs#228, we have a fix for this codemod not working correctly for typescript files. Making this a separate commit/PR to unblock publishing.
@threepointone
Copy link
Contributor

@threepointone threepointone commented Jul 27, 2019

I made a separate PR for the same here #234 so I can publish an update this weekend. Thank you for the effort!

threepointone pushed a commit that referenced this issue Jul 27, 2019
Thanks to @skovy in #228, we have a fix for this codemod not working correctly for typescript files. Making this a separate commit/PR to unblock publishing.
@threepointone
Copy link
Contributor

@threepointone threepointone commented Jul 27, 2019

closing this since #234 landed.

@skovy skovy deleted the skovy/add-typescript-support branch Jul 27, 2019
@skovy
Copy link
Contributor Author

@skovy skovy commented Jul 27, 2019

@threepointone thanks for getting that merged. Should I pull out this test and use the Object.assign hack to prevent potential regressions?

@threepointone
Copy link
Contributor

@threepointone threepointone commented Jul 27, 2019

Ideally so, but I’m not sure how you’ll run the tests in tsx files. Or is that not necessary?

toptaldev92 pushed a commit to toptaldev92/react-codemod that referenced this issue Jul 28, 2021
Thanks to @skovy in reactjs/react-codemod#228, we have a fix for this codemod not working correctly for typescript files. Making this a separate commit/PR to unblock publishing.
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.

2 participants