Add block if parent is non-block statement for remove-console/debugger#3583
Add block if parent is non-block statement for remove-console/debugger#3583loganfsmyth merged 2 commits intobabel:masterfrom
Conversation
8447699 to
a57d2d4
Compare
Current coverage is 87.70%@@ master #3583 diff @@
==========================================
Files 193 194 +1
Lines 9285 9430 +145
Methods 1048 1069 +21
Messages 0 0
Branches 2100 2154 +54
==========================================
+ Hits 8129 8271 +142
- Misses 1156 1159 +3
Partials 0 0
|
| visitor: { | ||
| CallExpression(path) { | ||
| if (path.get("callee").matchesPattern("console", true)) { | ||
| const statementPath = path.parentPath.parentPath; |
There was a problem hiding this comment.
Does path.parentPath always exist? I assume even when the script is just debugger, the parent would be program or file. I just want to make sure that we never get "can't get property parentPath on undefined"
There was a problem hiding this comment.
@danez It looks path.parentPath.parentPath gets problem in top-level case for remove-console, and I just used path.parentPath for remove-debugger.
Anyway, @loganfsmyth's recommend will be better, I'll change it. :)
15a007e to
6574722
Compare
| @@ -0,0 +1,3 @@ | |||
| if (blah) console.log(blah); | |||
| for (;;) console.log(blah); | |||
| while (blah) console.log(blah); | |||
There was a problem hiding this comment.
do arrow functions work?
var a = (blah) => console.log(blah);There was a problem hiding this comment.
It looks done in this step of removal-hooks, it will transform to var a = (blah) => void 0;, should we add the case to expression fixture?
There was a problem hiding this comment.
Ah I see, no I think this is fine now.
|
👍 |
|
|
||
| function (self, parent) { | ||
| if ( | ||
| (self.key === 'consequent' || self.key === 'alternate') && parent.isIfStatement() || |
There was a problem hiding this comment.
Seems like we should check the parent type first to be consistent with the rest of these, and to help readability. I'd also be in favor of adding more parens around these && vs || groups since I never remember their precedence.
|
Thanks! |
This PR is re-work for #3453. Fixes the error if we use no block statement (e.g.
if (blah) console.log(blah)) :Transform
to