Gutenberg RTC: Support HTTP polling provider#47485
Conversation
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! Mu Wpcom plugin:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Wpcomsh plugin:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
There was a problem hiding this comment.
Pull request overview
Adds support for using Gutenberg Real-Time Collaboration (RTC) with the default HTTP polling provider by avoiding injecting the Jetpack MU script when it would unnecessarily override Gutenberg’s default provider behavior.
Changes:
- Detect when the only configured RTC provider is
http-pollingand skip enqueuing/injecting thegutenberg-rtcasset + inline config in that case.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
projects/packages/jetpack-mu-wpcom/src/features/gutenberg-rtc/gutenberg-rtc.php
Show resolved
Hide resolved
projects/packages/jetpack-mu-wpcom/src/features/gutenberg-rtc/gutenberg-rtc.php
Outdated
Show resolved
Hide resolved
projects/packages/jetpack-mu-wpcom/src/features/gutenberg-rtc/gutenberg-rtc.php
Show resolved
Hide resolved
Code Coverage SummaryCoverage changed in 2 files.
Coverage check overridden by
Coverage tests to be added later
|
aba34f0 to
97826d5
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
projects/packages/jetpack-mu-wpcom/src/features/gutenberg-rtc/gutenberg-rtc.php
Outdated
Show resolved
Hide resolved
projects/packages/jetpack-mu-wpcom/tests/php/features/gutenberg-rtc/Gutenberg_RTC_Test.php
Show resolved
Hide resolved
97826d5 to
5f59c52
Compare
5f59c52 to
2a02e4a
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
projects/packages/jetpack-mu-wpcom/src/features/gutenberg-rtc/gutenberg-rtc.php:65
wpcom_enqueue_gutenberg_rtc_assets()computes$providersonce, but then callswpcom_get_gutenberg_rtc_providers()again when building the inline data. Since this runs filters, the second call can be inconsistent (or expensive) and makes behavior dependent on filter side effects. Reuse the already-computed$providerswhen encoding the inline script.
$providers = wpcom_get_gutenberg_rtc_providers();
// If HTTP polling (Gutenberg’s built-in default provider when this script isn’t enqueued)
// is the only provider being used, then we don’t need to inject any assets since that’s
// already the default behavior.
if ( count( $providers ) === 1 && in_array( 'http-polling', $providers, true ) ) {
return;
}
$handle = jetpack_mu_wpcom_enqueue_assets( 'gutenberg-rtc', array( 'js' ) );
$data = wp_json_encode(
array(
'providers' => wpcom_get_gutenberg_rtc_providers(),
),
JSON_UNESCAPED_SLASHES | JSON_HEX_TAG | JSON_HEX_AMP
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
projects/packages/jetpack-mu-wpcom/src/features/gutenberg-rtc/gutenberg-rtc.php
Show resolved
Hide resolved
| public function test_wpcom_enqueue_gutenberg_rtc_assets_skips_http_polling_only() { | ||
| add_filter( 'wpcom_is_gutenberg_rtc_enabled', '__return_true' ); | ||
| add_filter( | ||
| 'wpcom_gutenberg_rtc_providers', | ||
| function () { | ||
| return array( 'http-polling' ); | ||
| } | ||
| ); | ||
|
|
||
| wpcom_enqueue_gutenberg_rtc_assets(); | ||
|
|
||
| $this->assertFalse( wp_script_is( 'jetpack-mu-wpcom-gutenberg-rtc', 'enqueued' ) ); | ||
| } |
There was a problem hiding this comment.
These enqueue-related tests can be order-dependent because the global script registry persists across tests. If another test enqueues jetpack-mu-wpcom-gutenberg-rtc first, test_wpcom_enqueue_gutenberg_rtc_assets_skips_http_polling_only() may fail even though the code is correct. Reset $wp_scripts (as other tests in this package do) or explicitly dequeue/deregister the script in tear_down() before asserting enqueue state.
2a02e4a to
cde0de9
Compare
3932e7c to
a0c2ee7
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
projects/packages/jetpack-mu-wpcom/tests/php/features/gutenberg-rtc/Gutenberg_RTC_Test.php:75
- Test isolation: storing/restoring $wp_scripts/$wp_styles without resetting them means any enqueued/registered scripts from a test can leak into later tests (the stored value is the same object reference). In this repo other tests explicitly reset the script registry (e.g. by setting $wp_scripts = null or creating a new WP_Scripts) to avoid flakiness. Consider resetting $wp_scripts/$wp_styles to a clean state in set_up (after saving originals), so each test starts with an empty registry, then restore originals in tear_down.
public function set_up(): void {
parent::set_up();
global $wp_settings_fields, $wp_scripts, $wp_styles;
$this->original_wp_settings_fields = $wp_settings_fields;
$this->original_wp_scripts = $wp_scripts;
$this->original_wp_styles = $wp_styles;
}
/**
* Clean up filters and restore global state after each test.
*/
public function tear_down(): void {
global $wp_settings_fields, $wp_scripts, $wp_styles;
$wp_settings_fields = $this->original_wp_settings_fields; // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited
$wp_scripts = $this->original_wp_scripts; // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited
$wp_styles = $this->original_wp_styles; // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited
remove_all_filters( 'wpcom_is_gutenberg_rtc_enabled' );
remove_all_filters( 'wpcom_gutenberg_rtc_providers' );
parent::tear_down();
}
projects/packages/jetpack-mu-wpcom/src/features/gutenberg-rtc/gutenberg-rtc.php:65
- wpcom_enqueue_gutenberg_rtc_assets() calls wpcom_get_gutenberg_rtc_providers() twice (once into $providers and again when building the inline data). If provider resolution is filter-driven, calling it twice can be inconsistent and makes debugging harder. Reuse the already-computed $providers when building the JSON data.
$providers = wpcom_get_gutenberg_rtc_providers();
// If HTTP polling (Gutenberg’s built-in default provider when this script isn’t enqueued)
// is the only provider being used, then we don’t need to inject any assets since that’s
// already the default behavior.
if ( count( $providers ) === 1 && in_array( 'http-polling', $providers, true ) ) {
return;
}
$handle = jetpack_mu_wpcom_enqueue_assets( 'gutenberg-rtc', array( 'js' ) );
$data = wp_json_encode(
array(
'providers' => wpcom_get_gutenberg_rtc_providers(),
),
JSON_UNESCAPED_SLASHES | JSON_HEX_TAG | JSON_HEX_AMP
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
projects/packages/jetpack-mu-wpcom/src/features/gutenberg-rtc/gutenberg-rtc.php
Show resolved
Hide resolved
| $allowed_providers = array( 'http-polling', 'pinghub' ); | ||
| $providers = apply_filters( 'wpcom_gutenberg_rtc_providers', array( 'pinghub' ) ); | ||
| if ( ! is_array( $providers ) ) { | ||
| return array(); | ||
| } | ||
|
|
||
| return array_values( | ||
| array_filter( | ||
| $providers, | ||
| function ( $provider ) use ( $allowed_providers ) { | ||
| return in_array( $provider, $allowed_providers, true ); | ||
| } | ||
| ) | ||
| ); | ||
| } |
There was a problem hiding this comment.
PR description mentions "limit 2 collaborators", but this change set only adds an allowlist + enqueue behavior for the http-polling provider and doesn’t appear to implement any collaborator limit. If the limit is handled elsewhere, consider linking to it in the PR description; otherwise, either implement the limit here or adjust the description to match what this PR actually changes.
Related DOTCOM-16238
Proposed changes:
Other information:
Jetpack product discussion
Does this pull request change what data or activity we track or use?
Testing instructions:
mu-plugins/0-sandbox.php/wp-sync/v1/sites/:site/updatesendpointChangelog