-
Notifications
You must be signed in to change notification settings - Fork 481
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
Added parenthesis around await expression in if statement #566
Comments
I do think this has to be changed in recast. @ElonVolo Have we thought about forking recast to roll in changes like this one without having to wait for upstream to merge them? I personally use parentheses in these cases since it makes the statement less ambiguous :) |
@Daniel15 I also do prefer parentheses for this cases and we'll probably going to use them if no fix can be applied shortly. It's just that the transformation alters the input in ways we aren't expecting which is a bit scary, especially when applied to a large codebase. |
(saying this foolishly before glancing at code)That’s one of the reasons why I created evcodeshift, which I’m thinking (knock on wood) will solve techleadguy’s problem out of the box.Sent from my iPhoneOn Aug 2, 2023, at 16:41, Daniel Lo Nigro ***@***.***> wrote:
I personally use parentheses in these cases since it makes the statement less ambiguous :)
I do think this has to be fixed in recast though. @ElonVolo Have we thought about forking recast to roll in changes like this one without having to wait for upstream to merge them?
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>
|
@ElonVolo (coughs awkwardly while looking down) Well, actually.... evcodeshift has the same issue. |
Abstract
When parsing a file to AST and back (no transformation), the unary expressions containing
await
inside if statements get wrapped in parenthesis.Details
I've stumbled upon the following situation that looks like a bug while developing some codemods for a large Typescript codebase:
AST Explorer repro link: https://astexplorer.net/#/gist/4f8d333d868a270bd53098d4f283e8fa/963bb4d108676986c24b5f5174e8ef36a98b14ea
It's worthy of mentioning that the code is valid and the functionality is not affected by this change but, as I'm applying the transform to a large codebase (23k files), I see a lot of files changed in Git not due to my desired transformation, but due to paranthesis being added in if statements, which makes the review process a lot harder.
Environment
jscodeshift: 0.15.0
node 16.20.0
npm 8.19.4
typescript 4.7.4 ( using "ts" parser )
OS: Microsoft Windows 10 Enterprise
The comand is being ran as npm script, with the following value:
jscodeshift -t src/transform.ts [path_to_source_directory] --parser=ts --extensions=ts
Note: setting
recast
version to 0.20.4 in package.json resolutions did not solve the issueRelated issues
While digging for an explanation, I found these issues which seem to be the opposite of mine ( missing parenthesis ):
#442
#420
And they got fixed as part of this PR, which I'm suspecting introduced the problem:
benjamn/recast#923
Later update: The issue seems to be caused by recast, as somebody stumbled upon a very similar problem: benjamn/recast#1191. Still, if there is a way this can be fixed independently of recast, my heart & soul will have a special place for the developer of that fix.
Thank you for taking the time to consider this!
The text was updated successfully, but these errors were encountered: