Add TypeScript tests for the rename-unsafe-lifecycles transform#236
Add TypeScript tests for the rename-unsafe-lifecycles transform#236threepointone merged 2 commits intoreactjs:masterfrom skovy:skovy/add-rename-lifecycle-typescript-test
Conversation
| }); | ||
|
|
||
| afterEach(() => { | ||
| jest.resetModules(); |
There was a problem hiding this comment.
This was necessary to clear the jest.mock above, otherwise the following typescript block was ran with the flow parser because of the above mock. Jest Docs.
Thought about doing a beforeAll/afterAll since all the flow tests need the same mock, or promoting this afterEach to the top-level describe. Thought this was cleaner to run before/after every test and keep it specific to where it's needed but happy to change.
|
@threepointone RE: #228 (comment) Here's an attempt at adding test cases for TypeScript. It works but it's not elegant. Not sure if it's better to have no tests or these hacky tests (relying on |
|
Looks good to me! Thank you for doing this, appreciate the weekend effort. |
|
@threepointone definitely, thanks for the reviews! If any new transforms get added in future React releases feel free to tag me and I can test on a larger TypeScript codebase and add tests if needed 👍 For the most part the ASTs seem to be pretty similar but there was that one difference with the class methods. (haven't tried any of the other transforms so those might have different issues) |
does not works in my case. All *.tsx files was skiped and For me looks like tests does nothing |
|
@FDiskas please file a fresh issue with a reproducing example (like a git repo). |
Only the tests from #228.
The transform has to be mocked because
jscodeshiftdoesn't support testing other parsers/file extensions.I named the examples
class.tsx.input.js(must bejsto match whatjscodeshiftexpects) but still keep thetsxto help show that it's expected to actually be a TSX file (and can be if something like facebook/jscodeshift#332 is merged). Happy to remove this and keep it onlyclass.input.js😕