Backfill the assets removed from minified WordPress bundles by bgrgicak · Pull Request #1604 · WordPress/wordpress-playground

bgrgicak · 2024-07-12T07:14:47Z

Motivation for the change, related issues

To achieve offline support we need to download all WordPress files into the Playground filesystem. Today Playground downloads WordPress partially on boot and downloads assets as they are needed using fetch requests.

This PR downloads all assets into the Playground filesystem without blocking the boot process.

Implementation details

Zip files are generated using the WordPress build script and are accessible in the /wp-VERSION/ folder like any other assets.

The download is triggered after WordPress is installed. The fetch is async and won't block the boot process.

In case some of the assets are needed immediately on boot they will still be downloaded using fetch to ensure they are available to WordPress.

Once the assets are downloaded they are stored in the Playground filesystem and will be served from there.

Testing Instructions (or ideally a Blueprint)

Run npx nx bundle-wordpress:nightly playground-wordpress-builds
Confirm that a packages/playground/wordpress-builds/public/wp-nightly/wordpress-static.zip file was created
Load Playground using this blueprint
In network tools see that the wordpress-static.zip file was downloaded
Open /wp-admin/
In network tools confirm that static assets like thickbox.js were loaded from the service worker instead of a fetch

…mments and errors

adamziel · 2024-07-12T12:07:08Z

 	}

