Destructuring declarations that bind nothing should probably be an early error

[allenwb]

In the ES6 spec., there is no check that a destructuring let/const/var actually introduces any bindings.

For example:

let ;

is a syntax error, but (updated 10/22/15)

let { } = obj;

is valid syntax that does nothing (it doesn't add any new variable bindings).

There are reports that some ES6 developers are encountering bugs cause by complex destructing patterns that unintentionally don't introduce any bindings.

This seems like a design bug in ES6. It would be trivial to specify that this is an early syntax error.

In 13.3.1.1 LexicalBinding : BindingPattern Initializer

• It is a Syntax Error if the BoundNames of BindingPattern has no elements.

In 14.1.2 FormalParameter : * BindingElement*

• It is a Syntax Error if the BoundNames of BindingElement has no elements.

Note that this is a breaking change for any program that actually contains such empty binding patterns. So there is some risk to making this change.

[bterlson]

On the surface fixing this bug seems like something we could get away with while there are no interoperable destructuring implementations. Nice find!

[nzakas]

Oops, just sent an email to esdiscuss before I saw this. Yay! :)

[bterlson]

@nzakas thanks much for pointing this out :) Definitely file bugs for any other issues you find!

[bterlson]

@allenwb what about non-binding forms (AssignmentPattern and friends)? Should [] = [] be an error? I say so...

[bterlson]

I also note that let {} and let [] are syntax errors at least since the initializer is not optional.

[getify]

I have strong concerns about this proposal:

1. I have on two occasions already had the need (during debugging, etc) to do things like this:

   var {
   // x,
   // y
   } = foo();

   ...where I comment out different parts of the destructuring declaration, and in some cases even comment out all of them.

   It would really suck if this practice of commenting them out during debugging and dev would work only while there was at least one left uncommented, but if I needed them all commented out, I'd have to re-arrange the code by commenting out the entire destructuring pattern part, but (for example) maybe still need to call the foo().

   In that scenario, this seems at best surprising and at worst rather hostile.

2. What about array destructuring? Will this proposal disallow this?

   var [
   // a,
   // b
   ] = foo()

   ... in the same way? Or what about?

   var [ , , ] = foo()

   ...if you have a stateful iterator on an array arr, you may want to do:

   var [ , , ] = arr;
   
   // later
   var [x, y] = arr;

   ...where you're expecting the [ , , ] to basically advance the stateful iterator by two steps, such that x and y are assigned from positions 2 and 3, respectively.

It sounds nice to assume that { }, [ ] and [ , , ] destructuring patterns are mistakes by developers, but that's just not a safe assumption.

Since @nzakas has already landed a linter rule for eslint for catching these things, I think that is the better path. Let tooling catch/warn about these, but don't make the language prevent these potentially not-mistake cases.

[rossberg]

I disagree with this proposal. It creates an irregularity and makes life harder for code generators, but also for people who want to comment out parts of a pattern. I don't think there is any real win either. Moreover, as Brian pointed out, destructuring without a RHS is an error already.

[nzakas]

I think [] = [] should definitely be an error.

Another scary pattern is this:

let { loc: [] } = foo;

@rossberg-chromium this isn't just about variable declarations, there are also assignments. You'd still be able to comment out parts of a pattern, just not every part at once. You're also missing this use case completely:

let { foo: {} } = bar;

@getify I'm having a hard time understanding why commenting out every property at once is useful for debugging. Isn't this:

var {
// x,
// y
} = foo();

The same as:

// var {
// x,
// y
// } = foo();

?

[caitp]

Isn't this:

var {
// x,
// y
} = foo();

The same as:

// var {
// x,
// y
// } = foo();

?

The RHS is evaluated before the binding pattern is walked, so foo() is still invoked, regardless of whether any bindings are created

[nzakas]

@caitp yes, but unless you're relying on side effects, that difference is inconsequential.

[nzakas]

Assuming goes both ways. :)

In any event, inability to comment out code just the way you want it doesn't seem like it should be a blocker for this. We all have editors, most have shortcut keys for inserting comments. If you need to use two keystrokes instead of one to help prevent unnecessary errors that are hard to find, that seems like a good tradeoff (IMHO). You can always move the RHS of an assignment to the next line temporarily and get the same effect.

[getify]

intentional side effects sometimes are as important but yet benign as console.log statements inside foo(), again for debugging purposes.

[jeffmo]

There's always...

/*var {
// x,
y
} =*/ foo();

Clarifying edit: I don't feel strongly about this, but I also don't think that changes to the ability to single-line-comment at dev time for some rare set of cases is all that compelling a reason not to do this. I mean if this is really an issue, what do you do when some code does var {x} = foo();? Or even just var x = foo();?

[getify]

you can always move the

yes of course you can. but my point is I shouldn't have to.

you make the one-sided assumption that my case ought to require me to type/edit more. i'll reiterate my similarly one-sided assumption/assertion from earlier: if you want protection from this "mistake" in your code, you should have to use a smart tool to protect you. but don't force that assumption on the rest of us.

[rwaldron]

FWIW, I agree with @getify and @rossberg-chromium for the same reasons they've stated. Please don't take this as dog-piling, as I will gladly argue against this at the next meeting ;)

[bterlson]

Can someone give a little more detail about how removing a binding is useful for debugging purposes? It seems unlikely that removing a binding would have an interesting effect on a program.

Also, it's clear this needs consensus!

[rwaldron]

@bterlson, less about the specific thing you might do and more about the strange error you'll encounter when doing perfectly reasonable things during development/debugging. In fact, you might want to eliminate all bindings to observe the outcome—it doesn't really matter why, it's just something a developer might do, and I think the language doesn't need to prohibit it just because it can. This is the sort of thing that dev tools (i.e. linters) can handle—if the end developer wants to.

[getify]

in one of my cases, the properties I was destructuring were getters. the side effects were noisy console.log statements that I wanted to avoid by simply commenting out the assignment item in the pattern.

[ljharb]

When commenting out every destructuring property in an object destructuring, could you not just add a nonexistent garbage property, and still do your debugging without having to worry about whether "no destructuring patterns" are an error?

