Repository navigation
[Web][Networking] Call setupFetchNetworkTransport() in the web worker, not in remote.html - #2286
Merged
adamziel merged 3 commits intoJun 19, 2025
Conversation
…, not in remote.html
This PR enables running simultaneous network calls asynchronously handled by multiple co-existing PHP
instances within the same worker.
The gist of the problem can be seen here:
```ts
const mgr = new PHPProcessManager({
phpFactory: async () =>
new PHP(await loadNodeRuntime(RecommendedPHPVersion)),
maxPhpInstances: 3,
});
const primaryPhp = await mgr.getPrimaryPhp();
// We setup a JavaScript handler for WP_HTTP_Fetch by listening
// to its post_message_to_js() call via primaryPHP.onMessage()...
setupFetchNetworkTransport(primaryPhp);
// ...but that does nothing for the next spawned PHP instance. When
// it calls post_message_to_js(), there's no message handler to
// translate that into fetch(). As a result, the response comes
// back empty.
const { php } = await mgr.acquirePHPInstance({ considerPrimary: false });
php.run({ code: "<?php wp_safe_remote_get('https://w.org'); " })
```
## Remaining work
A unit test or two would be nice.
## Testing instructions
1. Run this Blueprint: http://127.0.0.1:5400/website-server/?blueprint-url=https%3A%2F%2Fraw.githubusercontent.com%2Fautomattic%2Fremote-data-blocks-demo%2Ftrunk%2Fshopify%2Fshopify-blueprint.json
2. Click "Edit page"
3. Confirm the four requests to /wp-json/remote-data-blocks/v1/remote-data have actual data in their response body. Specifically, they should NOT have a JSON-encoded error message.
brandonpayton
approved these changes
Jun 18, 2025
Member
There was a problem hiding this comment.
I have not tested this, but it looks good to me.
Note: I think this is a web-only concern because AFAIK we do not have to set up a fetch transport for php-wasm/node. So I don't think we need to worry about the multi-worker scenario until we have multiple php-wasm/web workers, and even then, the fact that it is handled in the worker thread means it the fetch network transport should just work if we create more than one thread.
Thanks, @adamziel!
Collaborator
Author
|
Thank you for reviewing @brandonpayton! I've explored a test but it was quite tedious so I went for an inline comment instead to communicate the necessary context to future developers: |
adamziel
deleted the
setup-fetch-network-transport-in-worker-thread-not-in-remote
branch
June 19, 2025 09:31
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR enables running simultaneous network calls asynchronously handled by multiple co-existing PHP instances within the same worker.
The gist of the problem can be seen here:
Solution
We bind the
.onMessagehandler to every PHP instance created by thePHPProcessManager. As a side-effect, we also:onPHPInstanceCreatedcallback tobootWordPress()setupFetchNetworkTransportcall fromboot-playground-remote.tsto the web worker where PHP is spawned. This should make it marginally faster. Before this PR, the handler was one worker away.Testing instructions