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

Problems with JSX support #138

Closed
kvnneff opened this issue May 13, 2015 · 27 comments

Comments

@kvnneff
Copy link

commented May 13, 2015

I've recently started playing with segmentio/deku, an alternative to React, and I'm also using Babel to transpile my js and jsx code.

The first issue that I ran into was that eslint expects to see React imported whenever it sees JSX. In my .babelrc I have already configured Deku's element function as the jsx pragma to use versus react, however standard has no way to know that and in order to get my files to properly lint I have to annoyingly include /** @jsx element */ at the top of my files.

The second issue is that React's JSX uses className to denote the class property on an element, whereas Deku uses class. Therefor JSX written for Deku will always error when linted by standard.

It makes me wonder what would happen if yet another popular framework were to be released that used a JSX-like syntax that wasn't exactly like React's JSX.

If there are any possible workarounds I would be happy to try them.

@feross feross added the question label May 13, 2015

@feross

This comment has been minimized.

Copy link
Member

commented May 13, 2015

This is the problem with baking in support for frameworks with special cases in the codebase. You can never support all the frameworks. :-(

When I first wrote standard, I initially baked in support for mocha's it and describe globals, since eslint had an option for that. But we eventually removed it. See discussion in #18 and #9.

I don't know what we should do here.

@Flet

This comment has been minimized.

Copy link
Member

commented May 13, 2015

Interestingly, deku uses Standard Style itself :)

@anthonyshort have you run into similar challenges when running standard against a deku project? Any tips?

@kvnneff

This comment has been minimized.

Copy link
Author

commented May 13, 2015

Yeah, I kind of brought this up in anthonyshort/deku#109

I looked to see if they did something differently, but there's no jsx being linted in the Deku project itself.

Personally I think the best thing to do is to remove JSX support from standard. I agree with @malandrew's comment on eslint/eslint#1291 (comment), that JSX should not have been added to eslint core, and I think it was a mistake to enable it in standard. I also agree with @jprichardson #18 (comment) that standard shouldn't be supporting frameworks.

React and JSX aren't 'standard' in any way, and while there are other JSX linters out there that could be used in conjunction with standard if someone wanted their JSX linted, there does not seem to be any alternative for those using a JSX-like syntax in their code other than forking standard, which I see as defeating the purpose of standard.

@yoshuawuyts

This comment has been minimized.

Copy link
Contributor

commented May 13, 2015

+1 on removing JSX for all the reasons mentioned above.

@jprichardson

This comment has been minimized.

Copy link
Member

commented May 13, 2015

Does removing support for JSX mean that standard would then start throwing errors on JSX syntax or does it mean that standard would just ignore the JSX?

@kvnneff

This comment has been minimized.

Copy link
Author

commented May 13, 2015

@jprichardson That's a good question.

I've actually decided to avoid JSX in my code from here on out, opting to instead use either React's or Deku's element creation utilities in pure js, so this issue will no longer effect me but I'm sure as frameworks like Deku and Mercury gain popularity this will pop up again.

@anthonyshort

This comment has been minimized.

Copy link

commented May 13, 2015

@staygrimm is right, we're not actually using JSX in the project itself so it hasn't actually come up yet. It would be fine in most cases if it just skipped the JSX but I could see how this would be difficult.

JSX really isn't a standard and probably won't be, but the reality is that people are using it a lot lately and it would be nice to have some support for it, even if that means it just skips those blocks.

@parro-it

This comment has been minimized.

Copy link

commented May 14, 2015

Yesterday I start playing with deku, and I came to the same issue.
The quick and dirty solution I found is to manually edit these rules file from eslint-plugin-react module replacing react stuff with deku:

  • no-unknown-property.js
  • jsx-uses-react.js
  • react-in-jsx-scope.js

