-
-
Notifications
You must be signed in to change notification settings - Fork 32.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
[core] Enforce namespace import for ReactDOM #36208
Conversation
.eslintrc.js
Outdated
{ | ||
message: | ||
"Do not import default from ReactDOM. Use a namespace import (import * as ReactDOM from 'react-dom';) instead.", | ||
selector: 'ImportDeclaration[source.value="react-dom"] ImportDefaultSpecifier', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
selector: 'ImportDeclaration[source.value=/^react-dom(\\u002F(server|client))?$/] ImportDefaultSpecifier'
– this could work with only one additional entry using a regex but its clearer to separate out all the error messages
12edb1f
to
14845dd
Compare
@@ -1,7 +1,7 @@ | |||
import * as React from 'react'; | |||
import { createPortal } from 'react-dom'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd replace lines like this one with a namespace import as well to be consistent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I missed these kinds of imports, let me see if eslint can find them too
14845dd
to
88c3d1c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good!
b823dc5
to
6ebf94e
Compare
import { createContext } from 'react'; | ||
import * as React from 'react'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This case is interesting. I think that the initial problem was that:
import ReactDOMServer from 'react-dom/server';
is wrong because React plans to no longer export default facebook/react#18102.
This change goes after something else but why not.
Closes #36198
This PR updates the eslint
no-restricted-syntax
rule to enforceimport * as ReactDOM
(react-dom
,react-dom/client
,react-dom/server
) and give an error for:import ReactDOM from 'react-dom'
)import { createPortal } from 'react-dom'
)Example lint step: https://app.circleci.com/pipelines/github/mui/material-ui/91534/workflows/63fb99e1-8ccd-4c53-9106-401ead3f8c2d/jobs/485842/parallel-runs/0/steps/0-108