Join GitHub today
GitHub is home to over 40 million developers working together to host and review code, manage projects, and build software together.
Sign upJSX but not React in scope #351
Comments
This comment has been minimized.
This comment has been minimized.
|
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 :) |
This comment has been minimized.
This comment has been minimized.
|
Why not just disable it for your use case? Put this at the top of your file: /* eslint-disable react-in-jsx-scope */ |
This comment has been minimized.
This comment has been minimized.
|
@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. |
This comment has been minimized.
This comment has been minimized.
|
@feross yep.
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 comment has been minimized.
This comment has been minimized.
|
This error is almost impossible to miss, as the code simply will fail at |
This comment has been minimized.
This comment has been minimized.
|
@HenrikJoreteg want to send a PR? |
This comment has been minimized.
This comment has been minimized.
|
Sure, will do
|
HenrikJoreteg
added a commit
to HenrikJoreteg/eslint-config-standard-react
that referenced
this issue
Dec 3, 2015
This comment has been minimized.
This comment has been minimized.
|
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. |
This comment has been minimized.
This comment has been minimized.
|
it does raise an interesting question, however. Now what happens to the |
This comment has been minimized.
This comment has been minimized.
|
@HenrikJoreteg So, now you get an unused variable warning? |
This comment has been minimized.
This comment has been minimized.
|
right |
dcousens
added a commit
to standard/eslint-config-standard-react
that referenced
this issue
Dec 3, 2015
This comment has been minimized.
This comment has been minimized.
|
I merged that change, but, we can roll it back if necesssary. If |
This comment has been minimized.
This comment has been minimized.
|
@dcousens doing |
This comment has been minimized.
This comment has been minimized.
|
We currently have a way to define globals via |
This comment has been minimized.
This comment has been minimized.
|
not sure if this is relevant, but setting |
This comment has been minimized.
This comment has been minimized.
|
Roger that, @rstacruz! That should be a way to get around the error for sure! |
This comment has been minimized.
This comment has been minimized.
|
I agree that'd be a nice way to do it. I could see this kind of thing On Wed, Dec 9, 2015 at 9:38 PM Dan Flettre notifications@github.com wrote:
|
This comment has been minimized.
This comment has been minimized.
|
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)
I really wanna use standard on this new project but all of these things make it a pain. |
This comment has been minimized.
This comment has been minimized.
garth
commented
Jan 10, 2016
|
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? |
This comment has been minimized.
This comment has been minimized.
|
@feross perhaps we ditch |
This comment has been minimized.
This comment has been minimized.
|
As a lateral thought, it'd be nice for You can set your eslintrc to this: {
"extends": ["standard", "standard-react"]
}...and customize it as you see fit. |
This comment has been minimized.
This comment has been minimized.
|
@rstacruz that would be neat. |
This comment has been minimized.
This comment has been minimized.
|
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. |
This comment has been minimized.
This comment has been minimized.
|
@HenrikJoreteg it really does feel like it should just be an extension, so I'm not impartial to that either. |
This comment has been minimized.
This comment has been minimized.
|
Yeah, I'd like to eliminate built-in React rules (i.e. |
This comment has been minimized.
This comment has been minimized.
|
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 What can |
feross
added a commit
that referenced
this issue
Feb 6, 2016
kvz
added a commit
to freyproject/frey
that referenced
this issue
Feb 7, 2016
This comment has been minimized.
This comment has been minimized.
mike-engel
commented
Feb 9, 2016
|
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 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 {
"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 "standard": {
"jsxPragma": "h"
} |
This comment has been minimized.
This comment has been minimized.
|
@mike-engel /** @jsx h */
import { h, render } from 'preact'
// etc ...That should work |
This comment has been minimized.
This comment has been minimized.
mike-engel
commented
Feb 9, 2016
|
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 I understand the reasoning for keeping it out, however, and it's just a thought. |
This comment has been minimized.
This comment has been minimized.
|
@mike-engel Thanks for clarifying what you meant. I still think we'll keep this out of the package.json for now. |
This comment has been minimized.
This comment has been minimized.
borisirota
commented
Feb 16, 2016
|
@feross just want to add my point of view for maybe additional perspective. I totally agree that there should be zero configuration using The need of adding additional and same line to all |
This comment has been minimized.
This comment has been minimized.
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:
Magical globals do not fall into a beautiful category IMHO. |
This comment has been minimized.
This comment has been minimized.
borisirota
commented
Feb 16, 2016
|
@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 Is this the only use case for this scenario (explicit require and implicit use) ? |
kvz
added a commit
to freyproject/frey
that referenced
this issue
Mar 18, 2016
kvz
added a commit
to transloadit/uppy
that referenced
this issue
Apr 7, 2016
This comment has been minimized.
This comment has been minimized.
jsonmaur
commented
Dec 1, 2016
|
@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 |
This comment has been minimized.
This comment has been minimized.
mike-engel
commented
Dec 1, 2016
|
@jsonmaur preact didn't end up working back then for what we needed, so we never had to support standard + preact together |
This comment has been minimized.
This comment has been minimized.
|
I am using Preact in a new app I'm building. Right now, I'm just adding |
This comment has been minimized.
This comment has been minimized.
mattdesl
commented
Jan 22, 2018
|
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 comment has been minimized.
This comment has been minimized.
|
@mattdesl Re-opening to consider how to improve this in future versions. |
feross
reopened this
May 16, 2018
feross
added this to the
standard v12 milestone
May 16, 2018
This comment has been minimized.
This comment has been minimized.
stale
bot
commented
Aug 14, 2018
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
HenrikJoreteg commentedDec 2, 2015
I'm using JSX syntax to generate
virtual-dom/hcode using babel plugins (this specifically).Only problem is, now
standardcomplains becauseReactis not in scope.Not quite sure how this should be addressed, however. Because that check is useful when using React.