CoffeeScript 🔜 JavaScript #1347

gr2m · 2017-06-02T14:15:02Z

The JavaScript proposal for Hubot Evolution is still ongoing, so everything can change. If you have any questions better to discuss there.

But at this point I have the source code working in JavaScript and all existing coffee script tests are passing, so I thought I’d be a good time to start sharing progress.

🛑 this PR is not yet ready for any feedback on the code

Most of it is still generated from decaffeinate. I’ll test the current version with some popular adapters / scripts first to makes sure it all still works

closes – eventually – hubotio/evolution#4

Breaking Changes

[email protected] and [email protected] are no longer supported
require('hubot/src/adapter') does no longer work. It was never officially supported, but now it won’t work at all. Use require('hubot').Adapter instead
Middleware.ticker() has been removed. Use process.nextTick() instead

Features

We now export es2015 class declarations of all Hubot classes at require('hubot/es2015').

Progress

See JavaScript Evolution Proposal for details

Convert source files from CoffeeScript to JavaScript
see evolution/4/javascript-src branch
Convert test files from CoffeeScript to JavaScript
see evolution/4/javascript-tests branch,
# update package.json
# add script for JavaScript linting
# Test with popular projects depending on Hubot
Update CHANGELOG.md
review by other maintainers 👀
squash the WIP commits
merge 🎉

All steps will be part of the same PR but split into separate commits so people
can follow more easily.

Convert source files from CoffeeScript to JavaScript

Convert all source files from CoffeeScript to JavaScript with a tool like decaffeinate, see also github/hubot#1138.

Make sure the existing tests (still written in CoffeeScript) run against the new JavaScript.

Now go through each file and improve the code readability by hand as needed. From my experiences with converting Hoodie from CoffeeScript to JavaScript quite a lot of manual work will be required.

Convert test files from CoffeeScript to JavaScript

Same as the previous steps only for the tests files this time.

When finished, create a separate branch with the tests written in JavaScript but the source code still in CoffeeScript to assure integrity.

Update package.json

Remove all CoffeeScript related dependencies and tooling. Update engines

Make sure that Hubot scripts can still be written in CoffeeScript. We want to break as few existing scripts as possible and many of them are written in CoffeeScript. We treat it as legacy support and will drop support for scripts written in CoffeeScript in future.

Add script for JavaScript linting

The only tool I would like to introduce as part of this proposal is a linting tool which will be run as part of npm test.

I suggest standard. We’ve been using it in all our projects at Hoodie and Neighbourhoodie since 2+ years and never looked back. It's a zero-configuration JavaScript linter.

Test with popular projects depending on Hubot

Instructions to test an existing adapter with this PR:

Check out the evolution/4/javascript branch of this pull request

git clone [email protected]:github/hubot.git
cd hubot
git checkout evolution/4/javascript
npm install
npm link

Now link it into your test app using an example adapter

Using the example of slack here:

npm install -g yo generator-hubot
mkdir my-awesome-hubot && cd my-awesome-hubot
yo hubot --adapter=slack
npm link hubot

Now start hubot

NODE_PATH=`pwd`/node_modules HUBOT_SLACK_TOKEN=<your token here> ./bin/hubot --adapter slack

replace <your token here> with your own bots token from slack

gr2m · 2017-06-08T11:27:43Z

I’ve just tested it with the IRC adapter which is written in CoffeeScript and it seems there is still some problem with inheritance. this.adapter.run() is the empty dummy run () {} from the abstract /src/adapter.js instead of the one from the irc adapter

gr2m · 2017-06-08T15:58:19Z

okay, fixed via 9aa5fe7

Tested it with the IRC adapter and it works. Doesn’t mean that it wouldn’t break with something else. This isn’t super elegant, but still our best option right now as we transition to move forward with the new roadmap and revive the project’s community I think :)

gr2m · 2017-06-08T17:57:42Z

found a problem with hubot-hipchat thanks to @tombell, investigating now

