-
Notifications
You must be signed in to change notification settings - Fork 745
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
feat: [transformers] add example formatting + compatible parsers #586
feat: [transformers] add example formatting + compatible parsers #586
Conversation
17df1bc
to
fc48ed2
Compare
const getJscodeshiftParser = (parser, parserSettings) => { | ||
if (parser === 'typescript') { | ||
if (parserSettings.typescript && parserSettings.typescript.jsx === false) { | ||
return 'ts' | ||
} | ||
return 'tsx' | ||
} | ||
if (parser === 'flow') { | ||
return 'flow' | ||
} | ||
return 'babel' | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's probably some larger question about maintainability here since there are quite a few parsers that are capable of jsx + typescript these days beyond just the typescript parser, like @babel/parser, @typescript-parser/eslint, etc
I called out just the typescript parser as the primary one here - but alternatively, if a parser isn't defined here then we'll at least have the module.exports.parser so it'll be a bit more obvious what's needed to get jscodeshift less upset 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤷 Another alternative is to just downscope this entire feature to just adding the module.parser
to the codeExample and call it there.
I think this is a good solution for now, even if it's just for jscodeshift. In the long term I want parsers and transformers to be more integrated. The current isolation of parsers and transformers is causing confusion and also limits how transformers can be used. |
website/src/parsers/js/transformers/jscodeshift/codeExample.txt
Outdated
Show resolved
Hide resolved
fc48ed2
to
11132f3
Compare
This adds two new configuration options for transformers: - compatibleParserIDs - formatCodeExample compatibleParserIDs is used in SELECT_TRANSFORMER as an alternative to always resetting the parser when changing the transformer or turning it on. If we are already on a compatible parser, we won't change. formatCodeExample supports custom formatting/templating for code examples - in the case of jscodeshift, this is used to set the module.exports.parser based on current parser. In combination, this supports a workflows like: - Copy in a typescript file contents - Change the parser to typescript - Turn on the transform for jscodeshift
11132f3
to
c021e4e
Compare
Hi @fkling! Wanted to follow up to see if there was anything else you'd like me to address. Thanks! :) |
Sorry, just not finding much time recently. |
Looking for a little discussion on approach! I use AST explorer only every so often and most recently I found myself stumbling a bit again when I tried to write a jscodeshift transform, and was only met with the error below:
Here's an attempt to demystify some of this by adding the
module.exports.parser
directly to the code example (with a cute little formatter to automatically inject the "tsx" transformer)Here's a gif of the flow:
Jun-03-2021.00-53-41.mp4
This adds two new configuration options for transformers:
compatibleParserIDs is used in SELECT_TRANSFORMER as an alternative to
always resetting the parser when changing the transformer or turning it
on. If we are already on a compatible parser, we won't change.
formatCodeExample supports custom formatting/templating for code
examples - in the case of jscodeshift, this is used to set the
module.exports.parser based on current parser.
In combination, this supports a workflows like: