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

Getting JSX syntax to play nicely with jslint? #121

Closed
TYRONEMICHAEL opened this issue Jun 23, 2013 · 20 comments
Closed

Getting JSX syntax to play nicely with jslint? #121

TYRONEMICHAEL opened this issue Jun 23, 2013 · 20 comments

Comments

@TYRONEMICHAEL
Copy link

I have been playing around with the library. I am trying to use their JSX syntax, and was just wondering how you guys get it to play nicely with jslint/jshint?

JSLint obviously does not like the JSX syntax - ("expected an identifier and instead saw ' <';" - JavaScript syntax error), so how do I get around this in my .jshintrc file?

@jordwalke
Copy link
Contributor

fYou must first pass the source code through the JSX transform. Once that is done, everything should work perfectly. One benefit to how we've proceeded with JSX, is that we preserve line numbers in the transforms so using lint (once transformed) will "just work".

That means that jslint will even catch:

var p = <Typeahead />

If Typeahead is not a variable in scope! (It will complain that Typeahead is not defined.) I have a customized version of a vim jslint plugin that highlights lint errors with a red underline. I'm just working on getting Windows support. Would something like that work for you? What's your lint flow?

@petehunt
Copy link
Contributor

You can do this with jsx --watch. The in-place transform may benefit you as well; the tool supports a --extension directive such that you can transform .jsx files to .js files alongside each other.

@TYRONEMICHAEL
Copy link
Author

Thanks so much for the quick and detailed response. Actually running Sublime with Sublime Linter, so obviously it tries to lint the JSX syntax and thus keeps throwing errors. Might have to take your customization of the VIM plugin and try create something similar for SublimeLinter. I think it is quite easy to extend. But for the meantime, I think I will just run jslint on build after it has run through the JSX transform and turn it off while in dev. It's just nice to have it run in the background catching silly errors while developing. Thanks again for the hardwork! Find it as a powerful alternative to Views in Backbone.

@petehunt
Copy link
Contributor

No problem @TYRONEMICHAEL . I was experimenting with the idea of forking jshint to build jsxhint; if this is something you're interested in please let me know (not saying I can get it done anytime soon, but can try to get it on the roadmap at least.)

@TYRONEMICHAEL
Copy link
Author

Would definitely be something I am interested in as I definitely prefer the JSX syntax - much easier to visualize in code.

@danielmiladinov
Copy link

I recently put in a pull request to jshint that adds a feature to ignore a block of lines in javascript and resume linting afterward. I specifically had JSX in mind.

You could enclose your JSX within the jshint comment directives

/*jshint ignore:start */

and

/*jshint ignore:end */

and the rest of your code should lint as normal. If you're defining react components in the file that only get used inside a JSX block, we typically add the line

void MyReactComponent;

Near the JSX block to assuage the unused variable warnings.

I'd appreciate your thoughts and/or feedback on this.

@sophiebits
Copy link
Collaborator

As you can see in the earlier comments, it's recommended to just run jshint after doing the JSX transform. I implemented this for the Khan Academy linting tools this weekend, so if you download https://github.com/Khan/khan-linter and do runlint.py file.jsx, then everything should Just Work. (Not really designed for external use, but useful if you're looking for something to copy.) Maybe I'll make a jsxhint npm package that calls jsx then jshint in turn.

@danielmiladinov
Copy link

@spicyj, I understand the recommendation, and that's fine for doing something like running jshint as part of a CI build step or what have you.

My particular flow is that I use IntelliJ/Webstorm's JSHint plugin to automatically highlight linter errors before I've even checked my code in, let alone kicked off a build. I have found it very handy to have that immediate feedback. Since switching to react, my only regret was seeing the linter barf on the first line of JSX in my javascript files.

Having to comment out my JSX to see if the rest of my code lints was enough of a drag that it motivated me to work on this patch.

@sophiebits
Copy link
Collaborator

Gotcha. Maybe it makes sense for someone to write a JSXHint plugin. :) I have my vim configuration set up to use khan-linter; there's no reason I know of that IntelliJ can't do the same.

@jeffmo
Copy link
Contributor

jeffmo commented Sep 11, 2013

I wonder if you could hack your jshint plugin to run jsx first before passing off to jshint? #dogscience...

@danielmiladinov
Copy link

It's not my plugin, it's built into intellij/webstorm/phpstorm.

Not a separate plugin, but actually integrated into the javascript editor.

In the settings here's nothing to tweak on it except a version drop-down, which apparently just reads package.json from the jshint site, and you can specify a path to your .jshintrc file, if you have one.

Edit:
So, the sooner my patch gets accepted and a new version released, the sooner I can start using my new feature right inside my IDE. :)

@jordwalke
Copy link
Contributor

@danielmiladinov: How do they handle things like coffeescript linting? I'd really seriously recommend doing it right, and not avoiding certain regions. The reason is that you're actually missing some seriously awesome linting action by ignoring regions.

<Typeahead
    userName={"user:" + theUserName}
/>

Is just sugar for

Typeahead({
    userName:"user:" + theUserName
});

In my Vim linter, it tells me if Typeahead is not defined, or if I've mispelled theUserName or if theUserName variable is not in scope etc. Are there any webstorm linting plugins that you could fork? I'm sure people around here would help you integrate jsx + jshint into it.

@petehunt
Copy link
Contributor

Do you think jshint would be receptive to a syntax transform patch?

@jordwalke
Copy link
Contributor

@petehunt: It's hard to tell. JS+JSX is a superset of JavaScript, and syntactically compliant with E4X, which is another variant of JS - the trouble is that JSHint would need to assume the particular transformation that we apply (simple function calls) in order to extract value from the linting process. Then again, would anyone want to transform JSX into anything other than function calls (or something congruent with respect to linting)? Likely not.

@zpao
Copy link
Member

zpao commented Sep 26, 2013

For those who missed it, https://github.com/Enome/jshintreact may help, at least in the short term.

@vjeux
Copy link
Contributor

vjeux commented Sep 26, 2013

Another version from @toddself https://github.com/CondeNast/JSXHint

@danielmiladinov
Copy link

@jordwalke, sorry for my so-late reply, the JSHint plugin we use doesn't have too many options, other than setting a non-standard location for your .jshintrc file, and choosing which version of the main jshint repository you want to run.

That's why I was hoping for my pull request to make it into jshint proper, so that I could leverage it right in my IDE, to get that immediate feedback before having to run a build.

Failing that, I may reach out to JetBrains themselves and see what we can do to perhaps open up more options, say, something "use this build of jshint please, and thank you very much" and point it at a directory or something like that. Then anybody could use whatever custom features in their own jshint installs they needed the most.

@CircleCode
Copy link

@danielmiladinov as far as I understand it, the better approach for jetBrains would be to have a plugin specifically targetting jsx syntax. Since their ide allows the use of "language injections".
see feature request: http://youtrack.jetbrains.com/issue/WI-21217

@danielmiladinov
Copy link

@CircleCode, I agree, that would be an awesome feature! However, it's orthogonal to the question of linting in general, and their JSHint plugin in particular.

All their JSHint plugin does is "shell out", (not to make too much light of it; I'm so grateful for it) to whatever version of JSHint you've downloaded (it's configurable in the options), and puts the relevant error messages with red squigglies on the relevant line numbers in your editor. (You see what the error was when you mouse over the squigglies, very handy)

Their editor is already pretty good at inferring that you're using some kind of xml literal right in the middle of your javascript. It doesn't understand the notion of "valid-for-jsx" xml, but it already goes a long way to ensuring your jsx is at least well-formed xml: that is, unless you put your jsx expression inside parentheses, with a single root tag, and every starting tag is either self-closing or has a corresponding close tag, the rest of your javascript syntax coloring just goes completely crazy, indicating that you're doing something very wrong ;)

All it's missing is the convention of valid jsx, i.e., that tag names correspond either to "global" React.DOM components or your own custom components (which must then be visible in the current scope) and that attribute names correspond to component prop names and that attribute values correspond to expressions containing variables from the current scope. But I'm beginning to describe how the JSX Transform works, aren't I?

@sterpe
Copy link

sterpe commented Aug 29, 2014

I was actually hoping for something that would actually lint the raw JSX as part of the coding process. The jsx compiler obviously catches almost everything with specific line numbers and then it's easy to fix the typos after a compile attempt, but it sucks that you have to run the compiler to find syntax errors that a static analysis would reveal.

Simply ignoring sections of code doesn't fix the problem of static analysis.

Is there any formal definition of JSX syntax (like a language report) anywhere that could be used as a guide for a true static parser/linter?

As to linting after the JSX transform, I'm with the crowd that doesn't care whether the compiled by machine code of any altJS conforms to some arbitrary standard of best style.

Kagami added a commit to bitchan/bitchan-web that referenced this issue Dec 4, 2014
taneliang referenced this issue in MLH-Fellowship/react Aug 17, 2020
* Minor clean up of EventTooltip.js

* Fix root cause of #47 and improve tooltip styling

Tooltips would go offscreen because of the way `top` is defined. This
commit fixes that root cause by changing CanvasPage to position fixed so
that tooltips will be positioned relative to the DOM rather than the
CanvasPage div.

Undoes some of the changes made in #78 as this commit fixes the root
cause of the bug.

* Disable component name truncation

Although this was Brian's recommendation (see #67), I don't think
component names will be very long because the source code for those will
be really annoying.

* Set mouse location on every interaction to fix #87

* Introduce 768-char component name length limit

Covers the unlikely edge case that a component name is unreasonably
long.
taneliang referenced this issue in MLH-Fellowship/react Aug 19, 2020
* Minor clean up of EventTooltip.js

* Fix root cause of #47 and improve tooltip styling

Tooltips would go offscreen because of the way `top` is defined. This
commit fixes that root cause by changing CanvasPage to position fixed so
that tooltips will be positioned relative to the DOM rather than the
CanvasPage div.

Undoes some of the changes made in #78 as this commit fixes the root
cause of the bug.

* Disable component name truncation

Although this was Brian's recommendation (see #67), I don't think
component names will be very long because the source code for those will
be really annoying.

* Set mouse location on every interaction to fix #87

* Introduce 768-char component name length limit

Covers the unlikely edge case that a component name is unreasonably
long.
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