-
Notifications
You must be signed in to change notification settings - Fork 98
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
Offline commenting #92
Changes from 14 commits
7c74cba
93f5c5f
44b7747
ca07c6d
f6472eb
3cd1a33
7d67528
61b7b7f
df6b0dc
4e7381c
20bba99
8a62653
08250a5
78a7dcb
4da1a3f
69eb395
a3ce4b6
138b788
c4c88b3
de7b2eb
e631395
9594969
8c5dd63
544a747
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
/* global ERROR_MESSAGES, SITE_URL */ | ||
{ | ||
const queue = new wp.serviceWorker.backgroundSync.Queue( 'amp-wpPendingComments' ); | ||
const errorMessages = ERROR_MESSAGES; | ||
|
||
const commentHandler = ( { event } ) => { | ||
const clone = event.request.clone(); | ||
return fetch( event.request ) | ||
.then( ( response ) => { | ||
// @todo Make sure that 409 etc. error still work as expected. | ||
return response; | ||
} ) | ||
.catch( () => { | ||
const bodyPromise = clone.blob(); | ||
return bodyPromise.then( | ||
function( body ) { | ||
const request = event.request; | ||
const req = new Request( request.url, { | ||
method: request.method, | ||
headers: request.headers, | ||
mode: 'same-origin', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that |
||
credentials: request.credentials, | ||
referrer: request.referrer, | ||
redirect: 'manual', | ||
body: body | ||
} ); | ||
|
||
// Add request to queue. @todo Replace when upgrading to Workbox v4! | ||
queue.addRequest( req ); | ||
|
||
const jsonBody = JSON.stringify( { 'error': errorMessages.comment } ); | ||
return new Response( jsonBody, { | ||
headers: { | ||
'Access-Control-Allow-Credentials': 'true', | ||
'Content-Type': 'application/json; charset=UTF-8', | ||
'Access-Control-Expose-Headers': 'AMP-Access-Control-Allow-Source-Origin', | ||
'AMP-Access-Control-Allow-Source-Origin': SITE_URL, | ||
'Cache-Control': 'no-cache, must-revalidate, max-age=0' | ||
} | ||
} ); | ||
} | ||
); | ||
} ); | ||
}; | ||
|
||
wp.serviceWorker.routing.registerRoute( | ||
new RegExp('/wp-comments-post\.php\?.*_wp_amp_action_xhr_converted.*' ), // eslint-disable-line no-useless-escape | ||
commentHandler, | ||
'POST' | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -424,6 +424,7 @@ public function serve( WP_Service_Worker_Scripts $scripts ) { | |
'BLACKLIST_PATTERNS' => wp_service_worker_json_encode( $this->get_blacklist_patterns() ), | ||
'SHOULD_STREAM_RESPONSE' => wp_service_worker_json_encode( $should_stream_response ), | ||
'STREAM_HEADER_FRAGMENT_QUERY_VAR' => wp_service_worker_json_encode( self::STREAM_FRAGMENT_QUERY_VAR ), | ||
'ERROR_MESSAGES' => wp_service_worker_json_encode( wp_service_worker_get_error_messages() ), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @westonruter: Continued with the PR and addressed your comments, too. Question about There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Making it a method of the navigation routing component would probably be best. |
||
); | ||
} | ||
|
||
|
@@ -464,7 +465,7 @@ public function get_blacklist_patterns() { | |
* @return int Hook priority. A higher number means a lower priority. | ||
*/ | ||
public function get_priority() { | ||
return -99999; | ||
return 99; | ||
} | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,153 @@ | ||
<?php | ||
/** | ||
* WP_Service_Worker_Offline_Commenting_Component class. | ||
* | ||
* @package PWA | ||
*/ | ||
|
||
/** | ||
* Class representing the service worker core component for handling offline commenting. | ||
* | ||
* @since 0.2 | ||
*/ | ||
class WP_Service_Worker_Offline_Commenting_Component implements WP_Service_Worker_Component { | ||
|
||
/** | ||
* Internal storage for replacements to make in the error response handling script. | ||
* | ||
* @since 0.2 | ||
* @var array | ||
*/ | ||
protected $replacements = array(); | ||
|
||
/** | ||
* Adds the component functionality to the service worker. | ||
* | ||
* @todo This is all copied from navigation routing component and duplication should be removed. | ||
* | ||
* @since 0.2 | ||
* | ||
* @param WP_Service_Worker_Scripts $scripts Instance to register service worker behavior with. | ||
*/ | ||
public function serve( WP_Service_Worker_Scripts $scripts ) { | ||
$template = get_template(); | ||
$stylesheet = get_stylesheet(); | ||
|
||
$revision = sprintf( '%s-v%s', $template, wp_get_theme( $template )->Version ); | ||
if ( $template !== $stylesheet ) { | ||
$revision .= sprintf( ';%s-v%s', $stylesheet, wp_get_theme( $stylesheet )->Version ); | ||
} | ||
|
||
// Ensure the user-specific offline/500 pages are precached, and thet they update when user logs out or switches to another user. | ||
$revision .= sprintf( ';user-%d', get_current_user_id() ); | ||
|
||
if ( ! is_admin() ) { | ||
$offline_error_template_file = pwa_locate_template( array( 'offline.php', 'error.php' ) ); | ||
$offline_error_precache_entry = array( | ||
'url' => add_query_arg( 'wp_error_template', 'offline', home_url( '/' ) ), | ||
'revision' => $revision . ';' . md5( $offline_error_template_file . file_get_contents( $offline_error_template_file ) ), // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents | ||
); | ||
$server_error_template_file = pwa_locate_template( array( '500.php', 'error.php' ) ); | ||
$server_error_precache_entry = array( | ||
'url' => add_query_arg( 'wp_error_template', '500', home_url( '/' ) ), | ||
'revision' => $revision . ';' . md5( $server_error_template_file . file_get_contents( $server_error_template_file ) ), // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents | ||
); | ||
|
||
/** | ||
* Filters what is precached to serve as the offline error response on the frontend. | ||
* | ||
* The URL returned in this array will be precached by the service worker and served as the response when | ||
* the client is offline or their connection fails. To prevent this behavior, this value can be filtered | ||
* to return false. When a theme or plugin makes a change to the response, the revision value in the array | ||
* must be incremented to ensure the URL is re-fetched to store in the precache. | ||
* | ||
* @since 0.2 | ||
* | ||
* @param array|false $entry { | ||
* Offline error precache entry. | ||
* | ||
* @type string $url URL to page that shows the offline error template. | ||
* @type string $revision Revision for the template. This defaults to the template and stylesheet names, with their respective theme versions. | ||
* } | ||
*/ | ||
$offline_error_precache_entry = apply_filters( 'wp_offline_error_precache_entry', $offline_error_precache_entry ); | ||
|
||
/** | ||
* Filters what is precached to serve as the internal server error response on the frontend. | ||
* | ||
* The URL returned in this array will be precached by the service worker and served as the response when | ||
* the server returns a 500 internal server error . To prevent this behavior, this value can be filtered | ||
* to return false. When a theme or plugin makes a change to the response, the revision value in the array | ||
* must be incremented to ensure the URL is re-fetched to store in the precache. | ||
* | ||
* @since 0.2 | ||
* | ||
* @param array $entry { | ||
* Server error precache entry. | ||
* | ||
* @type string $url URL to page that shows the server error template. | ||
* @type string $revision Revision for the template. This defaults to the template and stylesheet names, with their respective theme versions. | ||
* } | ||
*/ | ||
$server_error_precache_entry = apply_filters( 'wp_server_error_precache_entry', $server_error_precache_entry ); | ||
|
||
} else { | ||
$offline_error_precache_entry = array( | ||
'url' => add_query_arg( 'code', 'offline', admin_url( 'admin-ajax.php?action=wp_error_template' ) ), // Upon core merge, this would use admin_url( 'error.php' ). | ||
'revision' => PWA_VERSION, // Upon core merge, this should be the core version. | ||
); | ||
$server_error_precache_entry = array( | ||
'url' => add_query_arg( 'code', '500', admin_url( 'admin-ajax.php?action=wp_error_template' ) ), // Upon core merge, this would use admin_url( 'error.php' ). | ||
'revision' => PWA_VERSION, // Upon core merge, this should be the core version. | ||
); | ||
} | ||
|
||
$scripts->register( | ||
'wp-offline-commenting', | ||
array( | ||
'src' => array( $this, 'get_script' ), | ||
'deps' => array( 'wp-base-config' ), | ||
) | ||
); | ||
|
||
if ( $offline_error_precache_entry ) { | ||
$scripts->precaching_routes()->register( $offline_error_precache_entry['url'], isset( $offline_error_precache_entry['revision'] ) ? $offline_error_precache_entry['revision'] : null ); | ||
} | ||
if ( $server_error_precache_entry ) { | ||
$scripts->precaching_routes()->register( $server_error_precache_entry['url'], isset( $server_error_precache_entry['revision'] ) ? $server_error_precache_entry['revision'] : null ); | ||
} | ||
|
||
$this->replacements = array( | ||
'ERROR_OFFLINE_URL' => wp_service_worker_json_encode( isset( $offline_error_precache_entry['url'] ) ? $offline_error_precache_entry['url'] : null ), | ||
'ERROR_500_URL' => wp_service_worker_json_encode( isset( $server_error_precache_entry['url'] ) ? $server_error_precache_entry['url'] : null ), | ||
'ERROR_MESSAGES' => wp_service_worker_json_encode( wp_service_worker_get_error_messages() ), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @westonruter Created a separate component -- above you can see that getting these 3 URLs is just a duplication of Routing Component. Perhaps we should move the logic for getting the URLs out of the components to avoid duplication. |
||
); | ||
} | ||
|
||
/** | ||
* Gets the priority this component should be hooked into the service worker action with. | ||
* | ||
* @since 0.2 | ||
* | ||
* @return int Hook priority. A higher number means a lower priority. | ||
*/ | ||
public function get_priority() { | ||
return 10; | ||
} | ||
|
||
/** | ||
* Get script for routing navigation requests. | ||
* | ||
* @return string Script. | ||
*/ | ||
public function get_script() { | ||
$script = file_get_contents( PWA_PLUGIN_DIR . '/wp-includes/js/service-worker-offline-commenting.js' ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents | ||
$script = preg_replace( '#/\*\s*global.+?\*/#', '', $script ); | ||
|
||
return str_replace( | ||
array_keys( $this->replacements ), | ||
array_values( $this->replacements ), | ||
$script | ||
); | ||
} | ||
} |
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.
Is there any need for this
then
handler now? Should it not just do:In reality a success
response
should always be a 302 redirect, correct? That is, unless AMP live list commenting is being used in which case it would be a 200 or 202 response.In particular, there is the
handleResponse
function.Given that the 302 response is irrelevant for streaming or app shell, we don't need any of that logic. But some 500 error handling would perhaps be nice, but not essential for now. So I think we can just eliminate the
then
.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.
Removed!