This repository has been archived by the owner on Feb 23, 2024. It is now read-only.
Ignore dashicons imports #2664
Merged
Ignore dashicons imports #2664
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nerrad
added
focus: performance
|
Size Change: -168 kB (10%) 👏 Total Size: 1.57 MB
ℹ️ View Unchanged
|
senadir
approved these changes
Jun 8, 2020
Comment on lines
7
to
9
| const iconClass = [ 'dashicon', 'dashicons-arrow-down-alt2', className ] | ||
| .filter( Boolean ) | ||
| .join( ' ' ); |
There was a problem hiding this comment.
why not use classnames, a dependency we already use (that is quite lightweight).
There was a problem hiding this comment.
Because I copy/pasted from gb for this blindly :). I'll add.
| return ( | ||
| <SVG | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| viewBox="0 0 20 20" |
There was a problem hiding this comment.
not sure how will the icon render, but if we want to keep with Gutenberg new grid system, this should be changed to "-2 -2 24 24", but if the icon renders fine and sharp, we should be okay.
There was a problem hiding this comment.
Icon seems to be rendering fine, this was copied and pasted directly from the existing dashicons implementation. Since this is mostly temporary (we can replace once we can bump component versions), not sure if we need to change.
Aljullu
approved these changes
Jun 8, 2020
bin/webpack-helpers.js
Outdated
| const WebpackRTLPlugin = require( 'webpack-rtl-plugin' ); | ||
| const chalk = require( 'chalk' ); | ||
| const { omit } = require( 'lodash' ); | ||
| const NODE_ENV = process.env.NODE_ENV || 'development'; | ||
|
|
||
| const dashIconReplacmentModule = path.resolve( |
There was a problem hiding this comment.
Typo:
Suggested change
| const dashIconReplacmentModule = path.resolve( | |
| const dashIconReplacementModule = path.resolve( |
Aljullu
added
the
skip-changelog
mikejolley
approved these changes
Jun 8, 2020
Comment on lines
7
to
9
| const iconClass = [ 'dashicon', 'dashicons-arrow-down-alt2', className ] | ||
| .filter( Boolean ) | ||
| .join( ' ' ); |
| .filter( Boolean ) | ||
| .join( ' ' ); | ||
| return ( | ||
| <SVG |
There was a problem hiding this comment.
Not sure if you did or not, but I tested and some very small savings could be made if running the SVG though https://jakearchibald.github.io/svgomg/ - I do this on all icon imports I introduce.
There was a problem hiding this comment.
I just copied and pasted directly from the existing dashicons... are the savings enough to warrant implementing?
There was a problem hiding this comment.
Ya the difference is super miniscule, not worth the change imo (1byte)
There was a problem hiding this comment.
168 bytes → 138 bytes for the X
There was a problem hiding this comment.
Really? it was only 1B when I used the tool, maybe I'm using it wrong (169->168B)
Aljullu
removed
the
skip-changelog
Aljullu
added a commit
that referenced
this pull request
Jun 9, 2020
* Add 2.7.0 changelog * Add 2.7.0 testing steps * Add #2664 to readme * Update testing notes to add checklist app creation link * Add note that 'No shipping methods placeholder when they are all disabled' requires WC 4.3 * Bumping version strings to new version. * Update readme.txt date Co-authored-by: Darren Ethier <darren@roughsmootheng.in>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes: #2280
As a part of using Notice components from
@wordpress/componentsdirectly for our new notices system, thedashiconpackage ended up getting pulled in for anything using notices. This resulted in an increase of ~85kb (unminified) for all builds!While dashicons are removed in later iterations of
@wordpress/components, we're unable to bump up our dependencies yet because of issues with non-available packages in the versions of WP we support.While we do have some components using dashicons (from the package version we use) the solution is to use the
webpack.NormalModuleReplacementPluginto replace anydashiconsimports with a custom version that eliminates the excessive size.To test
This impacts anything loading our notices components. While this includes the new Product Block, because this is blocking 2.7.0, testing should primarily focus on:
Essentially testing involves: