twilio / twilio-video.js

Twilio’s Programmable Video JavaScript SDK
https://www.twilio.com/docs/video/javascript
Other
571 stars 217 forks source link

Snowpack with Svelte and Typescript import error. #1347

Closed drozdzynski closed 1 year ago

drozdzynski commented 3 years ago

Code to reproduce the issue:

Source code and here is the result in browser console

Expected behavior:

I should be able to import the class from twilio-video package.

Actual behavior:

When just trying import import Video from 'twilio-video' I have error: Uncaught TypeError: Super expression must either be null or a function, not object and also on Firefox only I have error: Uncaught (in promise) TypeError: 'get peerIdentity' called on an object that does not implement interface RTCPeerConnection.

Software versions:

timmydoza commented 3 years ago

Hey @drozdzynski - thanks for the issue!

I'm able to reproduce this issue locally with snowpack. It looks like this issue has to do with the fact that out SDK relies on some built in node modules (events and util, specifically).

Snowpack uses rollup to create JavaScript bundles, and with rollup, it is not possible to include modules from node.js itself. Instead, a rollup plugin like rollup-plugin-node-polyfills must be used (which is what happens when the --polyfill-node flag is used with snowpack. It appears that this flag is used in your Codesandbox example).

This appears to be where the problem lies. Our code is expecting CommonJS modules when we require('events'), which will return the EventEmitter function. But when the --polyfill-node flag is used, these modules are imported as ES Modules, which returns the following:

{
  default: EventEmitter,
  EventEmitter: EventEmitter
}

This is what is causing the error that you are experiencing. What is being imported is not an EventEmitter, but an object that contains such. This also applies to the util library.

I found that if we stop relying on these built-in node modules in the SDK, and instead use the same libraries from NPM (see events and util), this will resolve the issue (as long as the @rollup/plugin-node-resolve rollup plugin is used with the { preferBuiltins: false } option).

I've created a ticket in our backlog to use the NPM version of these packages instead of the built-in node.js version. We will see if we can use these libraries in our SDK in a way that does not introduce any breaking changes.

marcelloma commented 3 years ago

@timmydoza Any known workarounds for this? I'm having the exact some issue without typescript with svelte+rollup.

ben-venntro commented 3 years ago

Having similar issues again with svelte+rollup

timmydoza commented 3 years ago

@marcelloma Unfortunately, it appears that the only workaround is to modify the SDK to remove the node dependencies and replace them with the equivalent dependencies from NPM. Then when you build the app with rollup, make sure that the @rollup/plugin-node-resolve rollup plugin is used with the { preferBuiltins: false } option.

drozdzynski commented 3 years ago

@timmydoza is there any progress with this issue? I reported it more than two months ago, and still no news about it. I also tested it with ViteJS which uses Rollup as a bundler so there also TwilioVideo does not work too.

vimtor commented 3 years ago

I also tried Snowpack for development and Twilio dependencies are blocking the start-up.

dariuszjastrzebski commented 3 years ago

I also tried with the snowpack and have the same issue. Any chance to fix it?

kasperkronborg commented 3 years ago

Just wanted to add, that I'm also seeing this with Vite 2.5.4 and React 17.0.0 and twilio-video 2.17.1. I believe that Vite relies on rollup too.

Furthermore when I tried to polyfill events, I got an error that the global object isn't defined. Which twilio-video also expects to be polyfilled. This is such a showstopper.