Skip to content

PHP: Do not pull WebGL in Playground web #2318

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

Merged
merged 2 commits into from
Jul 3, 2025

Conversation

adamziel
Copy link
Collaborator

@adamziel adamziel commented Jul 3, 2025

Motivation for the change, related issues

Only uses -s MAIN_MODULE when building for Node.js+JSPI to fix a crash in the web version of Playground.

#2248 added -s MAIN_MODULE to the main emcc call linking the final PHP.wasm file. It worked fine in Node.js, but on the web, after rebuilding with recompile:php:web:jspi:8.4, Playground threw a ReferenceError: document is not defined.

It turn out, -s MAIN_MODULE pulls in a pile of extra libraries in case some dynamically loaded lib needs them. One of them was webgl that assumes globalThis.document exists – hence the error. See this Emscripten thread for more details.

Implementation

Moves -s MAIN_MODULE to platform-only flags for Node.js+JSPI.

I've tried switching to -s MAIN_MODULE=2 as suggested in the mailing lists. It fixes it for the web build, but it breaks loading XDebug.so in Node. The error says missing export type for core_globals. I assume we need to pull in some additional dependencies and declare them explicitly.

Remaining work

Rebuild all the PHP versions – Node.js and Web – and confirm the tests pass


Unrelated – I've noticed to patch with-xdebug.ts as follows to be able to run it from source:

diff --git a/packages/php-wasm/node/src/lib/xdebug/with-xdebug.ts b/packages/php-wasm/node/src/lib/xdebug/with-xdebug.ts
index 3b40aa2b3f..41fec3d237 100644
--- a/packages/php-wasm/node/src/lib/xdebug/with-xdebug.ts
+++ b/packages/php-wasm/node/src/lib/xdebug/with-xdebug.ts
@@ -17,7 +17,11 @@ export async function withXdebug(
        }
 
        const fileName = 'xdebug.so';
-       const filePath = await getXdebugExtensionModule(version);
+       let filePath = await getXdebugExtensionModule(version);
+       if (filePath.startsWith('file://')) {
+               filePath = filePath.substring(7);
+       }
+
        const extension = fs.readFileSync(filePath);
 
        return {

cc @mho22 @zaerl

@zaerl zaerl added the [Type] Bug An existing feature does not function as intended label Jul 3, 2025
@mho22 mho22 mentioned this pull request Jul 3, 2025
10 tasks
@zaerl zaerl marked this pull request as ready for review July 3, 2025 12:16
@zaerl zaerl requested review from bgrgicak, mho22 and zaerl July 3, 2025 12:16
Copy link
Collaborator

@zaerl zaerl left a comment

Choose a reason for hiding this comment

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

I've tested on web and CLI, and everything is working. 👍

@zaerl zaerl merged commit 5821cee into trunk Jul 3, 2025
27 checks passed
@zaerl zaerl deleted the dont-pull-webgl-in-playground-web branch July 3, 2025 13:08
@adamziel
Copy link
Collaborator Author

adamziel commented Jul 3, 2025

Thank you!

@mho22
Copy link
Collaborator

mho22 commented Jul 7, 2025

@adamziel I am currently trying to reproduce the issue you had a couple days ago :

Unrelated – I've noticed to patch with-xdebug.ts as follows to be able to run it from source:

-       const filePath = await getXdebugExtensionModule(version);
+       let filePath = await getXdebugExtensionModule(version);
+       if (filePath.startsWith('file://')) {
+               filePath = filePath.substring(7);
+       }
+

I achieved to reproduce the issue with this command :

node --experimental-wasm-jspi --loader=./packages/meta/src/node-es-module-loader/loader.mts packages/php-wasm/node/src/lib/xdebug/test.ts

where my test is simply :

import { withXdebug } from "./with-xdebug";

withXdebug( '8.3', {} );

However, I found out the error occurred because the loader added file:// to the path I return with the following code at line 120 :

const specifierUrl = new URL(specifier, 'file://');
for (const format of ['raw', 'json', 'url']) {
	if (specifierUrl.searchParams.has(format)) {
		// This is a custom format import and can be handled by our custom loader.
		return {
			url: specifierUrl.href,
			format,
			shortCircuit: true,
		};
	}
}

So we have two solutions here :

  1. I apply what you suggested in the with-xdebug.ts file and add a comment to explain why.
  2. I customize the ?url in ?import-url or something to separate the Vite ?url behavior from our custom plugin.

Even if these solutions are temporary, because I guess we will soon propose a better feature based on this issue, I would like to suggest the second solution to avoid any questions about that particular behavior when using a loader.

What do you think about that ?

@adamziel
Copy link
Collaborator Author

adamziel commented Jul 8, 2025

I agree, thank you for good investigation!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants