search

twilio / twilio-video.js

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

Works great with <script> tags but failes with npm package #9

Closed enahum closed 8 years ago

enahum commented 8 years ago

I've been playing around with the Twilio Video and if I use the script tags I can create a new Twilio.AccessManager, new Twilio.Conversations.Client and so on.

If I try to load the library through npm and webpack, browserify, whatever the same code just doesn't work.

Any pointers on this?

enahum commented 8 years ago

I made it work but I had to comment this line https://github.com/twilio/twilio-conversations.js/blob/01bf25157561d64413336b5e776d5af1165e8f6e/lib/client.js#L5 because for some reason require('twilio-common') returns undefined while loading with webpack.

Feedback on this?

markandrus commented 8 years ago

Hi @enahum, and thanks for writing in.

I think what you are seeing is due to the browser override we have in place for twilio-common. It's defined here in package.json and points to src/twilio-common.js. So the line you point out ends up trying to require a browser global (Twilio) that does not exist when you go the Webpack/Browserify route.

We created the browser override for <script> tag users, thinking it would be nice if they could include twilio-common.js and twilio-conversations.js separately. But since then we've decided against it (makes updating harder, doesn't really give the user anything, etc.).

So I think in the next release we will change this: the <script> version of twilio-conversations.js will begin bundling twilio-common.js, and using Webpack/Browserify should "just work".

enahum commented 8 years ago

Thanks @markandrus so far I forked the project and made the change to that line of code and now it works with webpack.

BTW you should update dependancies and review tests cause two of them were failing for me

markandrus commented 8 years ago

@enahum this should be fixed in 0.13.6. Can you retest? Thanks! (also, integration tests should be a little more robust now)