Add pretty permalinks for service worker by swissspidy · Pull Request #289 · GoogleChromeLabs/pwa-wp

swissspidy · 2020-06-05T08:41:10Z

Fixes #287.

To-do:

Testing

westonruter · 2020-06-06T03:35:02Z

pwa.php

+function _pwa_activate_plugin() {
+	pwa_add_rewrite_rules();
+	flush_rewrite_rules( false );
+}
+
+register_activation_hook( PWA_PLUGIN_FILE, '_pwa_activate_plugin' );


The flushing is only happening upon activation. What about on network-activted multisite?

I've just inquired the same on wp-sitemaps: GoogleChromeLabs/wp-sitemaps#205

Haven‘t really tested Multisite, will take a look.

https://jeremyfelt.com/2015/07/17/flushing-rewrite-rules-in-wordpress-multisite-for-fun-and-profit/ 🤯

cc @felixarntz for Multisite expertise.

I don't think this will work since $wp_rewrite->extra_rules_top['^wp-service-worker\.js$'] will always be set by pwa_add_rewrite_rules on init.

Good point. In that case, perhaps pwa_add_rewrite_rules() should check to see if it actually added anything to the array, and if so, set a flag that can be accessed somewhere?

👋 y'all!

FWIW, since that post was written I've had success with the method posted by Thorsten on this ticket: https://core.trac.wordpress.org/ticket/18450

I've used it on init when the rewrite rules were very important, but it could be used on admin_init as well.

Thanks for the input! Yes, I'd say this qualifies as being very important since if the PWA plugin is network-activated, all the network sites would try to install the service worker. But if the rewrite rule is not present, then they'd fail to install.

Nevertheless, we're currently blocked on using rewrite rule, at least using .js file extension since various hosts will short-circuit such requests from going to WordPress to instead try to go straight to the filesystem: #287 (comment)

Now that we have /wp.serviceworker as the route, the only thing left here is making sure that the route will be detected across a network before rewrite rules have been flushed.

Here's an idea: en lieu of waiting for someone to log-in to each site to cause rewrite rules to be flushed, we can implement a fallback route of sorts.

I think I've got a good solution for that in 043beb8.

westonruter · 2020-06-07T05:40:08Z

Since this PR will result in an apparent static JS file being served, let's add a comment to the response to make it clear that the file is dynamically-generated. So in the line after:

pwa-wp/wp-includes/class-wp-service-workers.php

Line 146 in 330c76b

    
           printf( "/* PWA v%s-%s */\n\n", esc_html( PWA_VERSION ), is_admin() ? 'admin' : 'front' );

Add something like:

echo '/* ';
/* translators: %s is the WordPress action hook */
printf( __( 'Note: This JS file is dynamically-generated with PHP. To manipulate the contents of this file, use the "%s" action in WordPress.', 'pwa' ), is_admin() ? 'wp_admin_service_worker' : 'wp_front_service_worker' ) );
echo " /*\n\n";

swissspidy · 2020-06-07T08:02:05Z

Sounds good.

Does this also need a second rewrite rule for the admin scope service worker? 🤔

westonruter · 2020-06-07T08:15:38Z

No, the admin service worker is served via admin-ajax.php, so it wouldn't be subjected to the same issue here.

wp-includes/class-wp.php

Co-authored-by: Weston Ruter <westonruter@google.com>

…/sw-rewrite-rule * 'develop' of github.com:GoogleChromeLabs/pwa-wp: (30 commits) Bump prettier from 2.1.0 to 2.1.1 Bump grunt from 1.2.1 to 1.3.0 Avoid constructing asset versions with HTML entities in context of SW Bump grunt-wp-deploy from 2.0.0 to 2.1.2 Bump prettier from 2.0.5 to 2.1.0 Add support for additional version args [Security] Bump dot-prop from 4.2.0 to 4.2.1 Update the way a navigation route is registered Bump eslint from 7.6.0 to 7.7.0 Fix version handling for precached assets Update tested up to 5.5 Bump grunt from 1.2.0 to 1.2.1 Bump eslint from 7.4.0 to 7.6.0 Update wording. The serverOffline message was being shown even though the site was online & working perfectly fine while it was an issue with the client's connection (while they weren't necessarily considered offline.) Bump lodash from 4.17.14 to 4.17.19 CR Allow HTTPS detection and UI to be disabled Add manifest-URL helper Bump grunt from 1.1.0 to 1.2.0 Bump eslint from 7.3.1 to 7.4.0 ...

westonruter

To avoid the problem of wp-service-worker.js bypassing PHP and being routed straight to the file system by web servers, how about we just use wp.serviceworker.

See #287 (comment).

wp-includes/class-wp.php

pwa.php

wp-includes/service-workers.php

Co-authored-by: Weston Ruter <westonruter@google.com>

westonruter · 2020-09-09T06:03:43Z

This is working well according to #287 (comment). Any additional needed changes anyone sees prior to merging?

hideokamoto · 2020-10-02T09:00:00Z

@westonruter
Hi.
Has the PR published in wp.org?

I check wp.org today, but the version is still 0.5 ...

swissspidy · 2020-10-02T09:05:11Z

@hideokamoto No, as you can see at https://github.com/GoogleChromeLabs/pwa-wp/milestone/6 version 0.6 is not yet complete.

hideokamoto · 2020-10-02T09:07:32Z

@swissspidy
Roger that.

Thanks !

This was missed in #289.

Add pretty permalinks for service worker

47945e7

swissspidy mentioned this pull request Jun 5, 2020

Handling service worker query vars #287

Closed

westonruter reviewed Jun 6, 2020

View reviewed changes

westonruter added this to the 0.6 milestone Jun 6, 2020

swissspidy marked this pull request as draft June 6, 2020 16:41

felixarntz mentioned this pull request Jun 6, 2020

Rewrite rules may not get registered on multisite installs GoogleChromeLabs/wp-sitemaps#205

Closed

Add comment to auto-generated JS file

5b3e425

westonruter reviewed Jun 8, 2020

View reviewed changes

wp-includes/class-wp.php Show resolved Hide resolved

swissspidy and others added 2 commits June 11, 2020 15:26

Try flushing rewrite rules on admin init if they haven't been added yet

990b027

Co-authored-by: Weston Ruter <westonruter@google.com>

Add missing global definition and de-duplicate pattern

ba57b0d

westonruter changed the base branch from master to develop June 19, 2020 19:06

westonruter added the service-workers label Aug 28, 2020

westonruter reviewed Aug 29, 2020

View reviewed changes

wp-includes/class-wp.php Outdated Show resolved Hide resolved

pwa.php Outdated Show resolved Hide resolved

swissspidy commented Sep 2, 2020

View reviewed changes

wp-includes/service-workers.php Outdated Show resolved Hide resolved

Try using wp.serviceworker URLs

fca6728

Co-authored-by: Weston Ruter <westonruter@google.com>

westonruter mentioned this pull request Sep 7, 2020

PWA 'pages' should set meta robots indexing directives #300

Closed

Handle case where rewrite rules have not yet been flushed

043beb8

westonruter marked this pull request as ready for review September 7, 2020 06:50

Add tests for wp_get_service_worker_url()

f81858a

westonruter approved these changes Sep 9, 2020

View reviewed changes

westonruter merged commit 67aebff into develop Sep 10, 2020

westonruter deleted the try/sw-rewrite-rule branch September 10, 2020 03:41

westonruter added a commit that referenced this pull request Oct 23, 2020

Exclude /wp.serviceworker from navigation caching strategy

e611d58

This was missed in #289.

westonruter mentioned this pull request Oct 27, 2020

Add default caching strategies for navigations and page assets #338

Merged

4 tasks

westonruter added the changelogged label Dec 16, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add pretty permalinks for service worker#289

Add pretty permalinks for service worker#289
westonruter merged 8 commits intodevelopfrom
try/sw-rewrite-rule

swissspidy commented Jun 5, 2020 •

edited by westonruter

Loading

westonruter Jun 6, 2020

westonruter Jun 6, 2020

swissspidy Jun 6, 2020

swissspidy Jun 6, 2020

swissspidy Jun 6, 2020

swissspidy Jun 8, 2020

westonruter Jun 8, 2020

jeremyfelt Jun 26, 2020

westonruter Jul 5, 2020

westonruter Sep 7, 2020

westonruter commented Jun 7, 2020 •

edited

Loading

swissspidy commented Jun 7, 2020

westonruter commented Jun 7, 2020

Uh oh!

westonruter left a comment

Uh oh!

Uh oh!

Uh oh!

westonruter commented Sep 9, 2020

hideokamoto commented Oct 2, 2020

swissspidy commented Oct 2, 2020

hideokamoto commented Oct 2, 2020

Labels

5 participants

Conversation

swissspidy commented Jun 5, 2020 • edited by westonruter Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

westonruter commented Jun 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

swissspidy commented Jun 7, 2020

westonruter commented Jun 7, 2020

Uh oh!

westonruter left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

westonruter commented Sep 9, 2020

hideokamoto commented Oct 2, 2020

swissspidy commented Oct 2, 2020

hideokamoto commented Oct 2, 2020

Labels

5 participants

swissspidy commented Jun 5, 2020 •

edited by westonruter

Loading

westonruter commented Jun 7, 2020 •

edited

Loading