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

Coffeescript 2.x compatibility #526

Open
4 of 9 tasks
mistydemeo opened this issue Sep 6, 2018 · 13 comments
Open
4 of 9 tasks

Coffeescript 2.x compatibility #526

mistydemeo opened this issue Sep 6, 2018 · 13 comments
Labels
enhancement M-T: A feature request for new functionality
Milestone

Comments

@mistydemeo
Copy link
Contributor

Description

Describe your issue here.

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

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

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

  • I've read and understood the Contributing guidelines and have done my best effort to follow them.
  • I've read and agree to the Code of Conduct.
  • 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

✅ Officially supported and compatible.

Hubot 3.x + Coffeescript 1.x

✅ Officially supported and compatible.

Hubot 2.x + Coffeescript 2.x

🚫 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

🚫 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:
@aoberoi
Copy link
Contributor

aoberoi commented Sep 6, 2018

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 aoberoi added the enhancement M-T: A feature request for new functionality label Sep 6, 2018
@xxbiohazrdxx
Copy link

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

@aoberoi
Copy link
Contributor

aoberoi commented Oct 19, 2018

@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
Copy link
Contributor Author

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
Copy link
Contributor

phucnh commented May 6, 2019

Hi, I continue @mistydemeo works at #565
(Thank mistydemo for stared 🙇)

@phucnh
Copy link
Contributor

phucnh commented May 14, 2019

@aoberoi
Hi, could you please review #565 pull request 🙇

@quinn
Copy link

quinn commented May 31, 2019

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

@crccheck
Copy link

crccheck commented Jun 3, 2019

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
Copy link

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 seratch added this to the 4.8.0 milestone Apr 24, 2020
@seratch
Copy link
Member

seratch commented Apr 24, 2020

From #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
Copy link

nickiannone-tw commented Apr 29, 2022

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
Copy link

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
Copy link

so.. since #565 has been merged, what's still to do here..?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement M-T: A feature request for new functionality
Projects
None yet
Development

No branches or pull requests