-
Notifications
You must be signed in to change notification settings - Fork 219
Reduce number of dependencies for the product categories list block #771
Reduce number of dependencies for the product categories list block #771
Conversation
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.
Thanks for working on this @mikejolley, that was a big refactor and it results in much lower file sizes, good job! 👏
I added some comments below, but overall looking good. 👍
ie11 compatible for each on nodelist Co-Authored-By: Albert Juhé Lluveras <[email protected]>
…github.com/woocommerce/woocommerce-gutenberg-products-block into update/product-category-block-dependencies
Ready for another look @Aljullu. I handled the CSS change too (wasn't difficult) and cleaned up the ID generation using a HOC. Let me know your thoughts and if you think it's good for 2.3. |
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.
Thanks for the fixes @mikejolley! Everything is working fine and code looks good. I just added one minor comment but once it's addressed this is ready to go.
}; | ||
|
||
export default withInstanceId( ProductCategoriesBlock ); | ||
export default withComponentID( ProductCategoriesBlock ); |
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.
I know that's very nitpicky, but what do you think about renaming withComponentID
and componentID
to withComponentId
and componentId
(notice the camel case)? This way it's consistent with withInstanceId
and instanceId
.
This also seems to be the consensus they arrived in Gutenberg: WordPress/gutenberg#6741.
This PR improves how JS powered blocks work on the frontend, reducing the overall number of dependencies. In testing, this reduced downloaded JS from 4.8MB to 2.2MB.
Fixes #762
The changes to the webpack config are to prevent wp.element and lodash being enqueued on the frontend - we can use React only here.
How to test the changes in this Pull Request:
build/frontend.deps.js
only contains["react","react-dom","wp-i18n","wp-polyfill"]
Changelog