hubot-slack icon indicating copy to clipboard operation
hubot-slack copied to clipboard

Coffeescript 2.x compatibility

Open mistydemeo opened this issue 7 years ago • 13 comments

Description

Describe your issue here.

What type of issue is this? (place an x in one of the [ ])

  • [x] bug
  • [ ] enhancement (feature request)
  • [ ] question
  • [ ] documentation related
  • [ ] testing related
  • [ ] discussion

Requirements (place an x in each of the [ ])

  • [x] I've read and understood the Contributing guidelines and have done my best effort to follow them.
  • [x] I've read and agree to the Code of Conduct.
  • [x] I've searched for any related issues and avoided creating a duplicate issue.

Introduction

The primary incompatibility between Coffeescript 1.x and 2.x is the class type. Coffeescript 1.x's object model is a custom one which compiles down to JS prototype-based objects, since Coffeescript 1.x predates JS having its own class model. Coffeescript 2.x classes instead compile down to ECMAScript 2015 classes in order to improve interoperability with modern JS. This required two major backwards-incompatible changes in order to comply with JS class requirements:

  1. super is now both a keyword and a method. This means that super, when called as a method, has to be called as super() in order to disambiguate.
  2. ECMAScript 2015 requires that super always be called before attributes are read or assigned within a constructor, so Coffeescript now has the same requirement. This means that constructors using the @ivar attribute name/assignment shorthand must call super() immediately before doing anything else.

While Hubot 3.x is compatible with Coffeescript 2.x, since Hubot itself is no longer written in CS, hubot-slack isn't yet compatible. I've outlined the compatibility testing I've performed with the problems I've identified.

Compatibility testing

Hubot 2.x + Coffeescript 1.x

:white_check_mark: Officially supported and compatible.

Hubot 3.x + Coffeescript 1.x

:white_check_mark: Officially supported and compatible.

Hubot 2.x + Coffeescript 2.x

:no_entry_sign: Not supported and not compatible. Hubot 2.x classes are written in Coffeescript before the release of 2.x.

Hubot 3.x + Coffeescript 2.x

:no_entry_sign: Supported, but not compatible with hubot-slack.

It looks like there's probably only one compatibility issue, but I need to test more:

  1. One use of super without arguments or parentheses: https://github.com/slackapi/hubot-slack/blob/55d41be0de9abdbde5db7e814a6f54680283f1f4/src/bot.coffee#L19

mistydemeo avatar Sep 06 '18 18:09 mistydemeo

Thanks for conducting this initial set of research amongst the variations of packages @mistydemeo 🙌

I'm going to classify this as an enhancement, because forward compatibility with a new major version of Coffeescript seems more like a new feature than it does a bug. Do you agree?

For full transparency, we haven't prioritized getting hubot-slack up to speed with Coffeescript 2.x, but I'm happy to contribute to the research when possible, and of course, merge and release contributions related to this. My fear is that this will end up requiring a major version bump, which could mean we delay the release of those changes since there's several other breaking changes we'd like to lump in.

aoberoi avatar Sep 06 '18 22:09 aoberoi

Any chance we could get some action on this? A move to 2.x would allow for async/await which would be huge

RJohnsonPGH avatar Oct 19 '18 15:10 RJohnsonPGH

@xxbiohazrdxx we don't have a working implementation at the moment, or any commitment from anyone to work on it. if you'd like to commit to that, i'd be happy to merge and release that change.

aoberoi avatar Oct 19 '18 21:10 aoberoi

This is something I'd like to do (I started in #528), but it's not a priority for me and I've got no timeline in which I'd expect to do it.

mistydemeo avatar Oct 20 '18 00:10 mistydemeo

Hi, I continue @mistydemeo works at https://github.com/slackapi/hubot-slack/pull/565 (Thank mistydemo for stared :bow:)

phucnh avatar May 06 '19 16:05 phucnh

@aoberoi Hi, could you please review https://github.com/slackapi/hubot-slack/pull/565 pull request :bow:

phucnh avatar May 14 '19 02:05 phucnh

This ticket could probably be closed because it is superseded by https://github.com/slackapi/hubot-slack/issues/429 ?

quinn avatar May 31 '19 02:05 quinn

This ticket could probably be closed because it is superseded by #429 ?

Agreed. I was just investigating why a Hubot 3.0 deploy still needed Coffeescript and I found this issue. Switching to JavaScript (#429) on the surface, looks simpler than untangling the many Hubot 2/3 Coffeescript 1/2 compatibility problems

crccheck avatar Jun 03 '19 03:06 crccheck

Will there be any movement on this either way? I'd like to use Coffeescript 2 in my hubot scripts, and this is holding me up

dholdren avatar Feb 26 '20 18:02 dholdren

From https://github.com/slackapi/hubot-slack/pull/528

Currently, this mostly works, but trying to actually launch Hubot fails with a TypeError: Cannot read property 'bind' of undefined error. I believe this may be because of the following: ES2015 classes don’t allow bound (fat arrow) methods. The CoffeeScript compiler goes through >some contortions to preserve support for them, but one thing that can’t be accommodated is >calling a bound method before it is bound

This is still unresolved even with #565

seratch avatar Apr 24 '20 12:04 seratch

I'm also experiencing this error when trying to write a basic Hubot test in Coffeescript, since line 12 of message.coffee (the super call for the constructor for ReactionMessage) is also throwing an error.

ERROR Unable to load /home/admin/hubot/scripts/my_script: /home/admin/hubot/node_modules/hubot-slack/src/message.coffee:21:11: error: Can't call super with @params in derived class constructors
    super @user 
          ^^^^^

I'm not familiar with Coffeescript, but I'm trying to figure out where in my script is actually invoking that, and it looks like it's doing that within either bot.coffee or my_script.coffee, but I don't know how to trace through the execution of a test with the debugger, so I can't walk through the code and find it just yet (I'm using Mocha/Chai/hubot-adapter). It throws this error when I try to load in my_script.coffee but I'm not directly invoking ReactionMessage anywhere in my script.

It may be something within my code, since I can get through the hubot-adapter test example just fine until I load in my module under test, which throws this error without giving me a full stack trace when invoked via mocha, even when specifying --full-trace. Any tips/advice?

EDIT - Was able to get the changes from #528 and #565 working locally; will provide Hubot and Coffeescript version numbers for compatibility in a bit.

nickiannone-tw avatar Apr 29 '22 16:04 nickiannone-tw

Looking at this from my other account; I think I can hack on this enough to get it working for the project I'm on, but I don't know if I have the time to contribute a full Coffeescript 2.x-compatible branch on my own. If anyone finds it useful, I'll get it committed to this PR branch, but I'm not expecting it to be reopened.

nickiannone avatar May 23 '22 00:05 nickiannone

so.. since https://github.com/slackapi/hubot-slack/pull/565 has been merged, what's still to do here..?

mdschmitt avatar Feb 27 '24 05:02 mdschmitt