Destructuring #1346

ahejlsberg · 2014-12-03T01:11:37Z

This pull request adds support for ECMAScript 6 destructuring declarations and assignments with down-level ES3 and ES5 code generation. For example:

var [x, y] = [10, 20];
[x, y] = [y, x];

The above example generates:

var _a = [10, 20], x = _a[0], y = _a[1];
_b = [y, x], x = _b[0], y = _b[1];

Another example:

interface NameAndLocation {
    name: string;
    location: [number, number];
}

function getNameAndLocation(): NameAndLocation {
    return { name: "top-left", location: [10, 20] };
}

function displayNameAndLocation({name, location: [x, y] = [0, 0]}: NameAndLocation) {
    console.log("name=" + name + " x=" + x + " y=" + y);
}

var {name, location: [x, y]} = getNameAndLocation();
displayNameAndLocation(getNameAndLocation());

The above example generates:

function getNameAndLocation() {
    return { name: "top-left", location: [10, 20] };
}
function displayNameAndLocation(_a) {
    var name = _a.name, _b = _a.location, _c = _b === void 0 ? [0, 0] : _b, x = _c[0], y = _c[1];
    console.log("name=" + name + " x=" + x + " y=" + y);
}
var _a = getNameAndLocation(), name = _a.name, _b = _a.location, x = _b[0], y = _b[1];
displayNameAndLocation(getNameAndLocation());

Work still remaining:

Support for rest elements in array destructuring.
Pure ES6 code generation with no rewriting (under -target ES6 switch).
Updated spec.

Conflicts: src/compiler/binder.ts src/compiler/checker.ts src/compiler/diagnosticInformationMap.generated.ts src/compiler/diagnosticMessages.json

Require RHS of array destructuring to be an actual array type (i.e. assignable to any[]) Tighten test for tuple type (previously just required a "0" property)

…ry names

Support .d.ts generation for functions with destructuring parameters

CyrusNajmabadi · 2014-12-03T10:35:53Z

