bs-platform@7.2.2 produces unreachable code in output of switch expression

[arnihermann]

Our project has fallen way behind on bs-platform releases so I did an upgrade to 7.2.2 (from 5.0.4). We noticed change in behavior in our web app after upgrading: a component that was rendered from a switch expression when compiled with bs-platform@5.0.4 was no longer being rendered. I opened the App.bs.js (see gist) when using bs-platform@7.2.2 and vscode immediately told me line 104 was unreachable code. This unreachable code is not created when using bs-platform@5.0.4.

I've documented the difference in output between 5.0.4 and 7.2.2 in gist: https://gist.github.com/arnihermann/60cc7b01901c94048d9b2b2fbef6e43d but I didn't want to wait with submitting the issue while I set up a reproducable example. vscode reports these lines as unreachable with bs-platform@7.2.2.

[bobzhang]

hi @arnihermann a small reproducible example is very helpful

[arnihermann]

I wasn't able to get an example up and running last night, for some reasons I didn't end up with same output for 1/ same data types but 2/ slightly different branch handling (e.g. had Js.logs instead of components). I'll resume this later and will post an example when I have one.

[bobzhang]

I guess some optimizations around pattern match is fishy, it would be helpful if you can narrow down the case a little bit

[bobzhang]

@arnihermann we would like to prioritise it and land a fix, but some more user input wold be helpful

[arnihermann]

@bobzhang I'm having a hard time creating a reproducible example. Even using copies of some of the files from our repo I'm unable to get the unreachable code output. I'm going to try a snapshot of our codebase with all files deletes except the files needed for rendering this component tree, and see if that works to trigger the issue. Please let me know if there's anything else I can do.

[bobzhang]

@arnihermann that would be helpful, thanks

[EduardoRFS]

I wrote a reproduction

Removing any case or even the Js.Console.log of the reproduction will make it works, this seems like the smallest reproduction that I can write

If you want to laugh change the variant name `SignIn to `A, and `SignUp to `B then you code became reachable

edit: changing the variant name makes it work

it seems like it is a specialcase for single letters, but it emits the right code

[cristianoc]

Looks like a misplaced return "Redirect" after several cases that set exit=.... As if this last default case took precedence over the previous overlapping cases:

| (`Unauthenticated, _) => <div> {React.string("Redirect")} </div>

[EduardoRFS]

I updated my reproduction comment, showing a case where the only difference is the variant name, and it causes it to work.

edit: oh it seems like there is a special case for single letters variant

[bobzhang]

thanks for narrow down the issue, can you confirm that it is fixed in #4298

[arnihermann]

Thanks @EduardoRFS for the test case 🙏

@bobzhang Confirmed, fixed for me.