Skip to content
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

Allow TypeScript type assertions in array destructuring #10592

Merged
merged 5 commits into from Nov 11, 2019

Conversation

@SakibulMowla
Copy link
Contributor

SakibulMowla commented Oct 22, 2019

Q                       A
Fixed Issues? Fixes #10066
Patch: Bug Fix? Yes
Major: Breaking Change? No
Minor: New Feature? No
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes? No
License MIT
contextDescription: string,
): $ReadOnlyArray<N.Pattern> {
const isAssignmentExpression =
contextDescription === "assignment expression";

This comment has been minimized.

Copy link
@nicolo-ribaudo

nicolo-ribaudo Oct 25, 2019

Member

I would prefer not to rely on the exact value of this string, since it's only used for error messages. Could we use !isBinding instead of isAssignmentExpression?

This comment has been minimized.

Copy link
@SakibulMowla

SakibulMowla Oct 26, 2019

Author Contributor

Highly agree on not using the value of the string. What does isBinding represent? Can you point me to any doc or link?

This comment has been minimized.

Copy link
@nicolo-ribaudo

nicolo-ribaudo Oct 29, 2019

Member

In JavaScript, bindings (1, 2) are what you use to declare a variable. For example, in var { x } = {}, { x } is a binding pattern and x is a binding identifier. On the other hand, in ({ x } = {}) there isn't any binding because you aren't declaring anything new (you are reassigning an existing variable).

@@ -0,0 +1 @@
(a as number) = 42;

This comment has been minimized.

Copy link
@existentialism

existentialism Oct 26, 2019

Member

Shouldn't this be:

[a as number] = arr

?

This comment has been minimized.

Copy link
@SakibulMowla

SakibulMowla Oct 26, 2019

Author Contributor

It sure should be. Was playing around with input and forgot to change it back. Thanks for noticing!

@SakibulMowla SakibulMowla force-pushed the SakibulMowla:issue-10066 branch 2 times, most recently from cecc5d6 to 2b372b9 Oct 26, 2019
@SakibulMowla SakibulMowla force-pushed the SakibulMowla:issue-10066 branch from 2b372b9 to 35786c7 Oct 30, 2019
@buildsize

This comment has been minimized.

Copy link

buildsize bot commented Oct 30, 2019

File name Previous Size New Size Change
babel-preset-env.js 2.77 MB 2.77 MB 148 bytes (0%)
babel-preset-env.min.js 1.67 MB 1.67 MB 46 bytes (0%)
babel.js 2.95 MB 2.95 MB 148 bytes (0%)
babel.min.js 1.63 MB 1.63 MB 46 bytes (0%)
test262.tap 4.84 MB [deleted]
@SakibulMowla SakibulMowla marked this pull request as ready for review Oct 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.