+	/**
+	 * If the browser is online we can download WordPress assets asynchronously to speed up the boot process.


I got a bit confused why that's optional – this should clarify it:

Suggested change

* If the browser is online we can download WordPress assets asynchronously to speed up the boot process.

* If the browser is online we download the WordPress assets asynchronously to speed up the boot process.

Also, let's clarify this comment talks about the minified WordPress bundles and that we're not doing either of these when a full production build is used (since it ships all the static assets already).

I'm reopening this conversation because this isn't addressed yet:

Also, let's clarify this comment talks about the minified WordPress bundles and that we're not doing either of these when a full production build is used (since it ships all the static assets already).

Done 2ef0bcd. I kept that part short because it's described in detail in the function docblock.

adamziel · 2024-07-12T12:12:00Z

+	 * If the browser is online we can download WordPress assets asynchronously to speed up the boot process.
+	 * Missing assets will be fetched on demand from the Playground server until they are downloaded.
+	 *
+	 * If the browser is offline, we download WordPress assets synchronously to ensure the Playground is fully functional on load.


How are we downloading assets if the browser is offline?

Also, nitpick: Synchronous means blocking, meaning the event loop would be stopped until the download is complete. In here, however, we're not pausing the entire worker but just awaiting the download before continuing the boot process.

The comment was misleading, this is what it needs to say: _ If the browser is offline, we backfill WordPress assets synchronously from the cache to ensure Playground is fully functional before boot finishes._

I'm reopening this conversation because this isn't addressed yet:

Synchronous means blocking, meaning the event loop would be stopped until the download is complete. In here, however, we're not pausing the entire worker but just awaiting the download before continuing the boot process.

I get it now. In my mind, the comment sounded the same, but await and synchronous definitely aren't the same. 3e4cd07

adamziel · 2024-07-12T12:42:39Z

Most of my notes are about comments and code style. The only technical blocker I see is this:

If the browser is offline, we download WordPress assets synchronously to ensure the Playground is fully functional on load.

How would it download the static assets when the browser is offline? Would it use the zip file cached in the service worker, reject the promise, and display a reasonable error message if that file not cached yet? If yes, that sounds fine.

Co-authored-by: Adam Zieliński <adam@adamziel.com>

bgrgicak · 2024-07-15T08:48:48Z

How would it download the static assets when the browser is offline? Would it use the zip file cached in the service worker, reject the promise, and display a reasonable error message if that file not cached yet? If yes, that sounds fine.

The comment was misleading. The difference is that in offline mode boot waits for the backfill to finish, and when offline it doesn't.

For offline support to work, the wordpress-static.zip needs to be cached, so we expect it to be available.

adamziel

The implementation seems great so I'm approving. Thank you @bgrgicak! Let's still address the notes I left about the documentation and inline comments before merging.

Co-authored-by: Adam Zieliński <adam@adamziel.com>

bgrgicak · 2024-07-16T05:58:40Z

@adamziel I addressed your comments and will merge it now to unblock development.
Let me know if the comments require more changes and I can push them to another PR.

…1614) ## Motivation for the change, related issues #1604 accidentally introduced recursive calls to `webApi.backfillStaticFilesRemovedFromMinifiedBuild`. ## Implementation details Update `webApi.backfillStaticFilesRemovedFromMinifiedBuild` to call `phpApi.backfillStaticFilesRemovedFromMinifiedBuild`. ## Testing Instructions (or ideally a Blueprint) - checkout this branch - confirm that there are no errors related to calls to `backfillStaticFilesRemovedFromMinifiedBuild` in the browser console

bgrgicak added 30 commits June 20, 2024 13:37

ZIP WP assets

6d9e165

Add afterWordPressInstall hook to boot

75bc2bb

Download assets after WordPress is installed

6c2b0ad

Merge branch 'trunk' into add/wp-static-asset-zip

57daebb

Use getWordPressVersionFromPhp

42be4fc

Remove bootWordPress hook

8beee0c

Use wordPressSiteUrl

6cf5fef

Stream static asset response instead of awaiting

1f2347e

Use request handler to get document root

f1e2784

Add warning if assets weren't downloaded

3c98912

Merge branch 'trunk' into add/wp-static-asset-zip

e703eff

Async fetch

2e3910c

Update error messages

d2ca9c2

Use wpVersionToStaticAssetsDirectory

4a1f8d0

Use getLoadedWordPressVersion

efb4b57

downloadWordPressAssets on load

b2ba22b

Remove logs

1aad0d0

Merge branch 'trunk' into add/wp-static-asset-zip

db8c25f

Merge branch 'trunk' into add/wp-static-asset-zip

de206a1

Empty remote asset paths after done instead of deleting it, update co…

a6f20bd

…mments and errors

Merge branch 'trunk' into add/wp-static-asset-zip

ebfa224

Update packages/playground/remote/src/lib/worker-thread.ts

37c0b91

Linter fixes

ee044ae

Build static assets

f8dab44

Use unzipFile

6f48337

Fix dependencies

d34f066

Update static asset zips

1feb41a

Merge branch 'trunk' into add/wp-static-asset-zip

f3fe08d

Remove unused zip delete

f0378d0

Update remote asset PR comment

7c205fa

adamziel reviewed Jul 12, 2024

View reviewed changes

Comment thread packages/playground/remote/src/lib/worker-thread.ts Outdated

adamziel reviewed Jul 12, 2024

View reviewed changes

Comment thread packages/playground/remote/src/lib/worker-thread.ts Outdated

adamziel reviewed Jul 12, 2024

View reviewed changes

Comment thread packages/playground/remote/src/lib/worker-thread.ts Outdated

adamziel reviewed Jul 12, 2024

View reviewed changes

Comment thread packages/playground/remote/src/lib/worker-thread.ts Outdated

bgrgicak commented Jul 15, 2024

View reviewed changes

Comment thread packages/playground/remote/src/lib/boot-playground-remote.ts Outdated

Apply suggestions from code review

57dbb55

Co-authored-by: Adam Zieliński <adam@adamziel.com>

Update comments

1600ead

bgrgicak requested a review from adamziel July 15, 2024 08:58

bgrgicak added 4 commits July 15, 2024 12:15

Fix untrailingslashit

4f60fea

Replace untrailingslashit

d24fa25

Merge branch 'trunk' into add/wp-static-asset-zip

38095a6

Trim /

9a2ad0a

adamziel reviewed Jul 15, 2024

View reviewed changes

Comment thread packages/playground/remote/src/lib/boot-playground-remote.ts

adamziel reviewed Jul 15, 2024

View reviewed changes

Comment thread packages/playground/remote/src/lib/worker-thread.ts Outdated

adamziel changed the title ~~Download all WordPress assets on boot~~ Backfill the assets removed from minified WordPress bundles Jul 15, 2024

adamziel approved these changes Jul 15, 2024

View reviewed changes

bgrgicak and others added 5 commits July 16, 2024 07:42

Merge branch 'trunk' into add/wp-static-asset-zip

9f73a3f

Update packages/playground/remote/src/lib/worker-thread.ts

a6cfac1

Co-authored-by: Adam Zieliński <adam@adamziel.com>

Update comment and fix var name

3e4cd07

Update online comment

2ef0bcd

Update packages/playground/remote/src/lib/boot-playground-remote.ts

dc0649a

Co-authored-by: Adam Zieliński <adam@adamziel.com>

bgrgicak merged commit c8b1752 into trunk Jul 16, 2024

bgrgicak deleted the add/wp-static-asset-zip branch July 16, 2024 06:07

bgrgicak mentioned this pull request Jul 16, 2024

Fix recursive calls to backfillStaticFilesRemovedFromMinifiedBuild #1614

Merged

	* If the browser is online we can download WordPress assets asynchronously to speed up the boot process.
	* If the browser is online we download the WordPress assets asynchronously to speed up the boot process.

Conversation

bgrgicak commented Jul 12, 2024

Motivation for the change, related issues

Implementation details

Testing Instructions (or ideally a Blueprint)

Uh oh!

adamziel Jul 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamziel Jul 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bgrgicak Jul 16, 2024

Choose a reason for hiding this comment

Uh oh!

adamziel Jul 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bgrgicak Jul 15, 2024

Choose a reason for hiding this comment

Uh oh!

adamziel Jul 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bgrgicak Jul 16, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

adamziel commented Jul 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

bgrgicak commented Jul 15, 2024

Uh oh!

Uh oh!

Uh oh!

adamziel left a comment

Choose a reason for hiding this comment

Uh oh!

bgrgicak commented Jul 16, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

adamziel Jul 12, 2024 •

edited

Loading

adamziel Jul 15, 2024 •

edited

Loading

adamziel Jul 12, 2024 •

edited

Loading

adamziel Jul 15, 2024 •

edited

Loading

adamziel commented Jul 12, 2024 •

edited

Loading