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

Fix debug import #1585

Merged
merged 8 commits into from Jun 20, 2023
Merged

Fix debug import #1585

merged 8 commits into from Jun 20, 2023

Conversation

FossPrime
Copy link
Contributor

@FossPrime FossPrime commented Jun 2, 2023

The kind of change this PR does introduce

  • a bug fix

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)

socketio/socket.io#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:

  • import io from '../node_modules/socket.io-client/build/esm-debug/index.js'
  • import io from 'socket.io-client'
  • import io from 'socket.io-client' // TS with esModuleInterop

StackBlitz

Before PR:

Screenshot 2023-06-02 00 05 32

After PR:

Screenshot 2023-06-02 00 04 51

@darrachequesne
Copy link
Member

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

 the next line does.

Debug shouldn't be destructured...
@FossPrime
Copy link
Contributor Author

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.

@darrachequesne darrachequesne merged commit 781d753 into socketio:main Jun 20, 2023
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants