Coffeescript 2.x compatibility

mistydemeo commented 6 years ago

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:

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.
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:

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

aoberoi commented 6 years ago

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.

xxbiohazrdxx commented 5 years ago

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

aoberoi commented 5 years ago

@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.

mistydemeo commented 5 years ago

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.

phucnh commented 5 years ago

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

phucnh commented 5 years ago

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

quinn commented 5 years ago

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

crccheck commented 5 years ago

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

dholdren commented 4 years ago

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

seratch commented 4 years ago

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

nickiannone-tw commented 2 years ago

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 commented 2 years ago

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.

mdschmitt commented 7 months ago

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

slackapi / hubot-slack