GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Destructuring by ahejlsberg · Pull Request #1346 · microsoft/TypeScript

Conversation

@ahejlsberg

@ahejlsberg Anders Hejlsberg (ahejlsberg) commented Dec 3, 2014

Copy link
Copy Markdown
Member

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Anders Hejlsberg (ahejlsberg) added 30 commits November 5, 2014 06:21
@ahejlsberg
Initial parser support for destructuring patterns
86f290f
@ahejlsberg
Rework tracking and error reporting related to widening of types
a01b440
@ahejlsberg
Merge branch 'master' into destructuring
2d3752e
@ahejlsberg
Adding binding patterns to declaration names
3d3212b
@ahejlsberg
End-to-end support for destructuring in variable declarations
af00c71
@ahejlsberg
Rudimentary support in language service
3cda261
@ahejlsberg
Parser errors for destructuring declarations
bd65f16
@ahejlsberg
Adding destructuring variable declaration error messages
a6401bb
@ahejlsberg
Moving error reporting to getWidenedTypeForVariableDeclaration
7840f0d
@ahejlsberg
Destructuring parameter declarations
850f3cb
@ahejlsberg
Destructuring of tuple type cannot specify extra variables
2ed5f41
@ahejlsberg
Merge branch 'master' into destructuring
58c3c2f
Conflicts:
	src/compiler/binder.ts
	src/compiler/checker.ts
	src/compiler/diagnosticInformationMap.generated.ts
	src/compiler/diagnosticMessages.json
@ahejlsberg
Adding SyntaxKind.FunctionType/ConstructorType to isAnyFunction
d39749f
@ahejlsberg
Support for destructuring assignments
607140b
@ahejlsberg
Propagate node kind in variable/parameter destructuring declarations
38a2640
@ahejlsberg
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)
@ahejlsberg
Fine tuning array and tuple type checks in destructuring
8ed1f24
@ahejlsberg
Fixed bug in union type identity comparison
d8ecd8c
@ahejlsberg
Accepting new baselines
98eaca5
@ahejlsberg
Rewriting and emit for destructuring declarations
cf3e3ac
@ahejlsberg
Emit of rest parameter for loop uses unique temporary variable name
28a73bc
@ahejlsberg
Removing unused diagnostics related to '_i' variable name
ec7ce72
@ahejlsberg
Accepting new baselines
501a370
@ahejlsberg
Allow assignment expressions as destructuring assignment target
3ff2a62
@ahejlsberg
Emit for destructuring assignments
9e63911
@ahejlsberg
Baseline changes from updating rest parameter emit to use new tempora…
ab35da9
…ry names
@ahejlsberg
Emit for destructuring parameter declarations
7994e90
@ahejlsberg
Support exported destructuring variable declarations
0731a28
Support .d.ts generation for functions with destructuring parameters
@ahejlsberg
Removing fourslash test that no longer applies
dc39de1
@ahejlsberg
Fixing fourslash test
ecfcb9d
CyrusNajmabadi
CyrusNajmabadi reviewed Dec 3, 2014
View reviewed changes
Comment thread src/compiler/checker.ts Outdated

@CyrusNajmabadi Cyrus Najmabadi (CyrusNajmabadi) Dec 3, 2014

Copy link
Copy Markdown
Contributor

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 is repeated below in the emitter. Extract out a helper 'isDestructuringAssignment' function.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@ahejlsberg
Merge branch 'master' into destructuring
816abb1
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

Cyrus Najmabadi (CyrusNajmabadi) commented Dec 4, 2014

Copy link
Copy Markdown
Contributor

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@CyrusNajmabadi

Cyrus Najmabadi (CyrusNajmabadi) commented Dec 4, 2014

Copy link
Copy Markdown
Contributor

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ahejlsberg

Anders Hejlsberg (ahejlsberg) commented Dec 4, 2014

Copy link
Copy Markdown
Member Author