I think the real question here is, which is going to be more common: that an empty-bindings destructuring pattern is a developer mistake (worth noting that things like the TDZ attempt to address developer mistakes, and I think this is indeed the language's job), or, that an empty-bindings destructuring pattern is a temporary thing that an educated developer is doing for debugging?

I tend to believe that both the former case is more common, and that in the latter case, the developer will not have a practical problem avoiding this error during debugging.

@rwaldron @getify @rossberg-chromium, do you really think both that the former (a newbie developer making a mistake) is far less common than a developer needing to debug destructuring with things that have side effects such that they need to comment out every destructuring binding, and that discontinuing to allow the latter would be excessively burdensome on an experienced developer during their debugging?

[bterlson]

@rwaldron I'm not arguing this should be prohibited just because it's possible to prohibit. I see it as a mistake people might appreciate an error for like omitting other BindingIdentifiers. The error seems especially hard to spot in nested forms, and with initializers it gets harder still. But if this isn't a valuable error then I agree no reason to throw it.

[getify]

@bterlson

if this isn't a valuable error then I agree no reason to throw it.

that's not really the question, IMO. is it useful? it seems likely it might be useful to some people in some cases. but it's also something that clearly it's something that linting tools can easily check for, giving developers an opt-in for that checking, rather than locking developers into it by language design.

I think adding a lock-out to the language design deserves a higher bar than "useful/valuable error to some" (or even "many").

[getify]

@ljharb i don't think it's fruitful to have a guessing-match at who's got the corner on more common. it's clear what you and nicholas think, but i don't agree. shrugs.

however, no one seems to be addressing my second use-case above... the [ , , ] array-destructuring used to advance the cursor of a persistently-stateful generator. I suppose you'll also claim "very few will ever do that".

so i guess my only counter-argument is that this proposal has only presented one kind of use-case where the mistake happens (nested destructuring where they typed : instead of =). and i've presented two counter-arguments from what it unduly restricts.

[bterlson]

You can use linting tools to find errors, but I believe some errors are helpful enough to throw anyway. Regardless, I am not strongly in favor of making this an error (we allow many empty braces in various places already). I can see the rationale for keeping the current semantics.

[anba]

Similar to @getify's [ , , ] example for destructuring assignment, I've also already used [...[]] = iterable to drain an iterable. IMO a quite handy shortcut when working on the REPL compared to the more verbose for (let ignored of iterable) ;.

[rossberg]

@ljharb:

When commenting out every destructuring property in an object destructuring, could you not just add a nonexistent garbage property, and still do your debugging without having to worry about whether "no destructuring patterns" are an error?

Sure, but why require such extra tedium?

I think the real question here is, which is going to be more common: that an empty-bindings destructuring pattern is a developer mistake (worth noting that things like the TDZ attempt to address developer mistakes, and I think this is indeed the language's job), or, that an empty-bindings destructuring pattern is a temporary thing that an educated developer is doing for debugging?

Aside: TDZ is not just to prevent mistake, but to have meaningful behaviour for something like const declarations, and to be future-proof regarding other possible extensions to declarations (e.g. guards were under discussion at the time).

In most other places, JavaScript consciously does not help the programmer avoiding mistakes, quite the opposite in fact. Special case rules and irregularities are generally bad -- principle of least surprise, refactoring, compositionality, etc. Before introducing another such case I'd like to see strong evidence that it actually addresses a real problem, i.e., that the supposed mistake is a common (and difficult to diagnose) error in practice. I find that very hard to believe.

[jeffmo]

So yea, the fact that empty destructuring itself can incur side effects (i.e. draining an iterator) now seems to me like a compelling reason to not do this.

[nzakas]

@rossberg-chromium we got several reports of this error on ESLint:

let { foo: {} } = bar;

Here's one of the issues: https://github.com/eslint/eslint/issues/4067

I'm personally less worried about patterns like let {} = foo because it seems more obvious that there may be a problem. The more insidious pattern is the one I mentioned above, as it's unclear that this is most likely a mistake.

[allenwb]

Much of the above involves recurring points of disagreement that have come up in many ES design discussions. Here is how I personally prioritize such issues:

1. All other things being equal, language should help humans write correct code.
2. The language should be helpful to humans who need to debug incorrect code, but not by sacrificing 1
3. The language should be friendly to mechanical code generators, but not by sacrificing 1. or 2.

I think these criteria should guide all of TC39's decisions.

This proposal changed is saying that the current spec. missed a point 1 detail about this feature.and @nzakas is reporting user experience to back that up.

[jeffmo]

@allenwb: Agreed on all points, but the most compelling reason not to do this at this point is the var [...[]] = iter; use case. var = foo; is not useful in this sense and thus it makes a lot of sense to error.

The explicit act of opening up the RHS via destructuring, however, is a useful feature.

I don't believe the dev-time commenting or codegen issues are that compelling, but I do think the short-hand mutation case is. A linter can easily be more restrictive here if that's what the developer desires, but if the lang restricts then there are no options at all for the dev.

[allenwb]

If you really want to exercise the side-effects of destructuring, the appropriate way to do so would be to use a destructuring assignment, not a destructuring declaration. E.G.:

{ foo: {} } = bar;

The primary purpose of let/const/var is to introduce new scoped bindings. If you write one that introduces no binding you have made a mistake: either you shouldn't have use a declaration form or one or more identifiers are missing.

[jeffmo]

I guess that seems reasonable at first thought...though it would mean that assignment-form and declaration-form destructuring would fall out of consistency in this sense, though.

[allenwb]

@getify said:

...if you have a stateful iterator on an array arr, you may want to do:

var [ , , ] = arr;

// later
var [x, y] = arr;

...where you're expecting the [ , , ] to basically advance the stateful iterator by two steps, such that x and y are assigned from positions 2 and 3, respectively.

That is not correct (or you are using a implementation of destructuring that doesn't conform to the spec). See 13.3.3.5, rules for BindingPattern : ArrayBindingPattern, step 5:

5 If iteratorRecord.[[done]] is false, return IteratorClose(iterator, result).

Array destructoring closes (by invoking the return method) any iternators it does not exhaust.

[allenwb]

@jeffmo

I guess that seems reasonable at first thought...though it would mean that assignment-form and declaration-form destructuring would fall out of consistency in this sense, though.

Not really, the proposed early error is part of the let/const/var/parameters static semantics, not the pattern semantics . It is saying something about declarations, not about patterns.

Regardless, there are many other differences between assignment patterns and declaration patterns semantics.

[getify]

@allenwb:

Here's my simple example of what I meant by "stateful iterator":

var a = [1,2,3,4,5], idx = 0;
a[Symbol.iterator] = function*() {
  while (idx < this.length) {
    yield a[idx++];
  }
};

var [,,] = a;
var [x,y] = a;

console.log(x,y);
// should be:
// 3 4

That shows "3 4" in Firefox.

[getify]

if you really want to exercise the side-effects of destructuring, the appropriate way to do so would be to use a destructuring assignment, not a destructuring declaration

Why on earth would we allow:

[,,] = foo()

But disallow:

var [,,] = foo()

That I would find highly surprising.

[UltCombo]

@getify Allen has already explained:

The primary purpose of let/const/var is to introduce new scoped bindings. If you write one that introduces no binding you have made a mistake: either you shouldn't have use a declaration form or one or more identifiers are missing.

[getify]

To the point of my snippets, couldn't someone just as easily make the argument that [] = foo() is as much a "mistake" as var [] = foo()?

Inconsistency is often the death of learning.

My stance on the main proposal is already stated. But secondarily, I would also argue against inconsistency. If you feel strongly enough to disallow var [,,] = foo(), I think you should accept disallowing [,,] = foo(), too.

[allenwb]

@getify A generator with external state dependencies like that seems is probably something that should not be encouraged

[getify]

@allenwb of course that was a quick snippet PoC. as you're aware, there are many, many ways that developers can build on side-effects. Despite not liking side-effects, it happens. I don't encourage side-effects, but I'm also not dogmatic enough to suggest the language should go to extra lengths to prevent you from doing so.

[bterlson]

I somewhat contest that the assignment case should allow no identifiers if the binding patterns don't. It's certainly not clear that this is the "proper" way to do it. Wouldn't the same arguments would apply to making the empty assignment forms an error as well. [...[]] = iter is a pretty neat pattern, though not often useful (and perhaps could be solved in other ways, eg. by allowing bindingless rest ala [...] = iter, or by saying forms that pull from iterators like rest and patterns with elisions are allowed to omit bindings).

[mohsen1]

What if I'm only triggering getters by let { bar: {} } = foo; for some reason?

'use strict';

let foo = {
    get bar() {
        console.log('...');
    }
}

let { bar: {} } = foo;

I agree that's a horrible code, just saying it can be valid...

[allenwb]

@mohsen1

What if I'm only triggering getters by let { bar: {} } = foo; for some reason?

Loose the let (ecause you aren't declaring any variables) and just say:

{ bar: {} } = foo;

[nzakas]

I'm not sure I understand the distinction between declarations and assignments, the error case is the same in both. In the former, the intent is to create new bindings but you fail to do so, in the latter, the intent is to assign a value and you fail to do so.

And what about destructured function arguments? It seems like you can be bitten by this in numerous places, and I'm not sure I can rationalize why you'd get a safety net for one place but not others.

[getify]

@allenwb of course, that doesn't just work as cited, because then you'll have to remember to put the ( ) around the whole assignment...

...which is not only unnecessary but also invalid when the 'let' is present.

[allenwb]

@getify oops, of course. since it is just being evaluate for effect, maybe:

0,{ bar: {} } = foo;

[allenwb]

@nzakas

I'm not sure I understand the distinction between declarations and assignments,

The big difference I see is that declarations have non-local effect upon the static meaning or validity of parts of a program. For example, a declaration can shadow a declaration that exists in an outer scope and reference in an inner scope. A declaration can also make other declaration invalid (duplicate names). An assignment pattern does not have such non-local static effects.

In my proposal at the top of this thread, I included a non-bindings check for destructuring function parameters.

[rossberg]

@allenwb, you can interpret the argument that some of us have made as: this change is potentially detrimental even to your criterion 1 ("All other things being equal, language should help humans write correct code."). I mentioned refactoring and principle of least surprise as reasons.

[nzakas]

@allenwb of your guiding principle is writing correct code, the local vs. nonlocal effects should be secondary to the fact that the assignment didn't actually assign, shouldn't it?

@rossberg-chromium you're arguing that principle of least surprise says { bar: {} } = foo; not doing an assignment is not surprising?

[rossberg]

@nzakas, no, I don't see what's surprising about that (except for somebody who doesn't know destructuring). Destructuring binds 0..N variables, or assigns 0..N mutables. A needless discontinuity between 0 and 1..N is surprising (and annoying).

[ljharb]

I feel like many developers see destructuring assignment as simply sugar for assigning to variables - var/let/const only ever creates 1 binding, so 1..N for destructuring assignment seems like a very logical step. From my perspective, allowing zero bindings is the discontinuity.