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

Rule proposal: prevent both children and dangerouslySetInnerHTML at the same time #710

Closed
lencioni opened this issue Jul 26, 2016 · 5 comments

Comments

@lencioni
Copy link
Collaborator

lencioni commented Jul 26, 2016

React will issue a warning if you try to use dangerouslySetInnerHTML and children at the same time.

https://github.com/facebook/react/blob/85dcbf83/src/renderers/dom/shared/ReactDOMComponent.js#L170

This should be easy enough to enforce for most cases with a linter rule.

Bad:

<div dangerouslySetInnerHTML={{ __html: 'HTML' }}>
  Children
</div>

Good:

<div dangerouslySetInnerHTML={{ __html: 'HTML' }} />
<div>
  Children
</div>
@petersendidit
Copy link
Contributor

I can work on this. What should this rule be named? jsx-no-children-and-dangerouslySetInnerHTML ?

@ljharb
Copy link
Member

ljharb commented Jul 26, 2016

The rule needs to be all lowercase - maybe jsx-no-danger-with-children?

@lencioni
Copy link
Collaborator Author

Since this is scoped to DOM elements, perhaps that should be part of the name? In #709 we settled on void-dom-elements-no-children, so perhaps this should be dom-elements-children-or-inner-html or dom-elements-no-inner-html-with-children (or dom-elements-no-danger-with-children, but I worry that "danger" may end up being too broad at some point)?

I think that the dangerouslySetInnerHTML is a React thing, not a JSX thing, so I'm not sure this should come with the jsx- prefix.

@ljharb
Copy link
Member

ljharb commented Jul 26, 2016

Ah yes, that's true, and it should catch React.createElement uses as well.

I think dom-elements-no-danger-with-children or dom-elements-no-inner-html-with-children is fine.

@lencioni
Copy link
Collaborator Author

lencioni commented Jul 26, 2016

Thinking about #712, which I think might be named dom-elements-no-inner-html, I think dom-elements-no-danger-with-children might be better actually. Plus it makes me laugh.

petersendidit added a commit to petersendidit/eslint-plugin-react that referenced this issue Jul 26, 2016
Prevents dangerouslySetInnerHTML and children from being used at the same time
petersendidit added a commit to petersendidit/eslint-plugin-react that referenced this issue Jul 26, 2016
Prevents dangerouslySetInnerHTML and children from being used at the same time
petersendidit added a commit to petersendidit/eslint-plugin-react that referenced this issue Jul 26, 2016
Prevents dangerouslySetInnerHTML and children from being used at the same time
petersendidit added a commit to petersendidit/eslint-plugin-react that referenced this issue Aug 8, 2016
Prevents dangerouslySetInnerHTML and children from being used at the same time
petersendidit added a commit to petersendidit/eslint-plugin-react that referenced this issue Aug 13, 2016
Prevents dangerouslySetInnerHTML and children from being used at the same time
fson added a commit to fson/create-react-app that referenced this issue Sep 21, 2016
New rules:
* `react/no-danger-with-children` (jsx-eslint/eslint-plugin-react#710)
* `react/no-find-dom-node` (jsx-eslint/eslint-plugin-react#678)
* `react/style-prop-object` (jsx-eslint/eslint-plugin-react#715)
gaearon pushed a commit to facebook/create-react-app that referenced this issue Sep 22, 2016
* Update eslint-plugin-react and enable new rules

New rules:
* `react/no-danger-with-children` (jsx-eslint/eslint-plugin-react#710)
* `react/no-find-dom-node` (jsx-eslint/eslint-plugin-react#678)
* `react/style-prop-object` (jsx-eslint/eslint-plugin-react#715)

* Remove react/no-find-dom-node for now
feiqitian pushed a commit to feiqitian/create-react-app that referenced this issue Oct 25, 2016
* Update eslint-plugin-react and enable new rules

New rules:
* `react/no-danger-with-children` (jsx-eslint/eslint-plugin-react#710)
* `react/no-find-dom-node` (jsx-eslint/eslint-plugin-react#678)
* `react/style-prop-object` (jsx-eslint/eslint-plugin-react#715)

* Remove react/no-find-dom-node for now
rrdelaney pushed a commit to rrdelaney/reason-scripts that referenced this issue May 23, 2018
* Update eslint-plugin-react and enable new rules

New rules:
* `react/no-danger-with-children` (jsx-eslint/eslint-plugin-react#710)
* `react/no-find-dom-node` (jsx-eslint/eslint-plugin-react#678)
* `react/style-prop-object` (jsx-eslint/eslint-plugin-react#715)

* Remove react/no-find-dom-node for now
sven3270350 added a commit to sven3270350/react-typescript that referenced this issue Aug 11, 2022
* Update eslint-plugin-react and enable new rules

New rules:
* `react/no-danger-with-children` (jsx-eslint/eslint-plugin-react#710)
* `react/no-find-dom-node` (jsx-eslint/eslint-plugin-react#678)
* `react/style-prop-object` (jsx-eslint/eslint-plugin-react#715)

* Remove react/no-find-dom-node for now
OrdinalKing pushed a commit to OrdinalKing/create-react-app-ts-redux-saga-mui that referenced this issue Aug 26, 2022
* Update eslint-plugin-react and enable new rules

New rules:
* `react/no-danger-with-children` (jsx-eslint/eslint-plugin-react#710)
* `react/no-find-dom-node` (jsx-eslint/eslint-plugin-react#678)
* `react/style-prop-object` (jsx-eslint/eslint-plugin-react#715)

* Remove react/no-find-dom-node for now
SmartCodiDev added a commit to SmartCodiDev/redux-saga-mui that referenced this issue May 31, 2024
* Update eslint-plugin-react and enable new rules

New rules:
* `react/no-danger-with-children` (jsx-eslint/eslint-plugin-react#710)
* `react/no-find-dom-node` (jsx-eslint/eslint-plugin-react#678)
* `react/style-prop-object` (jsx-eslint/eslint-plugin-react#715)

* Remove react/no-find-dom-node for now
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants