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

CommonJS incompatible? (events, stream) #87

Open
zardoy opened this issue Aug 28, 2023 · 5 comments
Open

CommonJS incompatible? (events, stream) #87

zardoy opened this issue Aug 28, 2023 · 5 comments

Comments

@zardoy
Copy link

zardoy commented Aug 28, 2023

to demonstrate:

console.log(typeof require("events")) // (Node.js, events npm package): function

console.log(typeof require("events")) // (from nodelibs of this package): object { default, ... }

I noticed after bundle errors like TypeError: Class extends value #<Object> is not a constructor or null of the following sample code below (coming here from esbuild plugin):

const EventEmitter = require('events');

class A extends EventEmitter {} // ...

Please ask if unclear.

@guybedford
Copy link
Member

Thanks for sharing, agreed this is an important case to fix. Tracking.

@chrisdostert
Copy link

Thanks for all the work on this library, just sharing one place this is currently causing breaks for me. Twilio-video attempts to extend EventEmitter in quite a few classes and the above error is encountered. Just also noting, the recently released Remix v2 migrates to the jspm-core polyfills from this repo so this likely will start effecting more people.

@zardoy
Copy link
Author

zardoy commented Sep 17, 2023

Thanks for all the work on this library, just sharing one place this is currently causing breaks for me.

IMO this is really huge breakage since as I understand the intention of that plugin is to support legacy code. Also, I'm curious to know why jspm was chosen instead of the classical webpack-way polyfillying? I'm using alias and banner options instead now which work fine :shrug :

@guybedford
Copy link
Member

guybedford commented Sep 19, 2023

One option here might be to use package exports conditions to match a CommonJS implementation as necessary for this specific builtin. A suitable conditions might be "commonjs", defining this condition to mean "the build environment supports commonjs" (effectively the converse of the "module" condition).

@guybedford
Copy link
Member

Note this story will improve with Node.js's new require(esm) support.

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

No branches or pull requests

3 participants