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

CJSX Coffeelint Rule #13

Open
skevy opened this issue Jun 30, 2014 · 17 comments
Open

CJSX Coffeelint Rule #13

skevy opened this issue Jun 30, 2014 · 17 comments

Comments

@skevy
Copy link

skevy commented Jun 30, 2014

This is not a bug, but rather a plea to the community that we build a coffeelint rule to allow for CJSX. I hate the little red underlines in Sublime. :)

By writing this here, I'm by no means saying I expect other people to do my dirty work for me, but if anyone has already started on this and they'd like my assistance, it'd be awesome to know about it. I've done some extensive searching and haven't found anything.

If not, well, then I plan to start on it as soon as I get a free moment. I feel like this transformer has already done much (if not all) of the dirty work, and we just need to repurpose the parser to work with Coffeelint. Even the ability to just not lint the CJSX parts of the coffeescript file, and not worry about actually linting the CJSX areas, would be a great start.

Thanks,
-Adam

@jsdf
Copy link
Owner

jsdf commented Jul 1, 2014

👍

@vyorkin
Copy link

vyorkin commented Aug 25, 2014

@skevy 👍 its really annoying

@tgriesser
Copy link
Contributor

👍

The coffee syntax highlighting in sublime could also use a bit more love, I tried my best but it doesn't know that the test within the body of react tags isn't code and therefore breaks on things like single quote marks when they're used as apostrophes in text, etc. If anyone has any ideas, that'd be great!

https://github.com/reactjs/sublime-react

@Benoss
Copy link

Benoss commented Aug 28, 2014

I tried the ignore approach was not good way to do it
I tried to modify coffee-lint and looks like it is a success with nearly no modification to the source, only 6 lines to modify
I am using the modified version with Atom using Linter Coffeescript and looks nice, at least for basing things, need more testing

linting

in package.json add the dependency

    "coffee-react-transform": "^0.5.0",

In the file coffeelint.coffee:
line 27 import coffee-react-transform

    CoffeeReactTransform = nodeRequire 'coffee-react-transform'

line 169 transform the source before checking for syntax errors

        source = CoffeeReactTransform(source)
        CoffeeScript.tokens(source)

line 223 send CoffeeReactTransform as parameter to ASTLinter

    # Do AST linting first so all compile errors are caught.
    astErrors = new ASTLinter(source, config, _rules,
                        CoffeeScript, CoffeeReactTransform).lint()
    errors = [].concat(astErrors)

In ast_linter.coffee

line 35 get ReactTransform parameter from the previous file

module.exports = class ASTLinter extends BaseLinter

    constructor : (source, config, rules, @CoffeeScript, @ReactTransform) ->
        super source, config, rules
        @astApi = new ASTApi @config

line 47 transform the source before sending to coffeescript

        try
            @source = @ReactTransform(@source)
            @node = @CoffeeScript.nodes(@source)
        catch coffeeError

Recompile using

npm run compile

Maybe an "official" fork of coffeelint for coffee-react-transform is a good idea
I can already imagine
npm install coffee-react-lint

Enjoy

jjoos pushed a commit to jjoos/coffeelint-cjsx that referenced this issue Oct 23, 2014
@jjoos
Copy link

jjoos commented Oct 23, 2014

@leyyinad created a pull request for coffeelint to support coffee-react-transform as a pre processor.
clutchski/coffeelint#346

@AsaAyers
Copy link
Contributor

I added a mechanism to use coffee-react-transform with CoffeeLint. here is the documentation

Before you close this issue. Does CoffeeReactTransform always maintain line numbers? If you take a one line JSX and break it into multiple lines, or take a multiline JSX and condense it into one line it's going to cause CoffeeLint errors to report the wrong line numbers.

@jsdf
Copy link
Owner

jsdf commented Dec 23, 2014

I took a fair bit of effort to maintain parity between input lines and output lines but I haven't had time to thoroughly test it. Hopefully I'll get to do that soon.

@jjoos
Copy link

jjoos commented Jan 6, 2015

@jsdf the parity never failed for us in practice

@AsaAyers
Copy link
Contributor

AsaAyers commented Jan 6, 2015

I just ran coffee-react-transform over my codebase and the only time where it changes the line number is if I include # @cjsx React.DOM. Since it isn't needed any more, I just don't use it except for this test.

@askholme
Copy link

askholme commented Feb 7, 2015

When using the react-bootstrap code the line parity is messed up if a Component configuration is split over several lines, i.e.

<Modal {...this.props}
   title="something"
   backdrop="false"
   animation="false">
...
</Modal>

Looks like coffee-react-transform will put the full Modal component creation on 1 line and thus the parity goes.

@jsdf
Copy link
Owner

jsdf commented Feb 8, 2015

Thanks, I recently noticed this regression (though it seems it was introduced with spread operator) and I'm working on it presently

@screendriver
Copy link

Any news about that? I get some coffeelint errors because it lints the generated coffeescript that coffee-react-transform produce.

@williamstein
Copy link

I made a fork of the CoffeeLint cjsx fork which works for me today, which somebody might find useful

https://github.com/sagemathinc/coffeelint-cjsx

@AsaAyers
Copy link
Contributor

There's no need for a fork

@williamstein
Copy link

This works -- Awesome! I'll update the README, etc., in my fork to make this clear. Then send another coffeelint-cjsx pull request to do the same to theirs, since I wasted time by being misled by the existence of coffeelint-cjsx.

@AsaAyers
Copy link
Contributor

I know the documentation on CoffeeLint isn't great. I know people have a hard time finding some info. I'm actually moving away from using CoffeeScript as IMO ES6 is superior and provides everything I need.

If anyone here has an interested in helping out, especially around improving the discoverability of documentation it would be greatly appreciated. I still deal with PRs and do releases occasionally, but I barely work on CoffeeLint any more.

@williamstein
Copy link

Well many thanks for what you've done. It's massively appreciated (by me at least)!

ABaldwinHunter pushed a commit to ABaldwinHunter/coffeelint that referenced this issue Aug 16, 2016
See jsdf/coffee-react-transform#13 (comment)

Reconcile CJSX changes with CoffeeLint project
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants