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

perf: optimize image assets #16170

Merged
merged 3 commits into from Aug 10, 2022
Merged

Conversation

chenxsan
Copy link
Contributor

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[x] Other, please explain:

Optimize svg images with svgo removing redundant and useless information.

What changes did you make? (Give an overview)

Just run svgo command:

$ npx svgo -r -f ./docs/src/assets/images
Processing directory './docs/src/assets/images':


example-dowhilestatement.svg:
Done in 47 ms!
7.905 KiB - 50.6% = 3.906 KiB

example-hello-world.svg:
Done in 6 ms!
3.484 KiB - 47.1% = 1.844 KiB

example-forinstatement.svg:
Done in 18 ms!
11.619 KiB - 52.3% = 5.54 KiB

example-forstatement.svg:
Done in 17 ms!
16.751 KiB - 50.5% = 8.288 KiB

example-switchstatement-has-default.svg:
Done in 24 ms!
22.586 KiB - 52.2% = 10.791 KiB

example-ifstatement.svg:
Done in 9 ms!
9.64 KiB - 51.6% = 4.667 KiB

example-trystatement-try-catch.svg:
Done in 13 ms!
15.222 KiB - 49% = 7.762 KiB

example-trystatement-try-catch-finally.svg:
Done in 11 ms!
11.217 KiB - 46.2% = 6.034 KiB

example-trystatement-try-finally.svg:
Done in 11 ms!
11 KiB - 47.6% = 5.766 KiB

example-when-there-is-a-function-f.svg:
Done in 8 ms!
7.754 KiB - 50.9% = 3.806 KiB

example-switchstatement.svg:
Done in 18 ms!
18.793 KiB - 51.6% = 9.101 KiB

example-when-there-is-a-function-g.svg:
Done in 3 ms!
3.306 KiB - 48.5% = 1.701 KiB

helo.svg:
Done in 11 ms!
9.702 KiB - 50.7% = 4.785 KiB

example-whilestatement.svg:
Done in 14 ms!
13.826 KiB - 52.6% = 6.552 KiB

loop-event-example-for-1.svg:
Done in 7 ms!
6.885 KiB - 53.2% = 3.219 KiB

loop-event-example-for-3.svg:
Done in 7 ms!
9.426 KiB - 50.1% = 4.708 KiB

loop-event-example-for-2.svg:
Done in 7 ms!
8.994 KiB - 50.7% = 4.434 KiB

loop-event-example-for-4.svg:
Done in 9 ms!
9.429 KiB - 49.7% = 4.746 KiB

loop-event-example-while-1.svg:
Done in 6 ms!
6.552 KiB - 54.2% = 3 KiB

loop-event-example-while-3.svg:
Done in 8 ms!
9.5 KiB - 51.9% = 4.573 KiB

loop-event-example-while-2.svg:
Done in 5 ms!
6.855 KiB - 52.7% = 3.239 KiB

loop-event-example-for-5.svg:
Done in 12 ms!
12.005 KiB - 50% = 6 KiB

arrow-right.svg:
Done in 1 ms!
0.223 KiB - 13.2% = 0.193 KiB

arrow-left.svg:
Done in 0 ms!
0.221 KiB - 11.9% = 0.194 KiB

arrow-top-right.svg:
Done in 0 ms!
0.227 KiB - 12.5% = 0.198 KiB

copy.svg:
Done in 1 ms!
0.736 KiB - 41.4% = 0.432 KiB

chevron-down.svg:
Done in 0 ms!
0.229 KiB - 17.9% = 0.188 KiB

discord.svg:
Done in 4 ms!
3.313 KiB - 64.3% = 1.183 KiB

facebook.svg:
Done in 1 ms!
0.48 KiB - 24.8% = 0.361 KiB

correct.svg:
Done in 0 ms!
0.365 KiB - 8.6% = 0.334 KiB

features-list-icon.svg:
Done in 0 ms!
0.287 KiB - 11.9% = 0.253 KiB

github-img.svg:
Done in 1 ms!
1.159 KiB - 30.2% = 0.809 KiB

