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

[BUG] cannot import Device or Call #204

Open
7 of 9 tasks
electrovir opened this issue Sep 29, 2023 · 13 comments
Open
7 of 9 tasks

[BUG] cannot import Device or Call #204

electrovir opened this issue Sep 29, 2023 · 13 comments
Labels
bug Something isn't working jira added

Comments

@electrovir
Copy link

  • I have verified that the issue occurs with the latest twilio.js release and is not marked as a known issue in the CHANGELOG.md.
  • I reviewed the Common Issues and open GitHub issues and verified that this report represents a potentially new issue.
  • I verified that the Quickstart application works in my environment.
  • I am not sharing any Personally Identifiable Information (PII)
    or sensitive account information (API keys, credentials, etc.) when reporting this issue.

Code to reproduce the issue:

import {assert} from '@open-wc/testing';
import {Call, Device} from '@twilio/voice-sdk';

describe('Twilio Call and Device', () => {
    it('are defined', () => {
        assert.isDefined(Call);
        assert.isDefined(Device);
    });
});

Expected behavior:

I can run tests for my browser code that imports Twilio Call or Device using web-test-runner

Actual behavior:

Safari (WebKit), Firefox, and Chrome (Chromium) all throw various errors about not being able to import Call or Device:

  • WebKit: "Indirectly exported binding name 'Device' is not found."
  • Chromium: "The requested module 'node_modules/@twilio/voice-sdk/es5/twilio.js' does not provide an export named 'Call'"
  • Firefox: "ambiguous import: Call"

Software versions:

  • Browser(s): Webkit 17.0, Chromium 116.0.5845.82, Firefox 115.0 (all from playwright 1.37.1)
  • Operating System: macOS
  • twilio.js: N/A
  • @twilio/voice-sdk: 2.7.2
  • Third-party libraries (e.g., Angular, React, etc.): @web/test-runner 0.17.1
@electrovir electrovir added the bug Something isn't working label Sep 29, 2023
@electrovir
Copy link
Author

electrovir commented Sep 29, 2023

Originally I thought this was related to #55, but our actual app runtime (built with webpack currently) imports Device and Call without issue (ever since #55 was fixed). Webpack may be doing some kind of magic (or maybe one of our plugins) to make this work.

@electrovir
Copy link
Author

I notice that web-test-runner is trying to the es5 files, which use require instead of import (which fails in the browser). I expected this to not be an issue since this package defines both module and main in its package.json, but it looks like the exports definition is actually overriding these so they (main and module) are not doing anything.

I see ./esm is defined in exports, so I tried using that:

import {assert} from '@open-wc/testing';
import {Call, Device} from '@twilio/voice-sdk/esm';

describe('Twilio Call and Device', () => {
    it('are defined', () => {
        assert.isDefined(Call);
        assert.isDefined(Device);
    });
});

And now I get "Importing binding name 'EventEmitter' is not found." errors.

@electrovir
Copy link
Author

Importing from either of the dist files (dist/twilio.js and dist/twilio.min.js) doesn't help. (Plus, you can't import those without modifying node_modules/@twilio/voice-sdk/package.json anyway since the exports field prevents you from importing from dist.) They both use require anyway (so it seems to not actually be built-for-browser output?).

(Btw this is coming from an attempt at upgrading from the V1 api)

@electrovir
Copy link
Author

Further errors after making hacky fixes:

  • EventEmitter from node_modules/events/events.js
  • levels from node_modules/loglevel/lib/loglevel.js
  • Can't find variable: module from node_modules/events/events.js
  • countless other errors from loglevel

It appears that that this Twilio package is simply not expected to be used in the web without a significant build process in front of it.

Workaround for now: detect when tests are being run and use mock Twilio objects in that case. We'll have to write the mock ourselves (since I don't see one in any of these docs?), which will be cumbersome but at least possible.

@charliesantos
Copy link
Collaborator

Thanks for submitting @electrovir . Can you please confirm that you're only seeing this when using the web-test-runner?
Regarding the dist folder, those are meant to be loaded via a script tag

<script type="text/javascript" src="twilio.min.js"></script>

Once loaded, the SDK attaches a global Twilio object where you can access the Device class etc.

const Device = Twilio.Device;
const Call = Twilio.Call;

@electrovir
Copy link
Author

It's only happening with web-test-runner and not webpack (which we currently use to bundle our frontend code), yes.

@srtella
Copy link

srtella commented Oct 24, 2023

We are also facing similar issue with the voice device import statement (tried with voice sdk 2.8.0 ): import { Device } from "@twilio/voice-sdk/esm".
We are using dev-server . When we try to run our application, we are getting error:
/../../events/events.js' does not provide an export named 'EventEmitter' (at backoff.ts:8:10)

This is blocking us from updating the SDK to latest versions.

@charliesantos
Copy link
Collaborator

@electrovir @srtella we're going to start looking at this in the next couple of weeks. To speed up the process, do you have any example application that we can download and run to reproduce the issue without us having to build something from scratch?

@electrovir
Copy link
Author

I can set that up for you.

@charliesantos
Copy link
Collaborator

@electrovir that would be great! Thank you.

@electrovir
Copy link
Author

Here it is:
https://github.com/electrovir/broken-twilio-device-import-example

Note that you can see an example test failure here:
https://github.com/electrovir/broken-twilio-device-import-example/actions/runs/7064366321/job/19232242869#step:4:140

@charliesantos
Copy link
Collaborator

@electrovir @srtella looking at this more, it seems @web/test-runner is not properly handling CommonJS modules. Unlike webpack, even when using ESM, everything works fine. Does @web/test-runner have any config that stops treating CommonJS as ESM?

Anyway, we can try to update all our dependencies to use ESM but that would take a significant amount of time and effort. I don't see it happening soon. In the meantime, I see that @electrovir has a workaround. You can either:

@charliesantos
Copy link
Collaborator

@electrovir @srtella by the way, you have mentioned you observed this issue since you're upgrading to v2 of the voice sdk. Are you not seeing this on v1?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working jira added
Projects
None yet
Development

No branches or pull requests

3 participants