-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
JSX but not React in scope #351
Comments
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.
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 |
@HenrikJoreteg want to send a PR? |
Sure, will do
|
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 |
@HenrikJoreteg So, now you get an unused variable warning? |
right |
change react-in-jsx-scope as per standard/standard#351
I merged that change, but, we can roll it back if necesssary. If |
@dcousens doing |
We currently have a way to define globals via |
not sure if this is relevant, but setting |
Roger that, @rstacruz! That should be a way to get around the error for sure! |
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 [email protected] wrote:
|
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. |
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 |
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. |
@rstacruz that would be neat. |
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. |
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 |
I am using Preact in a new app I'm building. Right now, I'm just adding |
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). |
@mattdesl Re-opening to consider how to improve this in future versions. |
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 |
React is moving away from requiring that |
@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 |
Basic JSX sanity check rules. See: standard/standard#351 (comment)
I'm using JSX syntax to generate
virtual-dom/h
code using babel plugins (this specifically).Only problem is, now
standard
complains becauseReact
is not in scope.Not quite sure how this should be addressed, however. Because that check is useful when using React.
The text was updated successfully, but these errors were encountered: