100mslive / web-sdks

Web SDKs for 100ms
https://www.100ms.live/docs/javascript/v2/quickstart/mental-model
MIT License
20 stars 25 forks source link

[Bug]: ESM/CJS Compatibility #2658

Closed mikericher closed 6 months ago

mikericher commented 6 months ago

What happened?

Attempting to import either the base sdk, this sdk, or roomkit prebuilts into an ESM only environment fails with various: SyntaxError: Cannot use import statement outside a module

Current environment is: remix.run (1.8) with vite configuration. tsconfig is using ESNext and Bundler

Reviewing the github src with: https://publint.dev/@100mslive/react-sdk@0.10.0

Any chance of updating/fixing the following errors so that it works with the "Bundler" option?:

The package does not specify the "type" field. Environments may incorrectly identify a CJS file as ESM in the future. Consider adding "type": "commonjs". <-- prefer 'module' here

The types is not exported. Consider adding pkg.exports["."].require.types: "./dist/index.d.ts" to be compatible with TypeScript's "moduleResolution": "bundler" compiler option. (More info)

./dist/index.js is written in ESM, but is interpreted as CJS. Consider using the .mjs extension, e.g. ./dist/index.mjs (More info)

./dist/index.js is written in ESM, but is interpreted as CJS. Consider using the .mjs extension, e.g. ./dist/index.mjs (More info)

How can we reproduce the bug?

No response

What browsers are you seeing the problem on?

No response

raviteja83 commented 6 months ago

@mikericher can you submit a minimum repro example(codesandbox or stackblitz), so that we can take a look at the exact errors. would you be open to contribute?

raviteja83 commented 6 months ago

https://discord.com/channels/843749923060711464/1171777502901973002 is this relevant for you?

mikericher commented 6 months ago

The discord chat configuration is using CommonJS, and I'm running ESM with the Vite Plugin. It will be default going forward as of remix 1.8. The discord chat was using an older V2 (that used a CommonJS base). (That and the other 20+ packages I have installed are also ESM only). There was an integration with remix.run and the react-sdk that worked, but it was a much older version of remix.

I did setup a github repo for you: https://github.com/mikericher/remix-vite-express-100mslive.git

If you need me to transfer to a codesanbox or stackblitz, I'll see if I have some time today (unlikely.. but will see if I can)

Mike

On Tue, Mar 5, 2024 at 12:33 PM Ravi theja @.***> wrote:

https://discord.com/channels/843749923060711464/1171777502901973002 is this relevant for you?

— Reply to this email directly, view it on GitHub https://github.com/100mslive/web-sdks/issues/2658#issuecomment-1979289898, or unsubscribe https://github.com/notifications/unsubscribe-auth/AUVHU5PKRSC3CEFS73ERXJ3YWX6VJAVCNFSM6AAAAABEHNEX7KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNZZGI4DSOBZHA . You are receiving this because you were mentioned.Message ID: @.***>

mikericher commented 6 months ago

Just for historical reference, here is a blog showing the remix and 100mslive integration with the older version of remix: https://dpnkr.in/blog/remix-video-chat-app

raviteja83 commented 6 months ago

@mikericher Thanks for the repro example. we will take a look and get back to you by early next week. I am not well versed with remix, so just want to confirm if the app is client side rendered or server side?

mikericher commented 6 months ago

It's "bundled" server side. tsconfig.json (relevant parts): "module": "ESNext", "moduleResolution": "Bundler"

a route file: export action <-- treeshaken server side only (mutations, POSTS,PUTS, etc.) export loader <-- treeshaken server side only (queries, GETS, HEADS) export default Component React Component (first render is server rendered, then hydrated client side) (subsequent renders, eg. useEffects, etc. are client rendered) (it could have a child component that is wrapped in a useEffect for client only rendering)

similar to next.js

raviteja83 commented 6 months ago

@mikericher we did publish an alpha version with type: 'module', I have also checked on the repo you shared. still did not fix the issue. if you want to try out, here is the version. '@100mslive/react-sdk': '0.10.2-alpha.3'. I have noticed other build issues. so, we won't be able to pursue this as of now.

mikericher commented 6 months ago

This does fix the main reported here in the package. (i.e. it will import correctly into an ESM "Bundler" Config).

It does cause additional errors with some of the dependencies, notably EventEmitter2.

SyntaxError: Named export 'EventEmitter2' not found. The requested module 'eventemitter2' is a CommonJS module, which may not support all module.exports as named exports.

It looks like it will be a similar problem with that package. (That is the package will be incorrectly inferred as an ESM, even though it is CJS) https://publint.dev/eventemitter2@6.4.9 I assume your using the EventEmitter2 for the wildcard support, rather than the built in node:events. I'll see about submitting a bug/feature request over there (not sure it will get any traction as the package looks unmaintained, but it wouldn't hurt).

raviteja83 commented 6 months ago

@mikericher You can try using only client side render, that should work. Unless you have any other concerns in doing so.