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

Set web worker startup options with messages instead of query strings #1574

Merged

Conversation

bgrgicak
Copy link
Collaborator

@bgrgicak bgrgicak commented Jul 4, 2024

Motivation for the change, related issues

When the web worker is loaded from browser storage it loses the query string context we use to send startup options like WP and PHP version.

To enable offline support, this PR changes how the worker obtains startup options to ensure it works when cached.

Implementation details

Instead of fetching startup options from the query string, spawnPHPWorkerThread will post startup options as a message to the worker.

When worker-utils loads it will await the startup-options message and continue once it receives the data.

Testing Instructions (or ideally a Blueprint)

  • Ensure all tests pass

@brandonpayton @adamziel I'm not sure about this solution. It works, but I'm not happy with it. How does it look to you?

@bgrgicak bgrgicak self-assigned this Jul 4, 2024
@bgrgicak bgrgicak changed the base branch from trunk to add/vite-cache-paths-plugin July 4, 2024 11:07
@bgrgicak bgrgicak marked this pull request as ready for review July 4, 2024 11:20
@bgrgicak bgrgicak marked this pull request as draft July 4, 2024 11:38
@bgrgicak bgrgicak mentioned this pull request Jul 4, 2024
@bgrgicak bgrgicak requested a review from a team July 5, 2024 03:59
@bgrgicak bgrgicak marked this pull request as ready for review July 5, 2024 05:36
@adamziel adamziel changed the title Set service worker startup options with messages instead of query strings Jul 9, 2024
@adamziel
Copy link
Collaborator

adamziel commented Jul 9, 2024

I adjusted the title and description – it talked about the service worker, but it's actual about the regular web worker. The PR looks good, thank you @bgrgicak!

@adamziel adamziel merged commit 97d95f3 into add/vite-cache-paths-plugin Jul 9, 2024
5 checks passed
@adamziel adamziel deleted the update/set-startup-options-with-messages branch July 9, 2024 12:53
@adamziel
Copy link
Collaborator

adamziel commented Jul 9, 2024

Ah, I merged and then realized it's the topmost of the three stacked PRs and now #1573 contains these changes.

@adamziel
Copy link
Collaborator

adamziel commented Jul 9, 2024

It seems like we remove the startupOptions from the worker URL now:

-	workerUrl = addQueryParams(workerUrl, startupOptions);
	const worker = new Worker(workerUrl, { type: 'module' });
@bgrgicak
Copy link
Collaborator Author

It seems like we remove the startupOptions from the worker URL now:

I did it now in 7c886d4. I wasn't sure if there is any value in keeping it around.

@bgrgicak bgrgicak restored the update/set-startup-options-with-messages branch July 12, 2024 07:29
bgrgicak added a commit that referenced this pull request Jul 12, 2024
adamziel added a commit that referenced this pull request Jul 15, 2024
[For offline
support](#1600) we
need to load the service worker as early as possible in the boot
process.

Booting early will allow fetch caching to cache more files on the first
making the backfill of assets simpler.

Read the full research recap here
#1600 (review)
 
## Implementation details

To move `registerServiceWorker` to the start of the boot process we had
to generate the scope in `bootPlaygroundRemote` and pass it to the PHP
worker thread together with other startup options.

The service worker had to be registered before spawning the PHP worker
thread to ensure WP and PHP assets were cached. But the service worker
also depends on the `phpApi` returned by `spawnPHPWorkerThread`.
We had to register the service worker first, after that, spawn the PHP
worker thread, and then set the `phpApi`.

This PR also includes changes from
#1574 because they
allow us to pass the scope.

## Testing Instructions (or ideally a Blueprint)

- Ensure all tests pass

---------

Co-authored-by: Adam Zieliński <adam@adamziel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment