socketio / socket.io-client

Realtime application framework (client)
https://socket.io
10.62k stars 3.04k forks source link

Fix debug import #1585

Closed FossPrime closed 1 year ago

FossPrime commented 1 year ago

The kind of change this PR does introduce

Current behaviour

esm-debug will not work on most environments. Screenshot 2023-06-01 21 39 13

This is a hidden problem in Vite, which uses the browser version from node_modules. The browser version has debug stripped, which is VERY undesirable as vite is primary used in dev, and in production a built version with debug code stripped out is compiled.

New behaviour

esm-debug will work when called directly in node or via unpkg.

Other information (e.g. related issues)

https://github.com/socketio/socket.io/issues/4731 https://v2.vitejs.dev/config/#resolve-conditions

unpkg's ?module still wont work due to debug not being an ESM package, and unpackage not supporting ESM packages that import CJS. A fine solution would be to wrap it ourselves. We could also import the global by path, but I don't think we can rely on ESM to localize the global.

Confirmed working with:

StackBlitz

Before PR:

Screenshot 2023-06-02 00 05 32

After PR:

Screenshot 2023-06-02 00 04 51

darrachequesne commented 1 year ago

That sounds reasonable :+1: Any idea how we could enable the logs for the underlying engine.io-client module too?

FossPrime commented 1 year ago

I was doing this primarily for the front ent, where you're far more likely to be in ESM exclusive mode due to script element type=module, UNPKG's ?module and esm.sh like hosts.

On the server, it would almost always be a no-op situation... as node matches the condition and is listed first so it takes priority. I did make a deno debug module, a place where node would be ignored and the browser condition would win. But in that case, that's a really good thing, as Deno can't support CommonJS modules like debug at all.