Migrate from CoffeeScript to Babel

mnquintana commented 9 years ago

This migration is essentially a test run for converting an existing package under the atom org from CoffeeScript to Babel. This doesn't make much of an attempt to refactor the code, but I could always take a look at that in another PR. :smile:

One important thing to note though: we should settle on a standard eslint config, much like we did for coffeelint in https://github.com/atom/atom/issues/5982. I don't think the .eslintrc in this PR as it stands is quite complete enough to be that standard eslint config yet.

/cc @atom/feedback

thedaniel commented 9 years ago

Thank you very much for getting this started.

maxbrunsfeld commented 9 years ago

I would say that this is no longer blocked, now that we support bundling babel packages. @mnquintana Could you update the code to use standard style and replace eslint in the package.json w/ standard? Thanks for doing this!

mnquintana commented 9 years ago

So this build shouldn't be passing (it still contains tons of semicolons) but it is - standard seems to return no errors when invoked with a directory, but when run without a directory (just standard) then it picks up all these errors properly. Not sure if this is something we're doing wrong or a bug with standard.

mnquintana commented 9 years ago

Ah, it requires a glob. I'll submit a PR to atom/ci then.

mnquintana commented 9 years ago

@maxbrunsfeld This should be all set other than my one comment!

KnutHelland commented 8 years ago

Can I ask what the motivation for this is? I'm a bit surprised to find this in an official atom package. Is there any official plans about migrating the codebase to JavaScript? Any links/issues/discussions I've missed?

And is this going to be merged into this package, or is it just an experiment? Should we wait for this to be merged before we fix issues, or should we fix issues in CoffeeScript? I'm concidering posting a fix for #27.

anaisbetts commented 8 years ago

fwiw, here's our .eslintrc for Slack, note that eslint and babel-eslint should be installed as a dev dependency:

{
    "parser": "babel-eslint",
    "rules": {
        "strict": 0,
        "indent": [
            2,
            2
        ],
        "semi": [
            2,
            "always"
        ],
        "arrow-parens": 2,
        "arrow-spacing": 1,
        "comma-dangle": 0,
        "constructor-super": 2,
        "no-class-assign": 2,
        "no-const-assign": 2,
        "no-dupe-class-members": 2,
        "no-this-before-super": 2,
        "callback-return": 2,
        "no-path-concat": 2,
        "no-label-var": 2,
        "no-shadow": 2,
        "no-var": 0,
        "no-use-before-define": 0,
        "handle-callback-err": 1,
        "prefer-spread": 1,
        "prefer-arrow-callback": 1,
        "prefer-template": 1,
        "no-console": 0,
        "vars-on-top": 0,
    },
    "env": {
        "es6": true,
        "node": true,
        "browser": true
    },
    "extends": "eslint:recommended"
}

50Wliu commented 8 years ago

Is there any official plans about migrating the codebase to JavaScript

I believe (not a Github employee, so don't quote me on this) that the Atom team plans to start moving over to Javascript. There is now Javascript linting support in atom/atom, as well as some recent PRs that added Javascript code: atom/atom#9426, atom/atom#9213.

thedaniel commented 8 years ago

Hi, just confirming that we're pro moving to ES6 though we don't have it explicitly on the roadmap.

@paulcbetts rather than checking in an eslintrc, @thomasjo suggested using standard's babel-eslint parser here: https://github.com/atom/atom/pull/9213#commitcomment-13968828, which makes sense to me. That said, no matter how we do it, adding the needful items to devDependencies causes CI to fail due to the permissive license check. I think that's probably OK but I'm discussing it with our legal dept to be extra cautious.

anaisbetts commented 8 years ago

@thedaniel I'm not sure what standard picks up, but ESLint picks up many non style-related bugs that troll developers (i.e. 'no-shadow' or 'no-this-before-super') and cause incorrect behavior

mnquintana commented 8 years ago

@paulcbetts Standard catches most of that stuff too (since it's just a preconfigured eslintrc, essentially): https://github.com/feross/eslint-config-standard/blob/master/eslintrc.json#L23

@KnutHelland Yup, the plan is to merge this - I just need to make one tiny tweak and it should be good to merge.

anaisbetts commented 8 years ago

@mnquintana I give this full marks, except for that no semicolon business :P

zeke commented 8 years ago

standard ftw!

mnquintana commented 8 years ago

@jacobp100 Airbnb's style guide is definitely more comprehensive, but this PR isn't intended to implement a style guide - right now we're going with standard, but it's up to the core team to ultimately settle on one going forward.