Skip to content

Short circuit operator breaks TSC #25897

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

Closed
LiterallyDoge opened this issue Jul 24, 2018 · 8 comments
Closed

Short circuit operator breaks TSC #25897

LiterallyDoge opened this issue Jul 24, 2018 · 8 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@LiterallyDoge
Copy link

TypeScript Version: This is a compiler syntax issue across all versions.

Search Terms:
short circuit arrow function

Code

var functionHandler = {
    handleIt: () => {
        console.log("HANDLE IT MAN!  HANDLE IT!");
    }
};

var nextFunctionHandler = {
    handleIt: null
};

nextFunctionHandler.handleIt = functionHandler.handleIt || () => {
    console.log("We didn't handle it.");
};

Expected behavior:
The short-circuit operator should be agnostic to which type of function follows it (arrow versus traditional).

Actual behavior:
Traditional functions resolve as expected, but arrow functions compile malformed: "();"

Playground Link: https://www.typescriptlang.org/play/#src=class%20Greeter%20%7B%0D%0A%20%20%20%20greeting%3A%20string%3B%0D%0A%20%20%20%20constructor(message%3A%20string)%20%7B%0D%0A%20%20%20%20%20%20%20%20this.greeting%20%3D%20message%3B%0D%0A%20%20%20%20%7D%0D%0A%20%20%20%20greet()%20%7B%0D%0A%20%20%20%20%20%20%20%20return%20%22Hello%2C%20%22%20%2B%20this.greeting%3B%0D%0A%20%20%20%20%7D%0D%0A%7D%0D%0A%0D%0Alet%20greeter%20%3D%20new%20Greeter(%22world%22)%3B%0D%0A%0D%0Alet%20button%20%3D%20document.createElement('button')%3B%0D%0Abutton.textContent%20%3D%20%22Say%20Hello%22%3B%0D%0Abutton.onclick%20%3D%20function()%20%7B%0D%0A%20%20%20%20alert(greeter.greet())%3B%0D%0A%7D%0D%0A%0D%0Avar%20functionHandler%20%3D%20%7B%0D%0A%20%20%20%20handleIt%3A%20()%20%3D%3E%20%7B%0D%0A%20%20%20%20%20%20%20%20console.log(%22HANDLE%20IT%20MAN!%20%20HANDLE%20IT!%22)%3B%0D%0A%20%20%20%20%7D%0D%0A%7D%3B%0D%0A%0D%0Avar%20nextFunctionHandler%20%3D%20%7B%0D%0A%20%20%20%20handleIt%3A%20null%0D%0A%7D%3B%0D%0A%0D%0AnextFunctionHandler.handleIt%20%3D%20functionHandler.handleIt%20%7C%7C%20()%20%3D%3E%20%7B%0D%0A%20%20%20%20console.log(%22We%20didn't%20handle%20it.%22)%3B%0D%0A%7D%3B%0D%0A%0D%0Avar%20justSomeJavascript%20%3D%20function%20()%20%7B%0D%0A%20%20%20%20%2F%2F%20haha%0D%0A%7D%3B%0D%0A%0D%0Avar%20justSomeJavascript%20%3D%20()%20%3D%3E%20%7B%0D%0A%20%20%20%20%2F%2F%20hah%0D%0A%7D%3B%0D%0A%0D%0A%0D%0A%0D%0Adocument.body.appendChild(button)%3B

Related Issues: I didn't.

Thanks in advance.

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Jul 24, 2018

There's a parse error and that's why it's not emitting correctly. The specification doesn't expect a plain arrow function to follow a ||. Try it in a browser's console window.

Here's what I get in Firefox:

> "" || () => {}

SyntaxError: invalid arrow-function arguments (parentheses around the arrow-function may help)

Here's what I get in Chrome:

> "" || () => {}

VM56:1 Uncaught SyntaxError: Unexpected token )

@DanielRosenwasser DanielRosenwasser added the Working as Intended The behavior described is the intended behavior; this is not a bug label Jul 24, 2018
@DanielRosenwasser
Copy link
Member

But Firefox gave a good error message. We should too.

@DanielRosenwasser DanielRosenwasser changed the title Short Circuit Assignment of Arrow Function Breaks TSC Poor error message for arrow function in invalid locations Jul 24, 2018
@DanielRosenwasser DanielRosenwasser added Domain: Error Messages The issue relates to error messaging and removed Domain: Error Messages The issue relates to error messaging labels Jul 24, 2018
@DanielRosenwasser DanielRosenwasser changed the title Poor error message for arrow function in invalid locations Short circuit operator breaks TSC Jul 24, 2018
@LiterallyDoge
Copy link
Author

I respect your authority but this is plainly wrong.

@DanielRosenwasser
Copy link
Member

Can you show me what in https://tc39.github.io/ecma262/ indicates that this behavior is wrong?

@calebsander
Copy link
Contributor

calebsander commented Jul 24, 2018

@DanielRosenwasser is correct. If you surround the arrow function in parentheses, it compiles fine. This is just an operator precedence issue; without parentheses, it is parsed as (functionHandler.handleIt || ()) => { console.log("We didn't handle it."); };

@LiterallyDoge
Copy link
Author

LiterallyDoge commented Jul 24, 2018

Thanks for the ask. Two thoughts on this:

  1. MDN references odd operator precedence for this behavior, citing PEMDAS as a fix, but a lambda/arrow function/whatever is not acting as any type of operator for purposes of PEMDAS in this case, nor is it concatenating or behaving in any way similar (see: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Arrow_functions under "Parsing Order"). Because of this, from a developer's perspective, it is literally just a function closure by another name, which behaves predictably when give as a logicalAndExpression to the rref of the || operator when used during short-circuit assignment. Therefore it can only logically behave in the same way that a closure would.

  2. In terms of the linked document, I would say that the line on the || operator at the foot of section 12.13 is fairly clear as to its intent:
    "The value produced by a && or || operator is not necessarily of type Boolean. The value produced will always be the value of one of the two operand expressions."
    And that in this case, it can be argued that an arrow function is just as much a valid LogicalANDExpression as any closure.

So from the perspective of a human, or of that document, in my eyes, the argument I've put forth initially, that || should behave agnostically to closures versus lambads, does hold up.

Thank you for your time in any case.

PS: By the way I love this compiler.

@bterlson
Copy link
Member

bterlson commented Jul 24, 2018

@LiterallyDoge I just want to point out that this it doesn't have to do with the types of the operands (you could put your function in a var or wrap it in parens and || will do as you expect modulo type errors). This is how JS syntax works. Per the specification, a LogicalORExpression's right-hand operand is a LogicalAndExpression. The only way to get an arrow function is to parenthesize it. This is true for any alternative of AssignmentExpression, e.g. you can't do x || y = 1 either.

@typescript-bot
Copy link
Collaborator

Automatically closing this issue for housekeeping purposes. The issue labels indicate that it is unactionable at the moment or has already been addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

5 participants