fix: async transform test support #518
Conversation
| * Example jscodeshift async transformer. Simply reverses the names of all | ||
| * identifiers. | ||
| */ | ||
| function transformer(file, api) { |
There was a problem hiding this comment.
Make this async rather than returning a resolved promise? I think that'll more accurately reflect how people use it.
| @@ -50,6 +50,7 @@ | |||
| "devDependencies": { | |||
| "babel-eslint": "^10.0.1", | |||
| "eslint": "^5.9.0", | |||
| "is-promise": "^4.0.0", | |||
There was a problem hiding this comment.
Do we really need to pull in a third-party library here? Every third-party dependency adds extra liability.
I guess this is needed to support "promise-like" objects, when used in older versions of Node that require a polyfill? Node added native promises in v0.12 way back in 2015, so that shouldn't be an issue. I also guess it could be an issue if Jest mocks promises
There was a problem hiding this comment.
I want use util.types.isPromise, but it added in v10...
| if (isPromise(output)) { | ||
| return output.then(result => { | ||
| expect(result).toMatchSnapshot(); | ||
|
|
| return (output || '').trim(); | ||
| } | ||
| exports.applyTransform = applyTransform; | ||
|
|
||
| function runSnapshotTest(module, options, input) { | ||
| const output = applyTransform(module, options, input); | ||
|
|
||
| if (isPromise(output)) { |
There was a problem hiding this comment.
Too bad we can't use async here, because then this could be a bit cleaner:
let output = applyTransform(.......);
if (isPromise(output)) {
output = await output;
}
expect(output).toMatchSnapshot();
I think jscodeshift still supports Node.js versions without native async/await...
There was a problem hiding this comment.
Yes, I think so. On the other hand, using async/await will cause the return value to become a promise, I'm not sure if this will affect the existing logic...
I think these modifications are ugly, i'll close the PR.
fix #516