Migrate all React functionality to use @wordpress/element with dependency extraction #2756
Conversation
assets/js/autosuggest.js
Outdated
| /** | ||
| * WordPress dependencies. | ||
| */ | ||
| import apiFetch from '@wordpress/api-fetch'; |
There was a problem hiding this comment.
FYI: @wordpress/api-fetch will force loading lodash on the front-end which is a huge dep. I think you should stick to fetch on the front-end.
There was a problem hiding this comment.
Also recommend looking into this: https://github.com/10up/10up-toolkit/blob/develop/packages/toolkit/README.md#-react--wordpress
There are a few things you can prob dequeue on the front-end (assuming IE 11 is not supported anymore)
There was a problem hiding this comment.
@nicholasio Thanks for the tip re: fetch.
Regarding dequeuing the polyfill, we aren't supporting IE11 ourselves, but it looks like the suggestions would remove the polyfill as a dependency of wp-element for all themes and plugins that use it, not just ElasticPress, and I don't think that's something we'd want to do as a publicly distributed plugin. Thoughts @felipeelia?
We could remove wp-polyfill as a dependency of our scripts in the get_asset_info() function, but if it's a dependecy of wp-element anyway there's probably not much point.
Open to any other ideas though.
There was a problem hiding this comment.
Does the change introduced in this PR start to enqueue lodash and wp-polyfill or is that something that is already happening?
Curious to hear from @nicholasio about that anyway. It sounds like if someone out there is relying on those packages and we simply dequeue them in the name of performance, things will break. In that case, we won't be touching it in this release.
In general, that sounds more like an optimization that site owners should apply rather than something we could make in the plugin -- although the performance gain would be generally more significant for most of the sites, we obviously will only hear from the sites it actually broke what was already working before.
There was a problem hiding this comment.
You are both right, doing this optimization on the plugin wouldn't make sense! I'd however try to stick with standard apis on the front-end. Most of the @wordpress/packages are not optimized for usage outise of the editor. Most of them will load lodash (and some will load moment.js) both well know to be huge packages.
There was a problem hiding this comment.
At the very least I'd move away from @wordpress/date as @wordpress/element can be optimized by site owners that do not support IE 11 (and I know wordpress core is trying to optimize the polyfills they add as well)
Moving away from @wordpress/element means shipping react which could cause multiple react to be enqueed on the front-end if other plugins are also relying on react or wordpress/element.
There was a problem hiding this comment.
@nicholasio @wordpress/date does things that we need, namely take raw date data and display it in the site's timezone and language in JS. In the future we'd probably also want to let site owners or devs use their own date format as well. Would you recommend a different approach?
There was a problem hiding this comment.
There are smaller libraries tath can do what you need. Look at daysjs https://day.js.org/docs/en/plugin/utc
There was a problem hiding this comment.
@nicholasio Ok cool we'll check that out. @felipeelia In the meantime, this PR isn't introducing that package so it probably doesn't need to hold this up.
There was a problem hiding this comment.
I've filed an issue to look into those packages #2768
Description of the Change
Updates dependencies of scripts that were importing React from node_modules to use @WordPress packages with dependency extraction.
Also removes a number of third party dependencies in the Synonyms and Autosuggest features and replaces them with core equivalents where appropraite.
Closes #2613
Alternate Designs
NA
Possible Drawbacks
It's possible that there were compatibility issues with the scripts and other dependencies with the bundled versions of react, but everything appears to be working correctly.
Verification Process
Verify that all admin scripts are working, including the Synonyms editor, Custom Search Results and the Related Posts block.
Checklist:
Changelog Entry
Changed - Updated admin scripts to use WordPress's version of React instead of including their own.
Credits
Props @JakePT