Cyrus Najmabadi (@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).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Anders Hejlsberg (ahejlsberg) added 2 commits December 4, 2014 17:43
@ahejlsberg
Addressing CR feedback
b49e277
@ahejlsberg
Accepting new baselines
4118ffc
CyrusNajmabadi
CyrusNajmabadi reviewed Dec 5, 2014
View reviewed changes
Comment thread src/compiler/parser.ts Outdated

@CyrusNajmabadi Cyrus Najmabadi (CyrusNajmabadi) Dec 5, 2014

Copy link
Copy Markdown
Contributor

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.

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);
}

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@CyrusNajmabadi

Cyrus Najmabadi (CyrusNajmabadi) commented Dec 5, 2014

Copy link
Copy Markdown
Contributor

Anders Hejlsberg (@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!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ahejlsberg

Anders Hejlsberg (ahejlsberg) commented Dec 5, 2014

Copy link
Copy Markdown
Member Author

Cyrus Najmabadi (@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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@CyrusNajmabadi

Cyrus Najmabadi (CyrusNajmabadi) commented Dec 5, 2014

Copy link
Copy Markdown
Contributor

Anders Hejlsberg (@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?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Anders Hejlsberg (ahejlsberg) added 3 commits December 6, 2014 13:53
@ahejlsberg
Addressing CR feedback:
05c9966
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
@ahejlsberg
Addressing a bit more CR feedback
b3dffff
@ahejlsberg
Merge branch 'master' into destructuring
459dee0
Move downlevel vs. ES6 emit branching into individual emit functions
@ahejlsberg
Anders Hejlsberg (ahejlsberg) force-pushed the destructuring branch from 657f844 to 459dee0 Compare December 8, 2014 22:43
@mhegazy

Mohamed Hegazy (mhegazy) commented Dec 9, 2014

Copy link
Copy Markdown
Contributor

👍

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

1 similar comment
@JsonFreeman

Jason Freeman (JsonFreeman) commented Dec 9, 2014

Copy link
Copy Markdown
Contributor

👍

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ahejlsberg
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
Anders Hejlsberg (ahejlsberg) added a commit that referenced this pull request Dec 9, 2014
@ahejlsberg
Merge pull request #1346 from Microsoft/destructuring
bb70e9e
Destructuring
@ahejlsberg
Anders Hejlsberg (ahejlsberg) merged commit bb70e9e into master Dec 9, 2014
@ahejlsberg
Anders Hejlsberg (ahejlsberg) deleted the destructuring branch December 9, 2014 19:39
JsonFreeman
JsonFreeman reviewed Dec 9, 2014
View reviewed changes
Comment thread src/compiler/checker.ts

@JsonFreeman Jason Freeman (JsonFreeman) Dec 9, 2014

Copy link
Copy Markdown
Contributor

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@ahejlsberg Anders Hejlsberg (ahejlsberg) Dec 9, 2014

Copy link
Copy Markdown
Member Author

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 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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@JsonFreeman Jason Freeman (JsonFreeman) Dec 9, 2014

Copy link
Copy Markdown
Contributor

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.

Do we do that for contextual typing? An analogous example (courtesy of Cyrus Najmabadi (@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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@ahejlsberg Anders Hejlsberg (ahejlsberg) Dec 10, 2014

Copy link
Copy Markdown
Member Author

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.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@JsonFreeman Jason Freeman (JsonFreeman) Dec 10, 2014

Copy link
Copy Markdown
Contributor

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.

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?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@ahejlsberg Anders Hejlsberg (ahejlsberg) Dec 10, 2014

Copy link
Copy Markdown
Member Author

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.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@JsonFreeman Jason Freeman (JsonFreeman) Dec 10, 2014

Copy link
Copy Markdown
Contributor

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@Arnavion Arnav Singh (Arnavion) mentioned this pull request Apr 9, 2015
Support for object literal property value shorthand #418
Closed
@microsoft Microsoft (microsoft) locked and limited conversation to collaborators Jun 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@ahejlsberg @NoelAbrahams @CyrusNajmabadi @mhegazy @JsonFreeman
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.