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.

Sign up for GitHub

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

Jump to bottom

Devtools:-Removed unused CSS #27032

Merged
merged 2 commits into from
Jul 3, 2023
Merged

Devtools:-Removed unused CSS #27032

merged 2 commits into from
Jul 3, 2023

Conversation

Biki-das
Copy link
Contributor

@Biki-das Biki-das commented Jul 1, 2023 •
edited
Loading

I tried to check out for the CSS bundle using the eslint-plugin-css-modules, and ran it locally , seems like There are lot of unused styles and classes present .
I have attached the output below after running the lint command.

https://gist.github.com/Biki-das/647ceb7383b43cca9c8619e1dc33fe0d

All the shared.css files should not be touched i feel as they are being used randomly by some file

Thoughts @hoxyq

hoxyq
hoxyq reviewed Jul 1, 2023
View reviewed changes
packages/react-devtools-shared/src/devtools/views/Components/InspectedElement.js Outdated
element.isStrictModeNonCompliant
? styles.StrictModeNonCompliantComponent
: styles.Component
element.isStrictModeNonCompliant ? undefined : styles.Component
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe there was a typo in #22886

so the right fix would be:

Suggested change
element.isStrictModeNonCompliant ? undefined : styles.Component
element.isStrictModeNonCompliant ? styles.StrictModeNonCompliant : styles.Component

could you please update and verify that?

Other changes are fine, maybe we should explore supporting this eslint rule for react-devtools-* packages.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok sure, :-)

Copy link
Contributor Author

@Biki-das Biki-das Jul 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hoxyq i thought of supporting the same, but the library is an unmaintained one, the last update was 4 years ago.? is it fine to use such library ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes you are correct, it was a small typo!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What library are you referring to there? Sorry, didn't get that

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, you mean eslint plugin. Maybe there is a maintained one, which has the same purpose?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok @hoxyq meanwhile we can merge this one if its okay and let me research if there is something similar :-)

@Biki-das Biki-das requested a review from hoxyq July 1, 2023 17:53
@hoxyq hoxyq merged commit 53ac219 into facebook:main Jul 3, 2023
@hoxyq hoxyq mentioned this pull request Jul 4, 2023
Merged
hoxyq added a commit that referenced this pull request Jul 4, 2023
@hoxyq
React DevTools 4.27.8 -> 4.28.0 (#27051)
035a41c 
List of changes:
* Devtools:-Removed unused CSS ([Biki-das](https://github.com/Biki-das)
in [#27032](#27032))
* fix[devtools/profilingCache-test]: specify correct version gate for
test ([hoxyq](https://github.com/hoxyq) in
[#27008](#27008))
* fix[devtools/ci]: fixed incorrect condition calculation for
@reactVersion annotation ([hoxyq](https://github.com/hoxyq) in
[#26997](#26997))
* fix[ci]: fixed jest configuration not to skip too many devtools tests
([hoxyq](https://github.com/hoxyq) in
[#26955](#26955))
* fix[devtools/standalone]: update webpack configurations
([hoxyq](https://github.com/hoxyq) in
[#26963](#26963))
* fix[devtools]: check if fiber is unmounted before trying to highlight
([hoxyq](https://github.com/hoxyq) in
[#26983](#26983))
* feat[devtools]: support x_google_ignoreList source maps extension
([hoxyq](https://github.com/hoxyq) in
[#26951](#26951))
* chore[devtools]: upgrade to webpack v5
([hoxyq](https://github.com/hoxyq) in
[#26887](#26887))
* fix[devtools]: display NaN as string in values
([hoxyq](https://github.com/hoxyq) in
[#26947](#26947))
* fix: devtools cannot be closed correctly
([Jack-Works](https://github.com/Jack-Works) in
[#25510](#25510))
* Fix:- Fixed dev tools inspect mode on Shadow dom
([Biki-das](https://github.com/Biki-das) in
[#26888](#26888))
* Updated copyright text to Copyright (c) Meta Platforms, Inc. and its …
([denmo530](https://github.com/denmo530) in
[#26830](#26830))
* Fix strict mode badge URL ([ibrahemid](https://github.com/ibrahemid)
in [#26825](#26825))
@styfle styfle mentioned this pull request Jul 5, 2023
Merged
timneutkens pushed a commit to vercel/next.js that referenced this pull request Jul 6, 2023
@styfle @huozhi
chore(deps): bump react to 7118f5dd7 (#52282)
6dafc42 
Fixes #49409

### React upstream changes

- facebook/react#27045
- facebook/react#27051
- facebook/react#27032
- facebook/react#27031
- facebook/react#27029
- facebook/react#27028
- facebook/react#27027
- facebook/react#27019
- facebook/react#26954
- facebook/react#26987
- facebook/react#26985
- facebook/react#26933
- facebook/react#26625
- facebook/react#27011
- facebook/react#27008
- facebook/react#26997
- facebook/react#26989
- facebook/react#26955
- facebook/react#26963
- facebook/react#26983
- facebook/react#26914
- facebook/react#26951
- facebook/react#26977
- facebook/react#26958
- facebook/react#26940
- facebook/react#26939
- facebook/react#26887
- facebook/react#26947
- facebook/react#26945
- facebook/react#26942
- facebook/react#26938
- facebook/react#26844
- facebook/react#25510
- facebook/react#26932
- facebook/react#26896
- facebook/react#26913
- facebook/react#26888
- facebook/react#26827
- facebook/react#26889
- facebook/react#26877
- facebook/react#26873
- facebook/react#26880
- facebook/react#26842
- facebook/react#26858
- facebook/react#26754
- facebook/react#26753
- facebook/react#26881

---------

Co-authored-by: Jiachi Liu <[email protected]>
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
@Biki-das @AndyPengc12
Devtools:-Removed unused CSS (facebook#27032)
c7518f2 
I tried to check out for the CSS bundle using the
[eslint-plugin-css-modules](https://www.npmjs.com/package/eslint-plugin-css-modules),
and ran it locally , seems like There are lot of unused styles and
classes present .
I have attached the output below after running the lint command.

https://gist.github.com/Biki-das/647ceb7383b43cca9c8619e1dc33fe0d

All the shared.css files should not be touched i feel as they are being
used randomly by some file

Thoughts @hoxyq
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
@hoxyq @AndyPengc12
React DevTools 4.27.8 -> 4.28.0 (facebook#27051)
ce7fcbb 
List of changes:
* Devtools:-Removed unused CSS ([Biki-das](https://github.com/Biki-das)
in [facebook#27032](facebook#27032))
* fix[devtools/profilingCache-test]: specify correct version gate for
test ([hoxyq](https://github.com/hoxyq) in
[facebook#27008](facebook#27008))
* fix[devtools/ci]: fixed incorrect condition calculation for
@reactVersion annotation ([hoxyq](https://github.com/hoxyq) in
[facebook#26997](facebook#26997))
* fix[ci]: fixed jest configuration not to skip too many devtools tests
([hoxyq](https://github.com/hoxyq) in
[facebook#26955](facebook#26955))
* fix[devtools/standalone]: update webpack configurations
([hoxyq](https://github.com/hoxyq) in
[facebook#26963](facebook#26963))
* fix[devtools]: check if fiber is unmounted before trying to highlight
([hoxyq](https://github.com/hoxyq) in
[facebook#26983](facebook#26983))
* feat[devtools]: support x_google_ignoreList source maps extension
([hoxyq](https://github.com/hoxyq) in
[facebook#26951](facebook#26951))
* chore[devtools]: upgrade to webpack v5
([hoxyq](https://github.com/hoxyq) in
[facebook#26887](facebook#26887))
* fix[devtools]: display NaN as string in values
([hoxyq](https://github.com/hoxyq) in
[facebook#26947](facebook#26947))
* fix: devtools cannot be closed correctly
([Jack-Works](https://github.com/Jack-Works) in
[facebook#25510](facebook#25510))
* Fix:- Fixed dev tools inspect mode on Shadow dom
([Biki-das](https://github.com/Biki-das) in
[facebook#26888](facebook#26888))
* Updated copyright text to Copyright (c) Meta Platforms, Inc. and its …
([denmo530](https://github.com/denmo530) in
[facebook#26830](facebook#26830))
* Fix strict mode badge URL ([ibrahemid](https://github.com/ibrahemid)
in [facebook#26825](facebook#26825))
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
@hoxyq
Devtools:-Removed unused CSS (#27032)
437c15e 
I tried to check out for the CSS bundle using the
[eslint-plugin-css-modules](https://www.npmjs.com/package/eslint-plugin-css-modules),
and ran it locally , seems like There are lot of unused styles and
classes present .
I have attached the output below after running the lint command.

https://gist.github.com/Biki-das/647ceb7383b43cca9c8619e1dc33fe0d

All the shared.css files should not be touched i feel as they are being
used randomly by some file

Thoughts @hoxyq

DiffTrain build for commit 53ac219.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hoxyq hoxyq hoxyq approved these changes

Assignees
No one assigned
Labels
CLA Signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Biki-das @hoxyq @facebook-github-bot