-
Notifications
You must be signed in to change notification settings - Fork 100
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 service worker streaming #85
Conversation
This is awesome. Really looking forward to this coming together! |
redirectedUrl.searchParams.delete( STREAM_HEADER_FRAGMENT_QUERY_VAR ); | ||
const script = `<script>history.replaceState( {}, '', ${ JSON.stringify( redirectedUrl.toString() ) } );</script>`; | ||
return response.text().then( ( body ) => { | ||
return new Response( script + body ); |
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.
@jeffposnick here's how I ended up dealing with the challenge of handling a streamed body that redirects.
The “Try Redirect” nav menu item points to an old post URL that redirects, so you can see the URL go from "old-post" to "new-post".
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.
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.
😍
$header_html = ob_get_clean(); | ||
$libxml_use_errors = libxml_use_internal_errors( true ); | ||
$header_html = preg_replace( '#<noscript.+?</noscript>#s', '', $header_html ); // Some libxml versions croak at noscript in head. | ||
$dom = new DOMDocument( $header_html ); |
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.
Remove $header_html
as an arg. Not correct.
wp-includes/components/class-wp-service-worker-navigation-routing-component.php
Show resolved
Hide resolved
const canStreamResponse = () => { | ||
const url = new URL( event.request.url ); | ||
return ! ( | ||
/\.php$/.test( url.pathname ) || |
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.
Instead of doing this, why not add *.php
files to the blacklist patterns?
|
||
const url = new URL( event.request.url ); | ||
url.searchParams.append( STREAM_HEADER_FRAGMENT_QUERY_VAR, 'body' ); | ||
const request = new Request( url.toString(), {...event.request} ); |
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.
Verify that {...event.request}
is working.
.catch( sendOfflineResponse ), | ||
]); | ||
|
||
// @todo Handle error case. |
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.
This was done
return response; | ||
} | ||
} | ||
const channel = new BroadcastChannel( 'wordpress-server-errors' ); |
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.
Not a blocker at this point, but it would be needed to define the scenario for browsers that do not support BroadcastChannel (https://caniuse.com/#search=broadcastchannel)
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.
Yes, this is captured in #81.
} | ||
} | ||
|
||
const canStreamResponse = () => { |
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.
Add a brief comment here on the need to avoid PHP files. As discussed, this could be moved to the black listing
logic.
/** | ||
* Apply the stream body data to the stream header. | ||
* | ||
* @param {Array} data Data. |
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.
Perhaps expand the expected content of the data
argument.
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.
This is amazing work. Progressive Web, the WordPress way. Ship it!
Works best with AMP due to the fact that all scripts in AMP are asynchronous (ampproject/amp-wp#1503). See changes in theme: westonruter/twentyseventeen-westonson#2.
This pull request explores what it would look like to improve performance of navigating a site through the use of streaming. See @jeffposnick's article Beyond SPA for some key inspiration here. The goal is to speed up the perceived performance for navigating a site by showing a meaningful interface and loading indicator immediately. In short, this should reduce the first contentful paint time when navigating an AMP site on origin.
The proposed way that this would work is as follows:
add_theme_support( 'service_worker_streaming' )
) and adds aWP_Service_Worker_Navigation_Routing_Component::do_stream_boundary()
call to their theme'sheader.php
. This function outputs a loading message/progressbar to be displayed while the body is loading.wp_stream_fragment
is added which can take one of two values:header
andbody
(for example, https://example.com/?wp_stream_fragment=header and https://example.com/?wp_stream_fragment=body). This argument can be added to any URL, and whenheader
is supplied the AMP plugin returns the document up until thedo_stream_boundary()
call and whenbody
is supplied the document after thedo_stream_boundary()
call is returned.header
fragment is precached by the service worker. Thetitle
in this header fragment is filtered to say “Loading...”. This header fragment is also served with an inlinescript
containing amwpStreamCombine()
function which is called later in the body fragment.head
children andbody
attributes which would have normally been included in an entire response. Output buffering is used to capture theheader
for that given request and it is loaded into aDOMDocument
. A JSON representation of this data included in this response and into thewpStreamCombine
function (defined in the header fragment) at the very beginning of the body fragment which reconciles the differences between the precached header fragment. In Jeff's example linked above, he used this approach for reconciling thedocument.title
.location.replaceState()
.wpStreamCombine
function deletes the element containing loading indicator.The logic for obtaining the elements in the
head
can be deferred for later so another plugin (like AMP) can obtain the elements after it has sanitized it: ampproject/amp-wp#1503.Abbreviated example header fragment response
Abbreviated example body fragment response
Demo
See child theme PR for Twenty Seventeen that implements support: westonruter/twentyseventeen-westonson#2
Video: https://youtu.be/y5J69wLstUM
Live: https://streaming-westonruter.pantheonsite.io/
Todo
wp_offline_error_precache_entry
will be needed with arevision
that includes all variability that will be included in the header.location.replace()
call.