Skip to content
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

Prototype for Service Workers API #14

Merged
merged 29 commits into from
Jul 9, 2018
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
357ee58
Add initial skeleton and structure for registering service workers.
miina Jun 21, 2018
db2e496
Remove comment.
miina Jun 21, 2018
531092c
Add composer file.
miina Jun 21, 2018
076c3d0
Fix some phpcs issues.
miina Jun 21, 2018
e2cfd3e
Add caching for SW file.
miina Jun 21, 2018
296cf02
Add initial rewrite rules.
miina Jun 21, 2018
d6e8985
Add initial logic for considering deps.
miina Jun 21, 2018
22658ed
Modify comments.
miina Jun 21, 2018
ecc1431
Merge master.
miina Jun 22, 2018
4e8d750
Fix rewrite rules not working. Add skeleton for tests.
miina Jun 22, 2018
2b585fa
Make changes according to feedback.
miina Jun 25, 2018
a3ece90
Avoid http request in do_item()
miina Jun 25, 2018
7196301
Switch the order of params.
miina Jun 26, 2018
922e8d6
Support registering scope as an array. Replace using 'add' method.
miina Jun 26, 2018
7d63272
Use empty instead of ! for scopes.
miina Jun 26, 2018
7c28ba4
Add window load to registering SW.
miina Jun 27, 2018
ecdc6bf
Use array instead of null for in wp_register_service_worker()
miina Jun 27, 2018
20f037a
Avoid duplicate registering.
miina Jun 27, 2018
6a234f0
Fix incorrect SW registering.
miina Jun 27, 2018
645dc19
Add possible approach for allowing dynamic scripts.
miina Jun 28, 2018
02132c5
Merge master.
miina Jun 28, 2018
094a0d7
Move _doing_it_wrong(). Add console.warn() / source to output.
miina Jun 29, 2018
09506d1
Avoid duplicate constants by wrapping scripts into self-invoking func…
miina Jun 29, 2018
4cc95ca
Add global for accessing promises.
miina Jul 4, 2018
c920a65
Use window.wp; Use query_vars filter; Rename method to use wp_;
miina Jul 4, 2018
7362ae3
Remove using WP_Filesystem.
miina Jul 4, 2018
f44146c
Use new phpcs:ignore comment instead of codingStandardsIgnoreLine
westonruter Jul 4, 2018
4869e0e
Move files into wp-includes; remove unnecessary pwawp_init()
westonruter Jul 7, 2018
e9e902e
Ensure wp_get_service_worker_url() returns an HTTPS URL
westonruter Jul 8, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
node_modules
build/
composer.lock
10 changes: 10 additions & 0 deletions pwa-wp.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,14 @@ function pwawp_init() {

$wp_web_app_manifest = new WP_Web_App_Manifest();
$wp_web_app_manifest->init();

// These could be in ABSPATH . WPINC . '/script-loader.php' file.
/** WordPress Service Workers Class */
require PWAWP_PLUGIN_DIR . '/wp-includes/class-wp-service-workers.php';

/** WordPress Service Worker Functions */
require PWAWP_PLUGIN_DIR . '/wp-includes/service-workers.php';

/** Hooks */
require PWAWP_PLUGIN_DIR . '/wp-includes/default-filters.php';
}
59 changes: 59 additions & 0 deletions tests/test-class-wp-service-workers.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
<?php
/**
* Tests for class WP_Service_Workers.
*
* @package PWA
*/

/**
* Tests for class WP_Web_App_Manifest.
*/
class Test_WP_Service_Workers extends WP_UnitTestCase {

/**
* Tested instance.
*
* @var WP_Service_Workers
*/
public $instance;

/**
* Setup.
*
* @inheritdoc
*/
public function setUp() {
parent::setUp();
$this->instance = new WP_Service_Workers();
}

/**
* Test class constructor.
*
* @covers WP_Service_Workers::__construct()
*/
public function test_construct() {
$service_workers = new WP_Service_Workers();
$this->assertEquals( 'WP_Service_Workers', get_class( $service_workers ) );
}

/**
* Test adding new service worker.
*
* @covers WP_Service_Workers::add()
*/
public function test_add() {
$this->instance->add( 'foo', '/test-sw.js', array( 'bar' ), '1.0' );

$default_scope = site_url( '/', 'relative' );

$this->assertTrue( in_array( $default_scope, $this->instance->get_scopes(), true ) );
$this->assertTrue( isset( $this->instance->registered['foo'] ) );

$registered_sw = $this->instance->registered['foo'];

$this->assertEquals( '/test-sw.js', $registered_sw->src );
$this->assertEquals( $default_scope, $registered_sw->args );
$this->assertEquals( array( 'bar' ), $registered_sw->deps );
}
}
216 changes: 216 additions & 0 deletions wp-includes/class-wp-service-workers.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,216 @@
<?php
/**
* Dependencies API: WP_Service_Workers class
*
* @since ?
*
* @package PWA
*/

/**
* Class used to register service workers.
*
* @since ?
*
* @see WP_Dependencies
*/
class WP_Service_Workers extends WP_Scripts {

/**
* Param for service workers.
*
* @var string
*/
public $query_var = 'wp_service_worker';

/**
* Output for service worker scope script.
*
* @var string
*/
public $output = '';

/**
* WP_Service_Workers constructor.
*/
public function __construct() {
parent::__construct();
global $wp_filesystem;

if ( ! class_exists( 'WP_Filesystem' ) ) {
require_once ABSPATH . '/wp-admin/includes/file.php';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be moved into do_item as well, and then the __construct method can be eliminated.

}

if ( null === $wp_filesystem ) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this be better in do_items?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do_items is in the parent class only but will move it to do_item.

WP_Filesystem();
}
}

/**
* Initialize the class.
*/
public function init() {
/**
* Fires when the WP_Service_Workers instance is initialized.
*
* @param WP_Service_Workers $this WP_Service_Workers instance (passed by reference).
*/
do_action_ref_array( 'wp_default_service_workers', array( &$this ) );
}

/**
* Register service worker.
*
* Registers service worker if no item of that name already exists.
*
* @param string $handle Name of the item. Should be unique.
* @param string $path Path of the item relative to the WordPress root directory.
* @param array $deps Optional. An array of registered item handles this item depends on. Default empty array.
* @param bool $ver Always false for service worker.
* @param mixed $scope Optional. Scope of the service worker. Default relative path.
* @return bool Whether the item has been registered. True on success, false on failure.
*/
public function add( $handle, $path, $deps = array(), $ver = false, $scope = null ) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something that just came to mind: what if a script needs to be “enqueued” in multiple scopes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just pushed the changes for registering scopes as an array.


Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we check here for scope collisions? That is, what happens if two entities call register on the same scope?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is by design that two scripts can have the same scope. In fact this is the reason for this core API in the first place, to allow plugins and themes to each register their own scripts to run in the same scope as others'.

// Set default scope if missing.
if ( ! $scope ) {
$scope = site_url( '/', 'relative' );
}

if ( false === parent::add( $handle, $path, $deps, false, $scope ) ) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of passing $scope as the args, I think it should pass compact( 'scope' ). This will allow other args to be added in the future in addition to the scope.

Also, maybe a script should be able to added to multiple scopes as I mentioned above, so maybe it should be scopes instead.

return false;
}
return true;
}

/**
* Get service worker logic for scope.
*
* @param string $scope Scope of the Service Worker.
*/
public function serve_request( $scope ) {

header( 'Content-Type: text/javascript; charset=utf-8' );
nocache_headers();

if ( ! in_array( $scope, $this->get_scopes(), true ) ) {
status_header( 404 );
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #26. This should perhaps not return a 404 but rather return a 200 and a service worker script that simply unregisters itself.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(My suggestion won't work.)

return;
}

$scope_items = array();

// Get handles from the relevant scope only.
foreach ( $this->registered as $handle => $item ) {
if ( $scope === $item->args ) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per above, I think args should be preserved as an array to allow additional args in the future. Also, maybe a script should be in multiple scopes. So in that case, this could be:

if ( in_array( $item->args['scopes'], $scope, true ) ) {

$scope_items[] = $handle;
}
}

$this->output = '';
$this->do_items( $scope_items );

$file_hash = md5( $this->output );
header( "Etag: $file_hash" );

$etag_header = isset( $_SERVER['HTTP_IF_NONE_MATCH'] ) ? trim( $_SERVER['HTTP_IF_NONE_MATCH'] ) : false;
if ( $file_hash === $etag_header ) {
status_header( 304 );
return;
}

// @codingStandardsIgnoreLine
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of this, let's do:

echo $this->output; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped

echo $this->output;
}

/**
* Get all scopes.
*
* @return array Array of scopes.
*/
public function get_scopes() {

$scopes = array();
foreach ( $this->registered as $handle => $item ) {
$scopes[] = $item->args;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per above, I think we should consider other args being supplied and for multiple scopes to be registered for one script. So this could instead be:

$scopes = array_merge( $scopes, $item->args['scopes'] );

}
return $scopes;
}

/**
* Process one registered script.
*
* @param string $handle Handle.
* @param bool $group Group.
* @return void
*/
public function do_item( $handle, $group = false ) {
global $wp_filesystem;

$obj = $this->registered[ $handle ];
$this->output .= $wp_filesystem->get_contents( site_url() . $obj->src ) . "\n";
}

/**
* Remove URL scheme.
*
* @param string $schemed_url URL.
* @return string URL.
*/
public function remove_url_scheme( $schemed_url ) {
return preg_replace( '#^\w+:(?=//)#', '', $schemed_url );
}

/**
* Get validated path to file.
*
* @param string $url Relative path.
* @return null|string|WP_Error
*/
public function get_validated_file_path( $url ) {
$needs_base_url = ! preg_match( '|^(https?:)?//|', $url );
$base_url = site_url();

if ( $needs_base_url ) {
$url = $base_url . $url;
}

// Strip URL scheme, query, and fragment.
$url = $this->remove_url_scheme( preg_replace( ':[\?#].*$:', '', $url ) );

$includes_url = $this->remove_url_scheme( includes_url( '/' ) );
$content_url = $this->remove_url_scheme( content_url( '/' ) );
$admin_url = $this->remove_url_scheme( get_admin_url( null, '/' ) );

$allowed_hosts = array(
wp_parse_url( $includes_url, PHP_URL_HOST ),
wp_parse_url( $content_url, PHP_URL_HOST ),
wp_parse_url( $admin_url, PHP_URL_HOST ),
);

$url_host = wp_parse_url( $url, PHP_URL_HOST );

if ( ! in_array( $url_host, $allowed_hosts, true ) ) {
/* translators: %s is file URL */
return new WP_Error( 'external_file_url', sprintf( __( 'URL is located on an external domain: %s.', 'pwa' ), $url_host ) );
}

$file_path = null;
if ( 0 === strpos( $url, $content_url ) ) {
$file_path = WP_CONTENT_DIR . substr( $url, strlen( $content_url ) - 1 );
} elseif ( 0 === strpos( $url, $includes_url ) ) {
$file_path = ABSPATH . WPINC . substr( $url, strlen( $includes_url ) - 1 );
} elseif ( 0 === strpos( $url, $admin_url ) ) {
$file_path = ABSPATH . 'wp-admin' . substr( $url, strlen( $admin_url ) - 1 );
} else {
$file_path = ABSPATH . substr( $url, strlen( $this->remove_url_scheme( $base_url ) ) );
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also added the else here for now.

Wondering if we should restrict the SW file path to validating against WP_CONTENT_DIR only? Then the potential default core WP SW scripts could be registered via WP_Service_Workers::register_default_service_workers which would skip the check for get_validated_file_path. Not sure if plugins/themes should be able to register scripts that are outside of WP_CONTENT_DIR. Thoughts?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since plugins and themes would only enqueue their scripts that are inside WP_CONTENT_DIR I think it is safe to assume that the same would be true for service worker scripts.

}

if ( ! $file_path || false !== strpos( '../', $file_path ) || 0 !== validate_file( $file_path ) || ! file_exists( $file_path ) ) {
/* translators: %s is file URL */
return new WP_Error( 'file_path_not_found', sprintf( __( 'Unable to locate filesystem path for %s.', 'pwa' ), $url ) );
}

return $file_path;
}
}
14 changes: 14 additions & 0 deletions wp-includes/default-filters.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?php
/**
* Sets up the default filters and actions for PWA hooks.
*
* @package PWA
*/

foreach ( array( 'wp_print_footer_scripts', 'admin_print_footer_scripts', 'customize_controls_print_footer_scripts' ) as $filter ) {
add_filter( $filter, 'wp_print_service_workers' );
}

add_action( 'template_redirect', 'service_worker_loaded' );

add_action( 'init', 'wp_add_sw_rewrite_rules' );
Loading