src/compiler/checker.ts

        function checkBinaryExpression(node: BinaryExpression, contextualMapper?: TypeMapper) {
            var operator = node.operator;
+            if (operator === SyntaxKind.EqualsToken && (node.left.kind === SyntaxKind.ObjectLiteral || node.left.kind === SyntaxKind.ArrayLiteral)) {


this is repeated below in the emitter. Extract out a helper 'isDestructuringAssignment' function.

Conflicts: src/compiler/checker.ts src/compiler/diagnosticInformationMap.generated.ts src/compiler/diagnosticMessages.json src/compiler/emitter.ts src/compiler/parser.ts src/compiler/types.ts src/services/navigationBar.ts tests/baselines/reference/assignmentLHSIsValue.errors.txt tests/baselines/reference/objectTypesWithOptionalProperties.errors.txt tests/baselines/reference/parserErrorRecovery_ParameterList2.errors.txt

CyrusNajmabadi · 2014-12-04T04:04:52Z

One thing that's unclear to me is how this works with the 'helicoptering' type operations we use for LS operations.

CyrusNajmabadi · 2014-12-04T14:06:48Z

You may want to make a new branch for this, with your code copied into it. Right now it's difficult to make out the changes post master-merge (esp in Github). Even looking at the change in a diff tool that can handle the change is tough as your changes are now mixed into everyone else's who checked in in the last month.

ahejlsberg · 2014-12-04T19:37:53Z

@CyrusNajmabadi Not sure what you mean by the 'helicoptering' question, but certainly the code is written such that you can helicopter into something like var [[[a]]] = [[[3]]]; and ask for the type of a and we'll do the appropriate 'reaching up and over' to infer type number.

Regarding a new branch, not sure that would help any. To get an all up view of how this branch is different from the current master (without everyone's intervening edits) just use the "Files changed" tab at the top of this page (instead of the link to the commit for the merge with master).

CyrusNajmabadi · 2014-12-05T03:10:36Z

src/compiler/parser.ts

-            node.name = inGeneratorParameterContext()
-                ? doInYieldContext(parseIdentifier)
-                : parseIdentifier();
+            var savedYieldContext = inYieldContext();


Irony, i know: But this seems much more verbose than before.

however, that's not my major issue with this change. My primary concern is that by writing the code in this manner, it puts the onus on the maintainer to be sure that the saved state is restored properly. The prior form ensured that the state would always get saved/restored properly, and no changes could accidently affect that.

If you are concerned that you need to call parseIdentifierOrPattern with a parameter, and you don't want a lambda allocation, then this is a suitable way to write things:

node.name = inGeneratorParameterContext() ? doInYieldContext(parseParameterIdentifierOrPattern) : parseParameterIdentifierOrPattern(); ... function parseParameterIdentifierOrPattern() { return parseIdentifierOrPattern(SyntaxKind.Parameter); }

CyrusNajmabadi · 2014-12-05T03:28:08Z

@ahejlsberg That was indeed what i was referring to when i mentioned 'helicoptering'. Does this support fall out naturally from teh existing code we have that walks upwards/outwards? Or was new code necessary to support the destructuring case? If the latter, could you point out where i should look to find it?

Thanks!

ahejlsberg · 2014-12-05T15:01:07Z

@CyrusNajmabadi The 'helicoptering' ability is in a sense a core requirement because we need the ability to obtain the type of any variable (including one deeply buried in a destructuring declaration) at any time (including both regular compilation and language service use). So, you could say it naturally falls out of the code, but there wasn't really a choice to do it any other way.

The interesting functions to look at are getTypeForVariableDeclaration and getTypeForBindingElement. They recursively call each other to walk up the chain of parent binding patterns to finally arrive at the top declaration. The root type obtained for that declaration is then broken down as the recursion returns to the leaf binding element.

Further nuance is added by the way in which a binding pattern can imply a type when no annotation or initializer is available from which to infer, and the effects this has on contextual typing. The newly added comments on getWidenedTypeForVariableDeclaration explain how this works.

CyrusNajmabadi · 2014-12-05T16:58:02Z

@ahejlsberg Awesome. Thanks for the explanation. It's definitely helpful to understand how all that 'helicoptering in/out' works. BTW, do you have a better term to describe that process?

New SyntaxKind.BindingElement Introduced new VariableLikeDeclaration and BindingElement types Cleaned up VariableDeclaration, ParameterDeclaration, PropertyDeclaration types Node kind of binding element is always SyntaxKind.BindingElement Changed CheckVariableDeclaration to CheckVariableLikeDeclaration Reorganized CheckVariableLikeDeclaration

Move downlevel vs. ES6 emit branching into individual emit functions

mhegazy · 2014-12-09T18:17:44Z

👍

JsonFreeman · 2014-12-09T18:41:51Z

👍

Conflicts: src/compiler/binder.ts src/compiler/checker.ts src/compiler/emitter.ts src/compiler/parser.ts src/services/services.ts tests/baselines/reference/parserCommaInTypeMemberList2.errors.txt

Destructuring

JsonFreeman · 2014-12-09T21:37:54Z

src/compiler/checker.ts

+            // If no type was specified or inferred for parent, or if the specified or inferred type is any,
+            // infer from the initializer of the binding element if one is present. Otherwise, go with the
+            // undefined or any type of the parent.
+            if (!parentType || parentType === anyType) {


Shouldn't the type be any if the parent type was any?

It is if there is not an initializer. If you're suggesting it should be any regardless of the initializer, then no. Consider:

var a: any; var [x, y = 1] = a;

Here, a provides no clue as to the intended type of x and y, so we infer from the type of the initializer. This contrasts with:

var a: any; var [x, y = 1] = [a, a];

Here, the type of a is the intended type for y, and the initializer is required to be assignment compatible with that type.

Do we do that for contextual typing? An analogous example (courtesy of @CyrusNajmabadi):

var v: (a: any) => void = (a = 1) { a.blah };

Should a in the body be of type any or number? Applying the reasoning you stated from destructuring, it seems like in this example it should be number, not any.

For a parameter the order of precedence is this:

Explicit type annotation.

Contextual parameter type.

Initializer type.

For a binding element it is this:

Deconstructed type (i.e. the type of the corresponding property or array element in the deconstructed value).

Type of local default value (initializer) if type of deconstructed value is unknown or any.

So, you can't really apply the reasoning of one to the other. They're different kinds of constructs.

I understand that the way you described is how things are working, but it doesn't seem self explanatory why that is the case. Why are they different constructs?

I assume you agree with the already existing rules for parameters, so I won't try to elaborate on those.

For a destructuring element, the high order bit is the type of the value being deconstructed. Indeed, the initializer isn't even evaluated when a deconstructed value is available. So, this establishes the intuition that the binding element type should primarily come from the corresponding property or element in the deconstructed type. Then, in cases where the type of the deconstructed value isn't known or is any, it makes sense to take a clue from the initializer. That's the next best source of information.

That makes a lot of sense, but why is that not the case for parameters? I don't necessarily agree with the rules for parameters. I buy your argument about the destructuring and would like to apply it to parameters

ahejlsberg added 30 commits November 5, 2014 06:21

Initial parser support for destructuring patterns

86f290f

Rework tracking and error reporting related to widening of types

a01b440

Merge branch 'master' into destructuring

2d3752e

Adding binding patterns to declaration names

3d3212b

End-to-end support for destructuring in variable declarations

af00c71

Rudimentary support in language service

3cda261

Parser errors for destructuring declarations

bd65f16

Adding destructuring variable declaration error messages

a6401bb

Moving error reporting to getWidenedTypeForVariableDeclaration

7840f0d

Destructuring parameter declarations

850f3cb

Destructuring of tuple type cannot specify extra variables

2ed5f41

Merge branch 'master' into destructuring

58c3c2f

Conflicts: src/compiler/binder.ts src/compiler/checker.ts src/compiler/diagnosticInformationMap.generated.ts src/compiler/diagnosticMessages.json

Adding SyntaxKind.FunctionType/ConstructorType to isAnyFunction

d39749f

Support for destructuring assignments

607140b

Propagate node kind in variable/parameter destructuring declarations

38a2640

Allow string or numeric literal as property name of object binding

6b96386

Require RHS of array destructuring to be an actual array type (i.e. assignable to any[]) Tighten test for tuple type (previously just required a "0" property)

Fine tuning array and tuple type checks in destructuring

8ed1f24

Fixed bug in union type identity comparison

d8ecd8c

Accepting new baselines

98eaca5

Rewriting and emit for destructuring declarations

cf3e3ac

Emit of rest parameter for loop uses unique temporary variable name

28a73bc

Removing unused diagnostics related to '_i' variable name

ec7ce72

Accepting new baselines

501a370

Allow assignment expressions as destructuring assignment target

3ff2a62

Emit for destructuring assignments

9e63911

Baseline changes from updating rest parameter emit to use new tempora…

ab35da9

…ry names

Emit for destructuring parameter declarations

7994e90

Support exported destructuring variable declarations

0731a28

Support .d.ts generation for functions with destructuring parameters

Removing fourslash test that no longer applies

dc39de1

Fixing fourslash test

ecfcb9d

CyrusNajmabadi reviewed Dec 3, 2014
View reviewed changes

ahejlsberg added 2 commits December 4, 2014 17:43

Addressing CR feedback

b49e277

Accepting new baselines

4118ffc

CyrusNajmabadi reviewed Dec 5, 2014
View reviewed changes

ahejlsberg added 3 commits December 6, 2014 13:53

Addressing a bit more CR feedback

b3dffff

Merge branch 'master' into destructuring

459dee0

Move downlevel vs. ES6 emit branching into individual emit functions

ahejlsberg force-pushed the destructuring branch from 657f844 to 459dee0 Compare December 8, 2014 22:43

Merge branch 'master' into destructuring

7bc35b3

Conflicts: src/compiler/binder.ts src/compiler/checker.ts src/compiler/emitter.ts src/compiler/parser.ts src/services/services.ts tests/baselines/reference/parserCommaInTypeMemberList2.errors.txt

ahejlsberg added a commit that referenced this pull request Dec 9, 2014

Merge pull request #1346 from Microsoft/destructuring

bb70e9e

Destructuring

ahejlsberg merged commit bb70e9e into master Dec 9, 2014

ahejlsberg deleted the destructuring branch December 9, 2014 19:39

JsonFreeman reviewed Dec 9, 2014
View reviewed changes

Arnavion mentioned this pull request Apr 9, 2015

Support for object literal property value shorthand #418

Closed

microsoft locked and limited conversation to collaborators Jun 18, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Destructuring #1346

Destructuring #1346

ahejlsberg commented Dec 3, 2014

CyrusNajmabadi Dec 3, 2014

CyrusNajmabadi commented Dec 4, 2014

CyrusNajmabadi commented Dec 4, 2014

ahejlsberg commented Dec 4, 2014

CyrusNajmabadi Dec 5, 2014

CyrusNajmabadi commented Dec 5, 2014

ahejlsberg commented Dec 5, 2014

CyrusNajmabadi commented Dec 5, 2014

mhegazy commented Dec 9, 2014

JsonFreeman commented Dec 9, 2014

JsonFreeman Dec 9, 2014

ahejlsberg Dec 9, 2014

JsonFreeman Dec 9, 2014

ahejlsberg Dec 10, 2014

JsonFreeman Dec 10, 2014

ahejlsberg Dec 10, 2014

JsonFreeman Dec 10, 2014

Destructuring #1346

Destructuring #1346

Conversation

ahejlsberg commented Dec 3, 2014

Choose a reason for hiding this comment

CyrusNajmabadi commented Dec 4, 2014

CyrusNajmabadi commented Dec 4, 2014

ahejlsberg commented Dec 4, 2014

Choose a reason for hiding this comment

CyrusNajmabadi commented Dec 5, 2014

ahejlsberg commented Dec 5, 2014

CyrusNajmabadi commented Dec 5, 2014

mhegazy commented Dec 9, 2014

JsonFreeman commented Dec 9, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment