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] Optimize Images (part 1) #19972

Closed
wants to merge 2 commits into from
Closed

[DevTools] Optimize Images (part 1) #19972

wants to merge 2 commits into from

Conversation

ilhamsyahids
Copy link
Contributor

@ilhamsyahids ilhamsyahids commented Oct 6, 2020 •
edited
Loading

Summary

Image file size has been reduced by 25%

Details

File Before After Percent reduction
/packages/react-devtools-extensions/icons/128-development.png 5.08kb 2.28kb 55.20%
/packages/react-devtools-extensions/icons/128-unminified.png 5.08kb 2.28kb 55.20%
/packages/react-devtools-extensions/icons/128-deadcode.png 5.08kb 2.28kb 55.20%
/packages/react-devtools/icons/icon128.png 4.47kb 2.10kb 53.03%
/packages/react-devtools-extensions/icons/128-production.png 4.47kb 2.10kb 53.03%
/packages/react-devtools-extensions/icons/48-unminified.png 1.95kb 1.03kb 47.38%
/packages/react-devtools-extensions/icons/48-deadcode.png 1.95kb 1.03kb 47.38%
/packages/react-devtools-extensions/icons/48-development.png 1.95kb 1.03kb 47.38%
/packages/react-devtools-extensions/icons/128-outdated.png 4.06kb 2.17kb 46.71%
/packages/react-devtools-extensions/icons/128-disabled.png 4.68kb 2.63kb 43.76%
/packages/react-devtools-extensions/icons/48-production.png 1.63kb 0.92kb 43.55%
/packages/react-devtools-extensions/icons/32-deadcode.png 1.29kb 0.77kb 40.21%
/packages/react-devtools-extensions/icons/32-development.png 1.29kb 0.77kb 40.21%
/packages/react-devtools-extensions/icons/32-unminified.png 1.29kb 0.77kb 40.21%
/packages/react-devtools-extensions/icons/48-outdated.png 1.53kb 0.99kb 35.48%
/packages/react-devtools-extensions/icons/32-production.png 1.04kb 0.69kb 34.08%
/packages/react-devtools-extensions/icons/32-outdated.png 1.07kb 0.76kb 29.04%
/packages/react-devtools-extensions/icons/48-disabled.png 1.53kb 1.11kb 27.17%
/packages/react-devtools-extensions/icons/32-disabled.png 1.07kb 0.84kb 21.61%
/packages/react-devtools-extensions/icons/16-development.png 0.62kb 0.55kb 11.60%
/packages/react-devtools-extensions/icons/16-deadcode.png 0.62kb 0.55kb 11.60%
/packages/react-devtools-extensions/icons/16-unminified.png 0.62kb 0.55kb 11.60%
/packages/react-devtools-scheduling-profiler/src/assets/profilerBrowser.png 75.65kb 67.21kb 11.16%
/packages/react-devtools-extensions/icons/16-production.png 0.53kb 0.49kb 8.06%
/packages/react-devtools-extensions/icons/16-outdated.png 0.54kb 0.53kb 3.41%
Total : 129.12kb 96.40kb 25.34%

Test Plan

@facebook-github-bot
Copy link

Hi @ilhamsyahids!

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.

In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

@sizebot
Copy link

sizebot commented Oct 6, 2020

No significant bundle size changes to report.

Size changes (experimental)

Generated by 🚫 dangerJS against ad7f857

@codesandbox-ci
Copy link

codesandbox-ci bot commented Oct 6, 2020

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit ad7f857:

Sandbox Source
React Configuration

@sizebot
Copy link

sizebot commented Oct 6, 2020

No significant bundle size changes to report.

Size changes (stable)

Generated by 🚫 dangerJS against ad7f857

@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@gaearon
Copy link
Collaborator

gaearon commented Oct 9, 2020

How do we know this is correct? Can you provide screenshots before and after?

@IDrissAitHafid
Copy link
Contributor

IDrissAitHafid commented Oct 9, 2020 •
edited
Loading

@gaearon, you can see the changes by viewing the changed files, and click on Display on rich diff for each file:
image

@eps1lon
Copy link
Collaborator

eps1lon commented Oct 9, 2020

Execute document.querySelectorAll('[aria-label*="rich diff"]').forEach((btn, i) => setTimeout(() => btn.click(), i * 1000)) in the chrome devtools console for https://github.com/facebook/react/pull/19972/files for easier review.

These look all fine to me.

@ilhamsyahids
Copy link
Contributor Author

@gaearon yes, you can always check it in files changes and check for Display on rich diff

Thanks to @IDrissAitHafid for answering it and @eps1lon for providing easier code to check it.

@ilhamsyahids
Copy link
Contributor Author

@gaearon is there anything that I should change?

@bvaughn
Copy link
Contributor

bvaughn commented Oct 16, 2020

@ilhamsyahids How'd you generate these?

@ilhamsyahids
Copy link
Contributor Author

@bvaughn using bot/plugin that scan the images in the project and change them one by one with simplified size

@bvaughn
Copy link
Contributor

bvaughn commented Oct 16, 2020

@ilhamsyahids Which bot? (Is this reproducible when we need to update the images.)

@ilhamsyahids
Copy link
Contributor Author

@bvaughn Imgbot

@kachkaev
Copy link
Contributor

@ilhamsyahids could you share reproducible commands or instructions?

@ilhamsyahids
Copy link
Contributor Author

@kachkaev I'm not sure, because its integrated with my account and its generated automatically

@ilhamsyahids
Copy link
Contributor Author

@bvaughn @kachkaev is there any new image? Or there is something wrong using bot in this case?

@bvaughn
Copy link
Contributor

bvaughn commented Oct 16, 2020

@bvaughn @kachkaev is there any new image? Or there is something wrong using bot in this case?

@ilhamsyahids There will likely be new images, yes.

The concern behind these questions are: How does someone else reproduce this result if you're unavailable.

@ilhamsyahids
Copy link
Contributor Author

@bvaughn for instructions bot itself can be access in the website, and its completely free for open source.

@ilhamsyahids
Copy link
Contributor Author

@bvaughn anyway, do you find something wrong in this PR or the result? Bot only work for images, in this PR I also restyle svg file. Before add new images, maybe we can optimize the old images.

@IDrissAitHafid
Copy link
Contributor

@ilhamsyahids I think there may be some security concerns like steganography or some hidden script especially because we have no clue what algorithms that bot uses or what it is doing with the images.
I may be exaggerating, but I think I should mention this.

@ilhamsyahids
Copy link
Contributor Author

@ilhamsyahids I think there may be some security concerns like steganography or some hidden script especially because we have no clue what algorithms that bot uses or what it is doing with the images.
I may be exaggerating, but I think I should mention this.

@IDrissAitHafid For code itself, you can check here https://github.com/dabutvin/Imgbot and it has been offered by Github Student Developer, check this out https://education.github.com/pack?tag=Infrastructure+%26+APIs
Also in the official website, it says that it has been used in several company.

@ilhamsyahids
Copy link
Contributor Author

Hi, is there anything that I can update for this PR?

@bvaughn bvaughn changed the base branch from master to main July 15, 2021 00:46
bvaughn
bvaughn suggested changes Jul 15, 2021
View reviewed changes
Copy link
Contributor

@bvaughn bvaughn left a comment

Choose a reason for hiding this comment

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

@ilhamsyahids We'd need a Node script to run that automatically generates/updates these images, so that it could be done (in CI) as part of our build and release process. Unfortunately visiting a website and manually updating the images doesn't really fit with how we manage releases, at this scale.

@ilhamsyahids
Copy link
Contributor Author

@bvaughn We shouldn't visit the web and manually update. Only add it to the repo and it works as a trigger to create PR for optimized images. But, if we want to create Node script, alternatively, we can use Imagemin. What do you think?

@bvaughn
Copy link
Contributor

bvaughn commented Jul 15, 2021

I think I'm open for either, so long as it's an automated thing.

@ilhamsyahids
Copy link
Contributor Author

For option one, I cant proceed (only who is part of the organization), let me try the second one.

@ilhamsyahids ilhamsyahids mentioned this pull request Jul 27, 2021
Merged
@ilhamsyahids
Copy link
Contributor Author

I made another PR, #21968 for automated optimized images. Please kindly check it.

cc: @bvaughn

@bvaughn bvaughn changed the title Optimize Images [DevTools] Optimize Images (part 1) Jul 27, 2021
@bvaughn bvaughn closed this Aug 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bvaughn bvaughn bvaughn requested changes

Assignees
No one assigned
Labels
CLA Signed Component: Developer Tools
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@ilhamsyahids @facebook-github-bot @sizebot @gaearon @IDrissAitHafid @eps1lon @bvaughn @kachkaev