-
Notifications
You must be signed in to change notification settings - Fork 0
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
[Frontend] Remove react imports in agency dashboard #111
Conversation
You can leave publisher for me if you want - I don't mind cleaning that up after the settings feature. |
@@ -3,5 +3,7 @@ module.exports = { | |||
plugins: ["header"], | |||
rules: { | |||
"header/header": [2, "../license-header.js"], | |||
"react/jsx-uses-react": "off", | |||
"react/react-in-jsx-scope": "off", |
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 is just set once here, right? No need to update publisher's eslintrc or will we have to do that as well?
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.
Both apps read off the same eslintrc!
@@ -18,7 +18,7 @@ | |||
"web-vitals": "^2.1.4" | |||
}, | |||
"scripts": { | |||
"dev": "react-app-rewired start", | |||
"dev": "PORT=3001 react-app-rewired start", |
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.
Oh cool - didn't know you could set a port in a start script like that!
Cool! Curious, what's the benefit of this? Performance improvement? @mxosman I think it's okay for you to leave the Publisher parts to @terryttsai as well, though thanks for offering! You have enough on your plate with playtesting and the refactor after :) |
ed1c219
to
bcc12b9
Compare
@terryttsai what's the status of this PR? |
Should be good to go for agency-dashboard, so maybe I can get this PR merged in for now? I ran into lint errors trying to remove React imports in the publisher app and hadn't had the chance to figure out the issue |
Description of the change
As of React 17, there's no need to import React in every tsx file. See https://www.reactjs.org/blog/2020/09/22/introducing-the-new-jsx-transform.html#eslint
Will follow this up with removing React imports in Publisher and common, might have to update something or change a config to support that!
Type of change
Related issues
Closes #XXXX
Checklists
Development
This box MUST be checked by the submitter prior to merging:
These boxes should be checked by the submitter prior to merging:
Code review
These boxes should be checked by reviewers prior to merging: