Disallow destructuring assignment in using declarations#16613
Conversation
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/57276 |
nicolo-ribaudo
left a comment
There was a problem hiding this comment.
Thank you! This PR is already good as is, but it would be even better if we could give a good error also for this case:
using { x } = y;by adapting the logic in https://github.com/H0onnn/babel/blob/3ae03e1b53c69eec8850071eca53908c09c1d6ed/packages/babel-parser/src/parser/statement.ts#L505-L513 so that if using is followed on the same line by { (other than by an identifier) we parse it as an using declaration, and then it will raise the error that you added in this PR.
Unfortunately we cannot do the same for [, because using [x] = y is valid code (setting the x property of the variable using).
|
@nicolo-ribaudo Hello! Thank you for the quick review. Added code to allow for 'ArrayPattern'. Will this work meet your requirements? |
|
@H0onnn No, your original commit was correct :) We must report an error for My suggestion was to change the logic in https://github.com/H0onnn/babel/blob/3ae03e1b53c69eec8850071eca53908c09c1d6ed/packages/babel-parser/src/parser/statement.ts#L505-L513 to also check for If you prefer, we can also merge this PR (with only the first commit, and not the second), and then do this suggestion in a separate PR. |
|
@nicolo-ribaudo I understand. Please understand that my English skills are not good. 😅 I'll keep trying ! |
| "start":0,"end":30,"loc":{"start":{"line":1,"column":0,"index":0},"end":{"line":3,"column":1,"index":30}}, | ||
| "errors": [ | ||
| "SyntaxError: Using declaration cannot have destructuring patterns. (2:8)" | ||
| ], |
There was a problem hiding this comment.
This is a valid case, when the placeholder plugin is enabled, using %%name%% = null is a valid production. Can we throw the error when id is either ArrayPattern or ObjectPattern?
There was a problem hiding this comment.
Hello @JLHwung! Thank you for your feedback. As you said, modified it to throw an error only in the case of ObjectPattern and ArrayPattern.
|
@nicolo-ribaudo I hope that the logic added this commit will satisfy your requirements! |
nicolo-ribaudo
left a comment
There was a problem hiding this comment.
I pushed a commit to improve the error message of using { (and added a test) -- thank you!
using declarations
Fixes #1, Fixes #2usingdeclarations shoud disallow destructuring #16604A syntax error occurs when using destructuring assignment in a using declaration.