-
Notifications
You must be signed in to change notification settings - Fork 93
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
Add web worker offloading plugin #1247
Conversation
b1229da
to
88ff247
Compare
@westonruter @adamsilverstein I have converted this into a standalone plugin. Next, I'll add a settings page to opt-in offloading for scripts listed in the Partytown documentation (https://partytown.builder.io/common-services). This way, the WordPress plugin team won't classify it as a "framework" plugin, as @westonruter previously mentioned. |
9d4c497
to
e7668c6
Compare
* Web Worker module allows to run scripts within a worker thread with the help of Partytown library.
* Replace output buffering with get_echo() function * Update creation of expected string of script tag with type="text/partytown"
* Removed scripts from done items in global $wp_scripts
- Include partytown library files at plugin release.
Co-authored-by: Weston Ruter <westonruter@google.com>
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.
Just some trivial suggestions left from me. Pre-approving so that once @adamsilverstein approves this can finally be merged!
Co-authored-by: Weston Ruter <westonruter@google.com>
Co-authored-by: Weston Ruter <westonruter@google.com>
array_key_exists( $handle, wp_scripts()->registered ) && | ||
in_array( 'web-worker-offloading', wp_scripts()->registered[ $handle ]->deps, true ) | ||
) { | ||
$html_processor = new WP_HTML_Tag_Processor( $tag ); |
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!
Code looks good to me, doing some manual testing next. |
Testing this locally with the sample code from the ticket description, I noticed the script handle is different, I had to use Once I did that, the tag got the partytown type added: question: do we want to remove the I didn't test actual Partytown functionality yet, that will be easier to do once we ship the plugin. Are we planning add script specific handling (eg analytics) in the future? |
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.
Tremendous!
Ah, that's my bad. I updated the slug in the codebase but didn't do it in the description. Updated now.
You're right. This can be removed as well. Or the |
Co-authored-by: Weston Ruter <westonruter@google.com>
In 44c469c I addressed a case where a script may already have the ✅ Good to merge. |
Summary
Fixes #176
Previously: #556
Relevant technical choices
wwo_configuration
filter which can be used to update web worker configurations.Testing steps
Enable this performance plugin or use
wp-env
in which this plugin is enabled by default on this branch.Add a JS script with
web-worker-offloading
as a dependency. When the page is loaded, the JS script should be loaded within the web worker.Use
wwo_configuration
hook to update web worker configurations.