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
perf: optimize image assets #16170
Conversation
✅ Deploy Preview for docs-eslint ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
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.
Nice improvement. LGTM.
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.
LGTM, tahnk you for this enhancement.
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. |
Currently svg files are available in |
Just noticed we can further optimize with this
|
Aren't we already minifying images with Line 12 in 9c53ed4
|
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:
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. |
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? |
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. |
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.
LGTM. Thanks!
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 ([#​16190](eslint/eslint#16190)) (Nicholas C. Zakas) - [`fd5d3d3`](eslint/eslint@fd5d3d3) feat: add `methodsIgnorePattern` option to object-shorthand rule ([#​16185](eslint/eslint#16185)) (Milos Djermanovic) #### Documentation - [`9f5a752`](eslint/eslint@9f5a752) docs: optimize image assets ([#​16170](eslint/eslint#16170)) (Sam Chen) - [`61b2948`](eslint/eslint@61b2948) docs: add svgo command to pre commit hook ([#​16178](eslint/eslint#16178)) (Amaresh S M) - [`784096d`](eslint/eslint@784096d) docs: improve search result UI ([#​16187](eslint/eslint#16187)) (Sam Chen) - [`d0f4cb4`](eslint/eslint@d0f4cb4) docs: use shorthand property name in example ([#​16180](eslint/eslint#16180)) (Kevin Elliott) #### Chores - [`10a6e0e`](eslint/eslint@10a6e0e) chore: remove deploy workflow for playground ([#​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>
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:Is there anything you'd like reviewers to focus on?
Nope.