[BUG] [AMD] client with requirejs breaks

[symunona]

• [x] 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.
• [x] I reviewed the Common Issues and open GitHub issues and verified that this report represents a potentially new issue. The closest there is I found: CLIENT-7544
• [ ] I verified that the Quickstart application works in my environment. I do not think it's related
• [x] 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:

<script src="https://requirejs.org/docs/release/2.3.6/minified/require.js"></script>
<script src="twilio-client-1.10.js"></script> 

NOTE: It works with 1.9, breaks at 1.10 (introduced this logger)

Expected behavior: Twilio lib to work with requirejs.

Actual behavior: Uncaught TypeError: (intermediate value)(intermediate value)(intermediate value).getLogger is not a function

This is most probably a bundler scoping issue that does not play well with requirejs. If requirejs is imported before the script loads, it breaks.

This is an issue, because it breaks compatibility using AMD. i.e. twilio-client.js can NOT be imported through requirejs.

Software versions:

• [ ] Browser(s): Indifferent (chrome 83, ff 74)
• [ ] Operating System: Indifferent (linux)
• [x] twilio.js: 1.10 <- 1.9 is still good
• [x] Third-party libraries: [email protected]

[charliesantos]

Thank you @symunona for reporting. I added an internal ticket and we'll consider this on future releases. JIRA https://issues.corp.twilio.com/browse/CLIENT-7784

[mrkmrtns]

@charliesantos Any ETA? We have the same problem. Thank You!

[charliesantos]

Hi @mrkmrtns , no ETA at the moment. But we will take a look after this sprint (in a couple of weeks).

[charliesantos]

Hey @mrkmrtns @symunona, it seems the issue is related to loglevel when browserified then used with requirejs. In order to workaround this, try loading twilio first then load requirejs.

Instead of

<script src="https://requirejs.org/docs/release/2.3.6/minified/require.js"></script>
<script src="twilio-client-1.10.js"></script> 

You can do

<script src="twilio-client-1.10.js"></script> 
<script src="https://requirejs.org/docs/release/2.3.6/minified/require.js"></script>

I have verified both requirejs and twilio works well when loaded this way.

[symunona]

Hey @charliesantos !

This "workaround" is missing the very point. We use AMD for not having to include separate <script> tags, rather having one main require file, that will require Twilio lib only when needed -> making it possible to e.g. build it into a larger built bundle file, containing many modules.

So the problem is not really, that the first order does not work - that is for you helping to debug why your lib tries to use global require instead it's local one - the problem is that when you try to put it into a bundle, it will throw an error.

e.g.:

requirejs(['twilio-client'], function(Twilio){
   // This will already have the same error.
})

It has to be a scoping issue somewhere in the logger library added in that release.

[charliesantos]

Hey @symunona , Thanks for providing more details! I understand the use case. We'll investigate more on this.

Thanks, Charlie

[JeremyShapiro]

@charliesantos what's the status on https://issues.corp.twilio.com/browse/CLIENT-7784 ... ?

[charliesantos]

I'm going to pass this along to @ryan-rowland and @hawkinbj for prioritization.

[liberty-rowland]

Hi @JeremyShapiro, thanks for following up. We're currently working on some other big priorities, however, we'll be doing some planning in the coming weeks and I'll bring this up again for prioritization.

If anyone is in a hurry and feeling like contributing, this is what needed to change in our Video SDK to fix AMD: https://github.com/twilio/twilio-video.js/pull/1352/files

[JeremyShapiro]

@ryan-rowland , just circling back on this. What was the result of planning a few weeks back? Can we please get this scoping bug fixed?

[JeremyShapiro]

@ryan-rowland any update on this? @hawkinbj ? @charliesantos ?

[symunona]

I looked at the code here: https://github.com/twilio/twilio-video.js/pull/1352/files - and your solution was fixing the imported lib, I did the same: I imported the same, fixed lib from twilio-video.js.

Created the PR above, ran the build locally, it does fix the issue for us on v1.14.

Please let us know which version will it make!