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
Fix optional chain optimization in sequence expression #15888
Fix optional chain optimization in sequence expression #15888
Conversation
(t.isExpressionStatement(replacementPath.parent) && | ||
!replacementPath.isCompletionRecord()) || | ||
(t.isSequenceExpression(replacementPath.parent) && | ||
last(replacementPath.parent.expressions) !== replacementPath.node); |
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.
It might make sense to have a path.isValueObservable()
helper eventually.
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.
It certainly reminds me of the messy path._getCompletionRecords
. Thankfully the switch statement is not an expression.
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.
Similar usage:
babel/packages/babel-plugin-proposal-destructuring-private/src/index.ts
Lines 162 to 164 in c71a217
const shouldPreserveCompletion = | |
(!isExpressionStatement(parent) && !isSequenceExpression(parent)) || | |
path.isCompletionRecord(); |
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/55206/ |
Releasing a patch to fix the regression. |
The problem is that
isCompletionRecord
in the test code is returning false, because the sequence expression was not in a completion record position.