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
Parse destructuring private fields #13931
Parse destructuring private fields #13931
Conversation
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit ce9dba3:
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/49930/ |
5046646
to
d2979d2
Compare
cb6eadf
to
1c1b938
Compare
@@ -1,9 +1,6 @@ | |||
{ | |||
"type": "File", | |||
"start":0,"end":46,"loc":{"start":{"line":1,"column":0},"end":{"line":4,"column":1}}, | |||
"errors": [ | |||
"SyntaxError: Private names can only be used as the name of a class element (i.e. class C { #p = 42; #m() {} } )\n or a property of member expression (i.e. this.#p). (3:11)" | |||
], |
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.
This test has been moved to packages/babel-parser/test/fixtures/experimental/_no-plugin/destructuring-private-arrow-params/input.js
and throws expected plugins instead.
This test is also duplicated to packages/babel-parser/test/fixtures/experimental/destructuring-private/valid-arrow-params/input.js
as a test case for destructuring private. Here git confusingly marks that output.json
is moved to destructuring-private
.
...babel-parser/test/fixtures/es2022/class-private-properties/invalid-object-method/output.json
Outdated
Show resolved
Hide resolved
"type": "File", | ||
"start":0,"end":46,"loc":{"start":{"line":1,"column":0},"end":{"line":4,"column":1}}, | ||
"errors": [ | ||
"SyntaxError: Private names can only be used as the name of a class element (i.e. class C { #p = 42; #m() {} } )\n or a property of member expression (i.e. this.#p). (3:16)" |
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.
When the plugin is enabled, we should update the error message to also mention destructuring.
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.
The current error message does not cover #p in o
, either. At this point I feel like listing all the valid cases in error messages is a bit verbose. Should we just say unexpected private field?
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.
Uh yes, if adding every case it's too much lets stay vague to cover all of them.
1c1b938
to
176a1ab
Compare
8e2fb74
to
ce9dba3
Compare
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.
Approving, but I think the error message should be updated either to include all possible cases or none of them (half of them doesn't really make sense).
I agree. I will update the error message to the vague "Unexpected private field". |
Maybe "Private name", which is the name of the syntactic element (private field is the "thing" installed on an object that you access with its private name). |
4481524
to
b1aa55a
Compare
@@ -2197,8 +2206,12 @@ export default class ExpressionParser extends LValParser { | |||
return node; | |||
} | |||
|
|||
// https://tc39.es/ecma262/#prod-PropertyName | |||
// when refExpressionErrors presents, it will parse private name |
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.
Nit:
// when refExpressionErrors presents, it will parse private name | |
// when refExpressionErrors is present, it will parse private name |
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.
Nice work!
b1aa55a
to
370efa6
Compare
@JLHwung If you want to work on the transform, make sure to merge this to a |
Merged to |
* feat: parse destructuring private * add test cases * fix: register private name on destructuring * add typings * add syntax plugin * add generator test case * add syntax plugin to preset-stage-2 * fix flow errors * address review comments * fix: use private name in toAssignable * test: add case with undefined refExpressionErrors
* feat: parse destructuring private * add test cases * fix: register private name on destructuring * add typings * add syntax plugin * add generator test case * add syntax plugin to preset-stage-2 * fix flow errors * address review comments * fix: use private name in toAssignable * test: add case with undefined refExpressionErrors
* feat: parse destructuring private * add test cases * fix: register private name on destructuring * add typings * add syntax plugin * add generator test case * add syntax plugin to preset-stage-2 * fix flow errors * address review comments * fix: use private name in toAssignable * test: add case with undefined refExpressionErrors
* feat: parse destructuring private * add test cases * fix: register private name on destructuring * add typings * add syntax plugin * add generator test case * add syntax plugin to preset-stage-2 * fix flow errors * address review comments * fix: use private name in toAssignable * test: add case with undefined refExpressionErrors
Todo:
This PR implements parsing support for the proposal.
When parsing a binding property, it is unambiguous that
{ #x: x }
must be a pattern so we parse the private name and push it to the object pattern.When parsing an assignment property,
{ #x: x }
is ambiguous because it can be an object expression or an object pattern. We register the first private key position asrefExpressionErrors.privateKey
and then decide to throw or discard when the ambiguity is resolved. We addexpectPlugin
check when the private key is consumed in the object pattern, so users won't see the error "expect experimental destructuringPrivate plugin" when the private key is used in an object literal, which can be quite misleading.This PR includes commits from #13929, I will rebase after it gets merged.
Preview REPL (Currently it throws Unknown private name #x because the private field transform can't handle private name in property key yet.)