Nodes: Class body passthrough expressions get hoisted if anything in a class is hoisted

[stevefan1999-personal]

http://tinyurl.com/yb476vtn
Although it was still a legit prototype construct, this is very problematic for backtick embeded JS expression.
http://tinyurl.com/ya7xo5fz
The entire qux = 10 was shifted outside of the class declaration.
I do believe this is a compiler bug.

Workaround:
http://tinyurl.com/y7gzwfup for backtick property-based apply/or
http://tinyurl.com/y8xa58a6 for constructor apply

*PS: backtick property-based apply will still be boil down to constructor apply, tested with babel online repl: http://tinyurl.com/yd94hv4y

[stevefan1999-personal]

I thought MobX was cool with CS2 with relaxation on backtick JS expression generation, turns out I was wrong,

[GeoffreyBooth]

There’s a lot going on in your post, so I’m not sure exactly what bug or bugs you’re referring to. But I did find at least one issue in your examples, that I’ve simplified to the minimal case:

class Class
  `field = 3`
  hoisted: 4

var Class;

Class = (function() {
  class Class {
    ;

  };

  field = 3;

  Class.prototype.hoisted = 4;

  return Class;

})();

It appears that whenever anything in a class is hoisted, all passthrough expressions get improperly hoisted as well. If you change hoisted: 4 above to hoisted: -> 4 instead, the backticked block appears in the class body as desired.

[connec]

I will look into this asap.

[GeoffreyBooth]

@connec I noticed that I still had my original “backticked expressions in class body” branch (the one you refactored for #4668 / #4712) in my local repo. Out of curiosity, I checked out my old branch, where I left off before you started working in it, and I tried the minimal case above. To my surprise, my old branch compiles it correctly:

var Class;

Class = (function() {
  class Class {
    field = 3;
  };

  Class.prototype.hoisted = 4;

  return Class;

})();

I pushed the branch up: https://github.com/GeoffreyBooth/coffeescript/tree/backticked-expressions-in-class-body and added a test for this case.

I assume you probably want to start with current master, but maybe there’s something in this old branch that you might find useful? At least you can pluck GeoffreyBooth@1fdd6df to get the test.

[stevefan1999-personal]

I never believed it would involve hoisting, one of the nightmares of JS.

[connec]

@GeoffreyBooth / @stevefan1999 turned out to indeed be a bug in the hoisting logic that was pretty easy to fix - we were setting hoisted on the Value wrapping a PassthroughLiteral, and then unwrapping that Value before checking for the hoisted flag 😓