-
Notifications
You must be signed in to change notification settings - Fork 138
Integrate Web Worker Offloading with Google Site Kit #1686
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
Conversation
| plwwo_mark_scripts_for_offloading( | ||
| array( | ||
| 'google_gtagjs', | ||
| 'googlesitekit-consent-mode', |
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 don't know how the Consent Mode is supposed to work, especially as it relates to the WP Consent API plugin. On my test site I didn't get any consent banner even without WWO active, so I don't know what to look for to see if it is working as expected.
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.
you may need to have this plugin installed? https://wordpress.org/plugins/wp-consent-api/
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.
cc: @aaemnnosttv ^^
|
@joemcgill @aaemnnosttv If someone from the Site Kit team could test that this is working as expected with a pre-release version of Site Kit, that would be very helpful. |
|
Build for testing: web-worker-offloading.zip |
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Updated build for testing which includes additional compatibility with the WP Consent API: web-worker-offloading.zip |
adamsilverstein
left a comment
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.
Looks good, left some small feedback.
|
@westonruter I gave this a few tests using the latest test build (#1686 (comment)) as well and it seemed to work as expected. However, on closer inspection, it seems there may be an issue with inline scripts as the inline script that accompanies the Google tag doesn't seem to be running (e.g.
|
Co-authored-by: Adam Silverstein <[email protected]>
|
@aaemnnosttv Is this anywhere I can test? |
|
Another build for testing: web-worker-offloading.zip |
|
|
||
| // Expose on the main tread. See <https://partytown.builder.io/forwarding-event>. | ||
| $configuration['forward'][] = 'dataLayer.push'; | ||
| $configuration['forward'][] = 'gtag'; |
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.
The addition here means that scripts in the main thread will now be able to call gtag() whereas previously it was only available to other scripts offloaded to a worker.
|
|
||
| // Expose on the main tread. See <https://partytown.builder.io/forwarding-event>. | ||
| $configuration['forward'][] = 'dataLayer.push'; | ||
| $configuration['forward'][] = 'gtag'; |
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.
The addition of this line here means that scripts in the main thread will now be able to call gtag() whereas previously it was only available to other scripts offloaded to a worker.
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 can confirm this works now 👍
|
With the recent updates here and my understanding of the expectations, this seems to be working as expected 👍 |
adamsilverstein
left a comment
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 🎉

Fixes #1455
Depends on:
wp_print_inline_script_tag()google/site-kit-wp#9726Diff in page source when activating