Now (at least for the deku features I've used so far) deku JSX is linted correctly.
It would be super easy to place these file in a separate eslint-plugin-deku but standard should allow us to customize the rules set to use.

I understood why you don't like rule configuration files, but this is a different use case.

@yoshuawuyts

This comment has been minimized.

Copy link
Contributor

commented May 15, 2015

(..) frameworks like Deku and Mercury gain popularity this will pop up again.

Don't think so; template strings solve the problem JSX tries to tackle without forking the language. This is how you can create virtual-dom elements for mercury right now:

const virtual = require('virtual-html')
const els = virtual(`
  <html>
    <head></head>
    <body></body>
  </html>
`)

See Also

@yoshuawuyts

This comment has been minimized.

Copy link
Contributor

commented May 15, 2015

(...) but standard should allow us to customize the rules set to use.

The first rule of standard is no configuration. Whatever the outcome of this thread, adding options is the wrong solution.

@feross

This comment has been minimized.

Copy link
Member

commented May 15, 2015

I think if we can figure out a way to get standard to just skip JSX code, people using JSX will still have issues. Their code will appear to have unused variables, when those variables are actually being used in the JSX.

@parro-it

This comment has been minimized.

Copy link

commented May 18, 2015

What do you think about adding a plugin system to standard? JSX could be moved to a plugin, deku specific JSX could be another one, and to avoid configuration, it's possible to scan for modules in package.json named with a standard prefix. E.g .standard-jsx, standard-deku-jsx etc...

@tunnckoCore

This comment has been minimized.

Copy link

commented May 21, 2015

What do you think about adding a plugin system to standard?

It would be weird. Who want, just to use eslint cli + one global config (eslint-stanard-config) and with modified rules and that's it - not so hard deal.

It's funny, @feross and the community just want and need simple, minimal and clear code style which is like a standard for big part of the community, that's why he named it standard (i thought).
Standard not mean "I want this, but not that. second want other thing" and so on. Why won't go and say to W3C "I don't want the attribute 'placeholder', I want just 'holder', nah!"? LOL, I mean..

And I dont like that codeclimate aggressively signals me that "missing semicolon" and I just considered to just add /* jshint asi:true */ to pass GPA 4.0 and period. I mean... yea I also dont like the part of the stardard that says "no semicolons, really", but I started updating my libs - hybridify-all, npm-related and so on.

The only change should only be refactoring and bugfixes

@idan

This comment has been minimized.

Copy link

commented Jun 25, 2015

I ❤️ standard, coming from python it was refreshing to see something like PEP8.

I've been doing a bunch of react stuff lately (with babel, JSX). I'd be sad to lose JSX 'support' in standard, maybe eslint + babel-eslint + eslint-standard would be a reasonable facsimile. I've yet to try it out and see how it compares to standard as it is today.

Either way, options are anathema to the spirit of this project.

@ashaffer

This comment has been minimized.

Copy link

commented Jul 19, 2015

Why not just add support for reading .babelrc files and checking jsxPragma? Or a similar field for package.json.

@feross

This comment has been minimized.

Copy link
Member

commented Jul 30, 2015

@ashaffer standard is not the place for that. You can try to convince the maintainers of eslint-plugin-react to support detecting a custom pragma (i.e. don't do React-specific checks when an alternate pragma is present).

@feross

This comment has been minimized.

Copy link
Member

commented Jul 30, 2015

The espree parser (used by eslint) supports JSX out of the box. standard uses eslint-plugin-react to make warnings about unused variables that are actually used in JSX go away. Too many users depend on React support. If we remove it, the reason should be better than "it feels gross".

FWIW, Deku users can use eslint-config-standard directly, which makes no assumptions about React. I also found eslint-config-standard-deku on npm, which looks promising.

@feross feross closed this Jul 30, 2015

@ashaffer

This comment has been minimized.

Copy link

commented Jul 30, 2015

@feross Ya, I created that. I basically just forked the react plugin and pulled out the stuff that was react specific. It seems to work ok for my stuff so far. I also created a fork of standard that uses it:

https://github.com/deku-scrubs/standard

@andrewdeandrade

This comment has been minimized.

Copy link

commented Jul 30, 2015

I agree that "it feels gross" is a bad reason. "not paving a bad cowpath" is a much better reason. Supporting JSX out of the box is the engineering equivalent of Mozilla supporting DRM out of the box in FireFox.

The react community has become a big cargo cult. There are some good ideas in the community and many Bad Ideas™. Paving the bad idea cowpaths lends a sense of legitimacy to these bad technical ideas that is not merited.

One of the primary tasks of engineers is to minimize complexity. JSX changes such a fundamental part (syntax and semantics of the language) that the complexity bubbles up to everything it touches. Pretty much every pipeline tool I've had to work with has become far more complex than necessary because of JSX. It affects AST parsers, it affects linters, it affects code coverage, it affects build systems. That tons and tons of additional code that I now need to wade through and mentally parse and ignore whenever I need to debug or want to contribute to a library that adds JSX support.

I'm okay with an overall design that allows people to plugin the parts they need in order to be able to generically support a compile-to-javascript language, but to bake in support for one singular solution because its popular is simply bad engineering. Of all the compile-to-languages, the one that strikes me as having the least merit is JSX. It's basically a ton of added complexity for the sake of what boils down to syntax. There are no real gains in terms of language semantics in JSX.

Furthermore, JSX encourages bad non-dry code. Having seen a lot of JSX over the past few months, its encourages copypasta coding. Yes, you can embed loops in it and compose lots of small repeated JSX snippets, but that almost never happens in practice because mixing the turing complete of javascript with the markup of HTML eliminates the readability of JSX so that it is actually harder to parse than a solution like hyperscript (the syntactical approach taken by virtual-dom). Without elegant ways of expressing loops/iterators (like angular does with directives), the primary way to keep JSX readable thus becomes copying and pasting. (I'm not arguing in favor of angular's approach here, just saying that if you're going to try to go with a markup approach, at least go all the way, instead of the frankenstein that is JSX).

nzaka's theory of bunny code is relevant here: http://www.nczonline.net/blog/2015/05/14/the-bunny-theory-of-code/ (which is ironic since he made the decision to support JSX in eslint)

@dcousens

This comment has been minimized.

Copy link
Member

commented Jul 31, 2015

@malandrew I know where you're coming from, and to a degree, I think you're right.
However, as a developer that uses JSX, I find it too useful/concise to give up in the name of syntax purity, especially when I know that what it translates to is still very isolated and computationally pure.
I'm personally open to any other solutions, especially in how we might be able to untie from React, but, for now, inline-XML markup (JSX) is OK with me.

@andrewdeandrade

This comment has been minimized.

Copy link

commented Jul 31, 2015

@dcousens hyperscript is a perfectly good alternative and uses the same interface as matt esch's virtual-dom. https://github.com/mlmorg/react-hyperscript

The only "issue" it has is that its unfamiliar. People have been working with HTML for years and are comfortable with it. That's basically the only reason that people find it more readable. If you make an effort to spend sometime with hyperscript, it becomes as familiar and readable as jsx. I have a few colleagues that converted to hyperscript, and while they were adverse at first, they were satisfied with having switched once they had become comfortable with the way it looks/reads.

hyperscript is much simpler to refactor and DRY up your code than with JSX, because, being vanilla javascript, its easier to work with variable assignment, loops and conditionals.

hyperscript is more concise because it's just a function call and doesn't require a closing tag. Using it will greatly simplify your tooling chain.

If the react cargo cult didn't have the JSX cowpath paved for them and acclimated to describing their app interface with vanilla javascript, they'd cargo cult around that. It's really about the path of least resistance and familiarity.

@dcousens

This comment has been minimized.

Copy link
Member

commented Aug 1, 2015

Welp, I'm on the fence. @jprichardson ?

@jprichardson

This comment has been minimized.

Copy link
Member

commented Aug 1, 2015

Welp, I'm on the fence.

This conversation has taken many turns. So, on the fence about what? Checking for a JSX pragma?

@yoshuawuyts

This comment has been minimized.

Copy link
Contributor

commented Aug 2, 2015

@malandrew that's brilliant, I wasn't aware react-hyperscript existed! That means we can go full circle by compiling html -> hyperscript -> engine using ES6 tagged templates. E.g. this should work soonish with both virtual-dom and react:

const html = require('tagged-virtual-html') // needs to be written

function render (state) {
  return html`
    <div>
      <h1>clicked ${state.n} times</h1>
      <button onclick=${onclick}>click me!</button>
    </div>
  `
}

function onclick () {
  el({ n: state.n + 1 })
}

See Also

@subfuzion

This comment has been minimized.

Copy link

commented Mar 9, 2016

I'm not using JSX, just writing Node.js ES6 code (that gets transpiled with Babel), and I get a warning for my import statements. Ex:

import { Route } from 'atomiq';

Warning: Definition for rule 'react/jsx-no-duplicate-props' was not found...

Based on the discussion so far, am I out of luck?

@feross

This comment has been minimized.

Copy link
Member

commented Mar 16, 2016

@yoshuawuyts @malandrew @jprichardson Just wanted to make sure you're aware of https://www.npmjs.com/package/hyperx which is a much nicer solution than JSX. Doesn't require the use of transpiler or modifications to all JS tooling ever invented.

I'm using it in https://github.com/feross/webtorrent-app and it's working great!

@feross

This comment has been minimized.

Copy link
Member

commented Mar 16, 2016

@subfuzion That error looks unrelated to the existing discussion.

@lock lock bot locked as resolved and limited conversation to collaborators May 10, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.