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

Switch Case wrong syntax not warned? #1157

Closed
wheelhot opened this issue Jun 27, 2018 · 5 comments

Comments

@wheelhot
Copy link

commented Jun 27, 2018

Here's an example of what I meant

    switch(fruits) {
        case fruits = "Banana":
            text = "Banana is good!";
        break;
        case fruits = "Orange":
        text = "I am not a fan of orange.";
        break;
        case fruits = "Apple":
        text = "How you like them apples?";
        break;
        default:
        text = "I have never heard of that fruit...";
    }

Notice that the case syntax doesn't follow official guideline

And it doesn't get any sort of warning in VSCode with StandardJS installed.

The odd thing is, and something I hope someone would be able to answer me is: Why does this code still works on all browsers except Safari? And there's an additional interesting thing that happens in Safari as you can see below


This is how it works in Chrome
chrome copy


This is how it works in Safari
safari copy


And here's the weird part, if I turn on Safari Web Inspector, it works, but turn it off and it stops working
safari webinspector copy


So what's the reason and is there a way to config StandardJS to prevent this from happening?

And here's a JSFiddle for those who want to see the code in action yourself. Thanks, w3 Schools for the sample code

@LinusU

This comment has been minimized.

Copy link
Member

commented Jun 27, 2018

Why does this code still works on all browsers except Safari?

I actually have a hard time telling what the code should do, and I'm not sure which browser is doing the correct thing.

You are actually assigning to fruits in each switch case.

I'm guessing that you want something like this:

    switch(fruits) {
-       case fruits = "Banana":
+       case "Banana":
            text = "Banana is good!";
        break;
-       case fruits = "Orange":
+       case "Orange":
        text = "I am not a fan of orange.";
        break;
-       case fruits = "Apple":
+       case "Apple":
        text = "How you like them apples?";
        break;
        default:
        text = "I have never heard of that fruit...";
    }

That being said, I'm actually surprised that Standard doesn't warn about this, and it would be very nice to fix that.

@LinusU

This comment has been minimized.

Copy link
Member

commented Jun 27, 2018

Just tested on the "Try standard" feature of the site, the following passes without errors:

var fruit = 'banana'

switch (fruit) {
  case fruit = 'apple':
    console.log('it was apple')
    break
  case fruit = 'banana':
    console.log('it was banana')
    break
  case fruit = 'cucumber':
    console.log('it was cucumber')
    break
  default:
    console.log('i don\'t know that fruit')
}
@wheelhot

This comment has been minimized.

Copy link
Author

commented Jun 27, 2018

I actually have a hard time telling what the code should do, and I'm not sure which browser is doing the correct thing.

Okay, let me rephrase my points:

  1. The above code with case fruits = "Banana": works fine on all browsers except Safari
  2. You can test it by typing the case string in the text box in both Safari and Chrome and you'll notice that it doesn't work in Safari
  3. Turn on Safari Web Inspector and it'll work again

So my question was:

  1. Why does this wrong syntax, somehow work on all browsers except Safari? (when it should not work at all)
  2. why doesn't StandardJS warn that I used the wrong syntax format (hence avoiding all this issue, to begin with)

You are actually assigning to fruits in each switch case.

Yeah, I realised that but when I code I didn't realise that mistake and no errors on Chrome but when I tested in Safari, things didn't work and turning on Safari Web Inspector makes the code working again, so ended up spending more time figuring out what went wrong only to have someone point out that I got my syntax wrong. So I figure it's best to share my findings and see what people think about it. I know it's trivial, but I'm curious on why

I'm guessing that you want something like this:

Yes :)

@LinusU

This comment has been minimized.

Copy link
Member

commented Jun 27, 2018

Why does this wrong syntax

It's not technically the wrong syntax. You are allowed to put any expression in a case statement, and assignment is a valid expression.

A similar trick can be used to simulate pattern matching:

let bugs = 99

switch (true) {
  case (bugs < 10):
    console.log('Pretty good code')
    break
  case (bugs < 50):
    console.log('Code review, anyone?')
    break
  case (bugs < 100):
    console.log('🙈')
    break
  default:
    console.log('I don\'t even')
}

works fine on all browsers except Safari

I'm still not convinced which browser is "correct".

You are seeing different behavior in Safari or Chrome because they are evaluating the case expressions at a different point in time. This is potentially defined in the ECMAScript language spec...

@stale

This comment has been minimized.

Copy link

commented Sep 25, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale label Sep 25, 2018

@stale stale bot closed this Oct 2, 2018

@lock lock bot locked as resolved and limited conversation to collaborators Dec 31, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
2 participants
You can’t perform that action at this time.