JSX but not React in scope
Opened this issue · 46 comments
I'm using JSX syntax to generate virtual-dom/h code using babel plugins (this specifically).
Only problem is, now standard complains because React is not in scope.
Not quite sure how this should be addressed, however. Because that check is useful when using React.
Perhaps it'd just be best to remove that rule? Not having React in scope will blow up pretty quickly. That's doesn't seems like something that would cause subtle bugs, it just blows up :)
Why not just disable it for your use case?
Put this at the top of your file:
/* eslint-disable react-in-jsx-scope */@jprichardson Wouldn't disabling the rule be friendlier for people using React competitors like Riot and Deku? I don't really want to be opinionated and push people to React without a good reason.
@feross yep.
I don't really want to be opinionated and push people to React without a good reason.
Alright, fair enough. While it's nice to have the live feedback in my IDE (linter-js-standard) when I forget to add React with JSX, I definitely understand this perspective and don't have a strong feeling either way.
This error is almost impossible to miss, as the code simply will fail at require time in almost all cases.
I'm OK with it being disabled.
@HenrikJoreteg want to send a PR?
Sure, will do
On Wed, Dec 2, 2015 at 19:00 Feross Aboukhadijeh notifications@github.com
wrote:
@HenrikJoreteg https://github.com/HenrikJoreteg want to send a PR?
—
Reply to this email directly or view it on GitHub
#351 (comment).
Didn't know if you need anything else in terms or tests or whatnot. Just tweaked the config in the other module? Just lemme know if i missed something.
it does raise an interesting question, however. Now what happens to the h that's an unused variable?
@HenrikJoreteg So, now you get an unused variable warning?
right
I merged that change, but, we can roll it back if necesssary.
If h is undefined, then, you should global it out IMHO.
@dcousens doing /*global h*/ doesn't solve it because h is an unused variable. Have to do: /*eslint no-unused-vars: [2, {"varsIgnorePattern": "h"}]*/
We currently have a way to define globals via package.json, perhaps we need a way to define unused vars too?
not sure if this is relevant, but setting h as a global in package.json will not throw a no-unused-vars error if it's not used.
I agree that'd be a nice way to do it. I could see this kind of thing
happening more and more now that people are doing more things with babel
plugins.
On Wed, Dec 9, 2015 at 9:38 PM Dan Flettre notifications@github.com wrote:
We currently have a way to define globals via package.json, perhaps we
need a way to define unused vars too?—
Reply to this email directly or view it on GitHub
#351 (comment).
Running into this myself. When using something else for JSX other than React, a whole slew of errors actually happen. Here's a simple example using deku
/** @jsx element */
import 'element' from 'magic-virtual-element'
import { render, tree } from 'deku'
render(tree(<div class='box'></div>), document.body)- 'React' must be in scope when using JSX (react/react-in-jsx-scope)
- Unknown property 'class' found, use 'className' instead (react/no-unknown-property)
- "element" is defined but never used (no-unused-vars)
I really wanna use standard on this new project but all of these things make it a pain.
Same problem with snabbdom-jsx. We are trying to port cerebral to standard, but hit this blocking issue.
The only way to use standard is to wrap all jsx related imports with
/*eslint-disable no-unused-vars*/and every jsx file with
/*eslint-disable react/react-in-jsx-scope*/disabling react-in-jsx-scope is just an inconvenience, but disabling no-unsed-vars is bad.
In order to use jsx with something other than react it is necessary to declare the plugin in your .babelrc
"plugins": [
[ "transform-react-jsx", { "pragma": "Component.DOM" } ]
]Maybe standard could check this, then instead of checking for React it could now check for Component and auto fix both of the above rules?
@feross perhaps we ditch react in the next major? Is it possible for us to do things like eslint-env react?
As a lateral thought, it'd be nice for standard-engine to pick up your local .eslintrc.
You can set your eslintrc to this:
{
"extends": ["standard", "standard-react"]
}...and customize it as you see fit.
I ended up adding React as a global in package.json which isn't a terrible solution, though a bit hacky.
I almost wonder if standard should just ignore react-related settings beyond tolerating JSX.
@HenrikJoreteg it really does feel like it should just be an extension, so I'm not impartial to that either.
Yeah, I'd like to eliminate built-in React rules (i.e. eslint-config-standard-react) from standard. We can create a standard-react package for those who want the old behavior, with built-in react support.
I'm trying to wrap my head around the unused variable thing. Can someone help me understand?
@rstacruz In your example:
/** @jsx element */
import 'element' from 'magic-virtual-element'
import { render, tree } from 'deku'
render(tree(<div class='box'></div>), document.body)It does appear that 'element' is an unused variable. I'm guessing that the @jsx pragma thing is actually making use of it, though?
What can standard do to accommodate this better?
React-specific rules are removed in master now, in preparation of standard v6. 👍
@HenrikJoreteg @jprichardson @dcousens @Flet @rstacruz @garth Let me know if you think of a way to fix the JSX unused variable issue, or at least make it easier to deal with.
Alright, I've gotten it so that @rstacruz's example doesn't have any issues in standard v6 and will just pass. The /* @jsx element */ pragma is used when it exists, and the other react-specific rules are gone.
But I'm still going to keep the basic sanity-check JSX rules. Here's the list:
"react/jsx-boolean-value": 2,
"react/jsx-closing-bracket-location": 2,
"react/jsx-indent": [2, 2],
"react/jsx-indent-props": [2, 2],
"react/jsx-no-duplicate-props": 2,
"react/jsx-no-undef": 2, //
"react/jsx-uses-react": 2, // doesn't actually require react -- uses @jsx pragma
"react/jsx-uses-vars": 2, // marks variables in jsx as used
"react/self-closing-comp": 2
And the no-unknown-property is removed.
Looks like I'm a little late to the party, but I'd like to propose adding "jsxPragma" to the standard config.
note: If this should be in a different issue, let me know and I'll continue this in a new one.
Right now, I've got a component using preact (which uses virtual-dom) that requires h instead of React as a global.
import { h, render } from 'preact'
import { IndexRoute, Route, Router } from 'react-router'
import { createHistory } from 'history'
import { App } from './app'
import { Home } from './home'
import { About } from './about'
export const routes = (
<Route path='/' component={App}>
<IndexRoute component={Home} />
<Route path='/about' component={About} />
</Route>
)
render(<Router history={createHistory}>{routes}</Router>, document.querySelector('[data-hook="app"]'))Babel allows a config in .babelrc like so:
{
"jsxPragma": "h"
}Thus, is might work if standard could have it's jsxPragma set (vs. including the pragma statement on each page) like so in package.json which could be used to ignore the "defined but never used" errors. Should work for any jsx library I would think.
"standard": {
"jsxPragma": "h"
}@mike-engel standard's philosophy is to avoid configuration and opt for in-line documentation. You can set which JSX pragma you're using by adding a comment to the top of your file:
/** @jsx h */
import { h, render } from 'preact'
// etc ...That should work 👍
Yeah, I understand @feross. I was coming at it from the angle where instead of having to add the pragma to every file (I don't currently add them, they're inferred as react jsx by default), it would be nice to have one place to add them. It would be the same idea as defining globals in one place vs. adding /* global describe */ to every test file.
I understand the reasoning for keeping it out, however, and it's just a thought.
@mike-engel Thanks for clarifying what you meant. I still think we'll keep this out of the package.json for now.
@feross just want to add my point of view for maybe additional perspective.
I totally agree that there should be zero configuration using standard. But, as I see it, the pragma isn't a configuration. jsx already baked into standard (this is the configuration) and because there are different virtual dom libraries, jsx users must specify the pragma. So the question is not if to specify but where to specify and to me it seems similar to the parser option because its consistent along the project.
The need of adding additional and same line to all jsx files so a tool (standard) that already supports jsx will understand the input, just doesn't feel 100% to me so IMHO I think @mike-engel suggestion is better than the current state and setting jsxPragma should be part of standard somehow.
It would be the same idea as defining globals in one place vs. adding /* global describe */ to every test file.
Except that is exactly what we think should happen, because anything else means implied context which causes programmer errors and should be avoided.
I like to code by the phrase:
if it is ugly, make it as ugly as possible, unless its beautiful, in which case let it be beautiful
Magical globals do not fall into a beautiful category IMHO.
@dcousens I 100% agree with you.
Maybe I'm wrong but I just don't see the pragma falls into the globals category. As I see it, globals are implicitly defined and explicitly used and in the jsx case it something that explicitly required (not global) but used implicitly.
I thought that jsx support should include the configuration (rules) + pragma definition (setting by the user) because its not complete without the pragma but now I have better understanding about your point of view. Not sure about my opinion.
Is this the only use case for this scenario (explicit require and implicit use) ?
@mike-engel What's the workaround you're using for Preact (if you found one)? As of right now, standardjs seems unusable for a preact app since it catches every instance of h as unused. And I'm not adding /** @jsx h */ to every component file, that seems unreasonable.
@jsonmaur preact didn't end up working back then for what we needed, so we never had to support standard + preact together
I am using Preact in a new app I'm building. Right now, I'm just adding /** @jsx h */ to the top of every component file. It's not that bad.
Any chance this could be revisited? It's pretty annoying to have it in every file and adds a deal of visual noise IMHO.
It's easy enough for a power user to automate the comment and understand the importance of it (and why it doesn't appear in React code bases), but for a beginner just starting out with Frontend it's "yet another thing" they have to battle with before they can actually build something.
This affects a lot of frameworks (like hyperapp), not just preact.
Example: You are showing a user how to build a MVC website, and the first line of code they see is a strange comment. Instead of explaining the application to them, the first discussion you'll be having is about JSX, standard, linters, React...
IMHO forcing a comment at the header of each file goes against the main selling points of standard (zero-config, forget about styles & formatting, minimize lint noise to make the errors meaningful, etc).
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
I would like to address this in a future version of standard. Does anyone have interest in sending a PR to standard-engine to add a jsxPragma option that can get picked up from the package.json configuration key?
React is moving away from requiring that React is in scope, and instead the jsx transpiler adds the appropriate import. Maybe the way forward is to simply disable this rule? 🤔
@LinusU Happy to do that! But if we do that will we get another error related to using an undefined variable?
I guess JSX needs React to be in scope.
So, if you were to simply import react at the very top, you can get past this problem.
I am developing a React component as an npm package. I need this rule enabled, otherwise my code cannot be used in other projects. How can I enable react-in-jsx-scope?