UPDATE: fixed in 2d25335
UPDATE: I tested it with a new hipchat account and it all worked

bkeepers

First of all, soooo much ❤️ for doing this. It's a huge effort, so thanks for spearheading it.

A few comments, but I don't see anything that should hold this up from merging and fixing any issues in follow PRs.

bkeepers · 2017-06-07T15:51:09Z

.travis.yml

@@ -1,12 +1,8 @@
 language: node_js
 node_js:
-  - "node" # latest stable Node.js release


Would it make sense to keep this in so we're always running against the latest release? Or are you concerned it would lead to unexpected failures when new node releases are put out and we should make sure we're being explicit about which versions are supported?

yeah it would lead to unexpected failures, node major versions are still breaking versions. We are experimenting with Greenkeeper sending PRs to update the these versions, so as soon as version 9 comes out it will send a PR updating the .travis.yml file. I think that’d be the perfect way

bkeepers · 2017-06-08T21:26:33Z

CHANGELOG.md

@@ -1,3 +1,16 @@
+## [v2.19.0](https://github.com/github/hubot/tree/v2.19.0) (2017-06-09)


v2.19.0

Should this be v3.0?

bkeepers · 2017-06-08T21:32:40Z

bin/hubot

+# we left the `bin/hubot` file to remain in CoffeeScript in order prevent
+# breaking existing 3rd party adapters of which some are still written in
+# CoffeeScript themselves. We will depracate and eventually remove this file
+# in a future version of hubot


I'm not sure I understand this. As long as it still includes a require('coffee-script'), shouldn't it be able to load JavaScript?

Also, should we print a deprecation warning that coffee script will not be supported in the major release that follows this one?

I don’t think require('coffee-script') will be enough, I'll have to investigate it. If you don’t mind I’d make a follow up PR to change it later if it works.

Regarding the deprecation warning I’m not yet sure when we drop CoffeeScript support officially, but we can add a deprecation warning in a patch release, too, so we don’t keep that PR waiting

alangpierce · 2017-06-09T03:50:02Z

Really cool to see this. 😄 If you have any feedback about how decaffeinate could be improved, feel free to let me know or file an issue.

gr2m · 2017-06-09T07:06:17Z

@alangpierce decaffeinate was a great help! I compiled it with babel.js afterwards to target Node@4. It all wasn’t perfect, but I think the tooling is as great as it can be, thanks so much!

gr2m · 2017-06-09T07:15:35Z

I’ve tested again with all 3: Slack, IRC and HipChat with my latest changes and they all continue to work

…ubotio/hubot#1347

cjdelisle · 2017-06-13T14:12:56Z

I've ported the matrix adapter to js since this seems to be the evolution and I prefer js anyway, but I discovered an issue. I am getting the same run() {} issue as #1347 (comment) and I'm not entirely sure how to deal with this. Apparently it now works well for coffee but not so much for js.

gr2m · 2017-06-13T14:19:11Z

@cjdelisle Thanks for checking the PR with the matrix adapter! Do you have your code somewhere I can have a look? I thought I resolved the issue and tested it with both, adapters written in coffeescript as well as javascript.

cjdelisle · 2017-06-13T14:26:26Z

Well I think it worked but I screwed it up porting to js. You can test it here https://github.com/xwiki-labs/xbot/blob/master/bot.js#L35 just do a npm install

gr2m · 2017-06-13T14:41:51Z

I was able to reproduce the issue, I’ll look into it tomorrow

cjdelisle · 2017-06-13T15:34:42Z

I'm gonna push a workaround for this and you can continue debugging with https://github.com/xwiki-labs/xbot/tree/c38054d4d6d3311a6232e0f353349d41342c043a
Thanks !

gr2m · 2017-06-13T19:18:52Z

@cjdelisle it looks like you released a new version of hubot-matrix without pushing its code to GitHub is that possible?

The problem here is that you try to extend an es2015 class from a "CoffeeScript" class, this would not have worked before either. But what you can do now is to require('hubot/es2015') which gives you our internal es2015 class declarations (see https://github.com/hubotio/hubot/blob/master/es2015.js), then it will work.

Could you check that please?

The incompatibility between CoffeeScript and es2015 classes is the biggest headache for the upgrade to JavaScript. We don’t want to directly export the native es2015 classes because it would break all the existing apps out there. The require('hubot/es2015') workaround is the best we can do right now. Eventually we will return native es2015 classes at require('2015') and get rid of the CoffeeSCript compatibility hack, but that will happen in a future breaking version of hubot

cjdelisle · 2017-06-13T19:58:59Z

everything is definitely pushed to github because I deleted node_modules and then ran npm install and then tested once finally before providing you the link.
I can confirm that the version of hubot-matrix which I'm using is ok with hubot-2.19 because I tested against this when converting hubot-matrix to js, perhaps the coffeescript loader sees es6 and adds some shim?
Requiring hubot/es2015 is slightly annoying because I have to do it in a try/catch and fallback to the old behavior, otherwise break users with the old (current) version of hubot, but I can do this anyway. Thanks.

gr2m · 2017-06-13T20:18:27Z

sorry I looked at a fork: https://github.com/xwiki-labs/hubot-matrix

Did you test hubot-matrix with the currently released version of hubot ([email protected])? I don’t think that it would work. The problem is that es2015 classes cannot inherit from CoffeeScript classes and the other way around. The conversion you did here would not work: davidar/hubot-matrix@d8224d3#diff-44f2bbaefc4af762ff42533cf1f158d4R18

Requiring hubot/es2015 is slightly annoying because I have to do it in a try/catch and fallback to the old behavior, otherwise break users with the old (current) version of hubot

yes that is true :/ But if you want to be compatible with the current hubot than you can’t use es2015 class inheritance anyway

What you can do is a to release a new version of hubot-matrix which would already require the new hubot version. We are coordinating the release of hubot@3 with the default scripts which get installed by the generator ourselves right now

cjdelisle · 2017-06-13T20:28:33Z

This is what I tested and it seems to be working ok: https://github.com/xwiki-labs/hubot-test/blob/master/package.json#L15
Even if this is fine, I can propose @davidar another change to make it attempt to use 'hubot/es2015' before falling back.

HUBOT_MATRIX_PASSWORD=<lalala> HUBOT_LOG_LEVEL=debug node_modules/.bin/hubot --name pads -a matrix -d

technicalpickles · 2017-06-16T02:09:45Z

If there's any followup work from this PR, I think it makes sense to track it in a new issue so we can track and 'close' it when it's done.

gr2m force-pushed the evolution/4/javascript branch 2 times, most recently from 3295b68 to 21876a9 Compare June 7, 2017 10:51

This was referenced Jun 7, 2017

JavaScript hubotio/evolution#4

Open

move MockAdapter into hubot repository #1348

Merged

gr2m force-pushed the evolution/4/javascript branch 3 times, most recently from 27243f9 to fc32221 Compare June 7, 2017 18:25

gr2m changed the title ~~WORK IN PROGRESS - CoffeeScript 🔜 JavaScript~~ CoffeeScript 🔜 JavaScript Jun 8, 2017

gr2m force-pushed the evolution/4/javascript branch 2 times, most recently from 02f2459 to 7853af8 Compare June 8, 2017 18:17

bkeepers approved these changes Jun 8, 2017

View reviewed changes

gr2m force-pushed the evolution/4/javascript branch from 7853af8 to 6150328 Compare June 9, 2017 13:32

gr2m added 7 commits June 9, 2017 15:43

chore(travis): test in node maintenance/LTS/stable versions

dda9d72

test: CoffeeScript -> JavaScript

ab527de

refactor: CoffeeScript -> JavaScript

7939b8f

chore(package): remove coffee-script related scripts and dependencies

eb9a20b

chore(package): standard

3b2ec09

style: standard

0c4debb

CHANGELOG updates for v3.0.0

9987f07

gr2m force-pushed the evolution/4/javascript branch from 6150328 to 9987f07 Compare June 9, 2017 13:43

gr2m merged commit ea72eba into master Jun 9, 2017

gr2m deleted the evolution/4/javascript branch June 9, 2017 13:46

superbob mentioned this pull request Jun 13, 2017

Migration to JavaScript / ES2015 flowdock/hubot-flowdock#101

Open

cjdelisle added a commit to xwiki-labs/hubot-matrix that referenced this pull request Jun 13, 2017

Ported matrix adapter to javascript as per the changes in hubot here h…

d8224d3

…ubotio/hubot#1347

cjdelisle mentioned this pull request Jun 13, 2017

Changes coffee to js, gives display name to bot core, exports matrix client so bot plugins can access it davidar/hubot-matrix#12

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CoffeeScript 🔜 JavaScript #1347

CoffeeScript 🔜 JavaScript #1347

gr2m commented Jun 2, 2017 •

edited

Loading

gr2m commented Jun 8, 2017

gr2m commented Jun 8, 2017

gr2m commented Jun 8, 2017 •

edited

Loading

bkeepers left a comment

bkeepers Jun 7, 2017

gr2m Jun 9, 2017

bkeepers Jun 8, 2017

bkeepers Jun 8, 2017

gr2m Jun 9, 2017

alangpierce commented Jun 9, 2017

gr2m commented Jun 9, 2017

gr2m commented Jun 9, 2017

cjdelisle commented Jun 13, 2017

gr2m commented Jun 13, 2017

cjdelisle commented Jun 13, 2017

gr2m commented Jun 13, 2017

cjdelisle commented Jun 13, 2017

gr2m commented Jun 13, 2017 •

edited

Loading

cjdelisle commented Jun 13, 2017

gr2m commented Jun 13, 2017

cjdelisle commented Jun 13, 2017 •

edited

Loading

technicalpickles commented Jun 16, 2017

		@@ -1,3 +1,16 @@
		## [v2.19.0](https://github.com/github/hubot/tree/v2.19.0) (2017-06-09)

CoffeeScript 🔜 JavaScript #1347

CoffeeScript 🔜 JavaScript #1347

Conversation

gr2m commented Jun 2, 2017 • edited Loading

🛑 this PR is not yet ready for any feedback on the code

Breaking Changes

Features

Progress

Convert source files from CoffeeScript to JavaScript

Convert test files from CoffeeScript to JavaScript

Update package.json

Add script for JavaScript linting

Test with popular projects depending on Hubot

gr2m commented Jun 8, 2017

gr2m commented Jun 8, 2017

gr2m commented Jun 8, 2017 • edited Loading

bkeepers left a comment

Choose a reason for hiding this comment

bkeepers Jun 7, 2017

Choose a reason for hiding this comment

gr2m Jun 9, 2017

Choose a reason for hiding this comment

bkeepers Jun 8, 2017

Choose a reason for hiding this comment

bkeepers Jun 8, 2017

Choose a reason for hiding this comment

gr2m Jun 9, 2017

Choose a reason for hiding this comment

alangpierce commented Jun 9, 2017

gr2m commented Jun 9, 2017

gr2m commented Jun 9, 2017

cjdelisle commented Jun 13, 2017

gr2m commented Jun 13, 2017

cjdelisle commented Jun 13, 2017

gr2m commented Jun 13, 2017

cjdelisle commented Jun 13, 2017

gr2m commented Jun 13, 2017 • edited Loading

cjdelisle commented Jun 13, 2017

gr2m commented Jun 13, 2017

cjdelisle commented Jun 13, 2017 • edited Loading

technicalpickles commented Jun 16, 2017

gr2m commented Jun 2, 2017 •

edited

Loading

gr2m commented Jun 8, 2017 •

edited

Loading

gr2m commented Jun 13, 2017 •

edited

Loading

cjdelisle commented Jun 13, 2017 •

edited

Loading