example-forstatement-for-ever.svg:
Done in 7 ms!
4.984 KiB - 53.9% = 2.298 KiB

github-large.svg:
Done in 1 ms!
1.079 KiB - 45.2% = 0.591 KiB

github.svg:
Done in 1 ms!
1.126 KiB - 24.7% = 0.848 KiB

github-small.svg:
Done in 2 ms!
1.171 KiB - 39.1% = 0.713 KiB

google.svg:
Done in 1 ms!
1.202 KiB - 67.5% = 0.391 KiB

languages.svg:
Done in 2 ms!
1.49 KiB - 8.7% = 1.36 KiB

incorrect.svg:
Done in 1 ms!
0.373 KiB - 8.6% = 0.341 KiB

learn-more-arrow.svg:
Done in 1 ms!
0.269 KiB - 18.9% = 0.218 KiB

link.svg:
Done in 1 ms!
1.021 KiB - 65.8% = 0.349 KiB

linkedin.svg:
Done in 1 ms!
0.821 KiB - 36% = 0.525 KiB

menu.svg:
Done in 0 ms!
0.214 KiB - 10.5% = 0.191 KiB

minus-circle.svg:
Done in 1 ms!
0.311 KiB - 21.1% = 0.245 KiB

npm.svg:
Done in 1 ms!
0.66 KiB - 26.5% = 0.485 KiB

open-collectione-mono.svg:
Done in 1 ms!
1.014 KiB - 32.3% = 0.687 KiB

opencollective-img.svg:
Done in 0 ms!
0.831 KiB - 18% = 0.682 KiB

plus-circle.svg:
Done in 0 ms!
0.318 KiB - 20.9% = 0.252 KiB

twitter.svg:
Done in 1 ms!
0.879 KiB - 44.4% = 0.488 KiB

search.svg:
Done in 1 ms!
0.402 KiB - 35.2% = 0.261 KiB

brand-colors.svg:
Done in 0 ms!
0.234 KiB - 10.4% = 0.21 KiB

eslint-logo-color.svg:
Done in 5 ms!
4.403 KiB - 34.4% = 2.889 KiB

eslint-logo-white.svg:
Done in 6 ms!
4.404 KiB - 34.9% = 2.867 KiB

github-icon-mono.svg:
Done in 2 ms!
1.292 KiB - 36.5% = 0.82 KiB

example-ifstatement-chain.svg:
Done in 17 ms!
16.491 KiB - 51.5% = 8.001 KiB

dependency.svg:
Done in 73 ms!
29.861 KiB - 39.9% = 17.96 KiB

Is there anything you'd like reviewers to focus on?

Nope.

@eslint-github-bot eslint-github-bot bot added triage An ESLint team member will look at this issue soon chore This change is not user-facing labels Jul 31, 2022
@netlify
Copy link

netlify bot commented Jul 31, 2022

Deploy Preview for docs-eslint ready!

Name Link
🔨 Latest commit 623161f
🔍 Latest deploy log https://app.netlify.com/sites/docs-eslint/deploys/62ea09f6873a45000896b27b
😎 Deploy Preview https://deploy-preview-16170--docs-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link
Member

@harish-sethuraman harish-sethuraman left a comment

Choose a reason for hiding this comment

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

Nice improvement. LGTM.

@snitin315 snitin315 added accepted There is consensus among the team that this change meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels Aug 2, 2022
Copy link
Contributor

@snitin315 snitin315 left a comment

Choose a reason for hiding this comment

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

LGTM, tahnk you for this enhancement.

@amareshsm
Copy link
Member

can we minify all other svg files also?

 npx svgo -r -f  ./

@chenxsan
Copy link
Contributor Author

chenxsan commented Aug 3, 2022

can we minify all other svg files also?

 npx svgo -r -f  ./

Sure we can, but I'm not sure it's necessary, since others might not be eslint site related.

@amareshsm
Copy link
Member

Currently svg files are available in docs directory and I guess only one file is missing docs/src/static/icon.svg .

@chenxsan
Copy link
Contributor Author

chenxsan commented Aug 3, 2022

Just noticed we can further optimize with this --multipass argument:

--multipass Pass over SVGs multiple times to ensure all optimizations are applied

@mdjermanovic
Copy link
Member

Aren't we already minifying images with imagemin?

"images": "imagemin '_site/assets/images' --out-dir='_site/assets/images'",

@chenxsan
Copy link
Contributor Author

chenxsan commented Aug 5, 2022

Aren't we already minifying images with imagemin?

"images": "imagemin '_site/assets/images' --out-dir='_site/assets/images'",

Ah yeah, didn't notice that.

So the question comes down to whether it makes sense to optimize the size of source assets.

Here're some pros of optimizing source assets:

  1. we can save anyone cloning this repo some bits
  2. we don't have to run imagemin every time to optimize those non-optimized assets which could make the build faster

A pre commit hook like #16178 makes much more sense than a imagemin script IMHO although we can still have the latter just in case.

@mdjermanovic
Copy link
Member

An advantage of optimizing at build time is that it can produce a better output when the tool improves. It's also more reliable because commit hooks can be skipped. I'm not sure what the common practice is, but it looks like the original intent was to optimize images at build time only. @nzakas?

@nzakas
Copy link
Member

nzakas commented Aug 6, 2022

The idea was that it’s hard to get everyone to optimize images when they are checked in. Precommit hooks don’t catch all instances either, because people can use the GitHub web app, desktop app, or mobile app to add files. So in order to ensure the best version of the assets get deployed, we optimize during build time. I wouldn’t want to change that.

That said, there’s certainly no harm in optimizing the checked-in images.

Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@nzakas nzakas merged commit 9f5a752 into eslint:main Aug 10, 2022
@chenxsan chenxsan deleted the perf/optimize-image-assets branch August 10, 2022 04:30
crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this pull request Aug 23, 2022
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [eslint](https://eslint.org) ([source](https://github.com/eslint/eslint)) | devDependencies | minor | [`8.21.0` -> `8.22.0`](https://renovatebot.com/diffs/npm/eslint/8.21.0/8.22.0) |

---

### Release Notes

<details>
<summary>eslint/eslint</summary>

### [`v8.22.0`](https://github.com/eslint/eslint/releases/tag/v8.22.0)

[Compare Source](eslint/eslint@v8.21.0...v8.22.0)

#### Features

-   [`2b97607`](eslint/eslint@2b97607) feat: Implement caching for FlatESLint ([#&#8203;16190](eslint/eslint#16190)) (Nicholas C. Zakas)
-   [`fd5d3d3`](eslint/eslint@fd5d3d3) feat: add `methodsIgnorePattern` option to object-shorthand rule ([#&#8203;16185](eslint/eslint#16185)) (Milos Djermanovic)

#### Documentation

-   [`9f5a752`](eslint/eslint@9f5a752) docs: optimize image assets ([#&#8203;16170](eslint/eslint#16170)) (Sam Chen)
-   [`61b2948`](eslint/eslint@61b2948) docs: add svgo command to pre commit hook ([#&#8203;16178](eslint/eslint#16178)) (Amaresh  S M)
-   [`784096d`](eslint/eslint@784096d) docs: improve search result UI ([#&#8203;16187](eslint/eslint#16187)) (Sam Chen)
-   [`d0f4cb4`](eslint/eslint@d0f4cb4) docs: use shorthand property name in example ([#&#8203;16180](eslint/eslint#16180)) (Kevin Elliott)

#### Chores

-   [`10a6e0e`](eslint/eslint@10a6e0e) chore: remove deploy workflow for playground ([#&#8203;16186](eslint/eslint#16186)) (Milos Djermanovic)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox.

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzMi4xNTYuMSIsInVwZGF0ZWRJblZlciI6IjMyLjE1Ni4xIn0=-->

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1506
Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org>
Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Feb 7, 2023
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Feb 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion chore This change is not user-facing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants