Update @wordpress/components to v. 11.1.1 and @wordpress/base-styles to v. 3.2.0 #3457
Conversation
Aljullu
changed the title
|
Size Change: +25.1 kB (2%) Total Size: 1.14 MB
ℹ️ View Unchanged
|
Aljullu
force-pushed
the
update/wordpress-components
branch
5 times, most recently
from
36b7ccf to
54161ce
Compare
|
Using the Before: After: Most of the size increase comes from I investigated a little bit more why tl;dr: file increases seem legit and there is not an easy way to avoid them other than making optimizations upstream so components are slimmer. |
Aljullu
changed the title
Aljullu
added
skip-changelog
|
note that bundle comment shows gzipped while the image you showed above is just minified. Downshift, reakit, and spring-js are the big offenders here, but what changed between the before and after is Downshift (from 10 to 17). I don't think there's much we can do here really. Our reliance on certain components causes this. We can eliminate Downshift by using a regular select item instead of downshift and other stuff (see this PR #2281). I don't think we can do anything in this PR, but having the latest wp.components would allow us to provide fixes upstream. |
| $gap: $grid-unit-20; | ||
| $gap-small: $grid-unit-15; | ||
| $gap-smaller: $grid-unit-10; | ||
| $gap-smallest: $grid-unit-05; |
There was a problem hiding this comment.
Instead of hardcoding px values, let's use the $grid-unit variables from @wordpress/components.
| @@ -10,4 +9,4 @@ import withFilteredAttributes from '@woocommerce/base-hocs/with-filtered-attribu | |||
| import Block from './block'; | |||
| import attributes from './attributes'; | |||
|
|
|||
| export default compose( withFilteredAttributes( attributes ) )( Block ); | |||
There was a problem hiding this comment.
Since @wordpress/compose is no longer extracted, its dependency was generating a new frontend file specific for vendor files. Taking a closer look, it seems like compose() is not required here, so let's remove it.
| className="wc-block-components-chip__remove-icon" | ||
| focusable="false" | ||
| focusable={false} |
There was a problem hiding this comment.
This change was automatic when updating the snapshots and seems legit.
| expect( noPaymentMethods ).not.toBeNull(); | ||
| // We might get more than one match because the `speak()` function | ||
| // creates an extra `div` with the notice contents used for a11y. | ||
| expect( noPaymentMethods.length ).toBeGreaterThanOrEqual( 1 ); |
There was a problem hiding this comment.
Recent versions of the Notice component generate two elements with the notice text: one for the notice itself and another one used by the speak() a11y function. Our previous test was failing because it was expecting one and only one match.
| @@ -32,7 +32,7 @@ const Select = ( { | |||
| onChange( selectedItem.key ); | |||
| } } | |||
| options={ options } | |||
| value={ value } | |||
| value={ value || null } | |||
There was a problem hiding this comment.
value might be undefined, but when it changes from undefined to a value, downshift shows a warning that the component is changing from being uncontrolled to controlled. That can be fixed setting null as the default value when non is selected.
Aljullu
force-pushed
the
update/wordpress-components
branch
from
e81c312 to
1a86f0e
Compare
There was a problem hiding this comment.
Gave this a quick test - played with various blocks in editor and front end and didn't see any problems. FYI I was testing on WP 5.6-beta4.
Thanks for sorting this @Aljullu - it's great to be on recent versions, and nice to see our base CSS variables pulling from Gutenberg/core directly. Nice work 🙌
| @@ -165,7 +165,7 @@ | |||
| "reakit": "1.3.0", | |||
| "trim-html": "0.1.9", | |||
| "use-debounce": "3.4.3", | |||
| "wordpress-components": "npm:@wordpress/components@8.5.0", | |||
| "wordpress-components": "npm:@wordpress/components@11.1.1", | |||
There was a problem hiding this comment.
I'm curious why we have this dep and the other one in devDependencies. What does this mean? Please excuse my ignorance - feel free to point me to docs 😄
There was a problem hiding this comment.
Sure thing! Form the docs:
- "dependencies": Packages required by your application in production.
- "devDependencies": Packages that are only needed for local development and testing.
In our case:
wordpress-componentsis bundled in frontend scripts, so it's a dependency of the production builds. Because of that, it's listed independencies.@wordpress/componentsis extracted byDependencyExtractionWebpackPlugin, so it's not bundled in the production builds, so it's listed indevDependencies.
For reference, dependencies were re-ordered in this #1663, and there was a similar discussion about @wordpress/components: #1633 (comment).
|
Thanks for the reviews @senadir & @haszari!
Good catch! I could reproduce it too in WC Blocks |
Fixes #2140.
Fixes #3146.
Fixes #3311.
Fixes #3313.
How to test the changes in this Pull Request:
Regression testing
npm ci && npm run build.Testing #3146