Temasys / AdapterJS

AdapterJS Javascript Polyfill and Tools for WebRTC - Skylink WebRTC
http://skylink.io/web
Other
430 stars 100 forks source link

global objects should be exported or set to window object #163

Closed zdila closed 8 years ago

zdila commented 8 years ago

It is impossible to get attachMediaStream when using webpack and source

import AdapterJS from 'adapterjs';

Object AdapterJS has no reference to attachMediaStream or other objects/functions.

window.RTCPeerConnection is available (because it is set to the window object)

johache commented 8 years ago

I am not familiar with webpack. attachMediaStream is an attribute of AdapterJS. It is a global function. Did you take this in account ?

zdila commented 8 years ago

Maybe I do something wrong, but in my case AdapterJS.attachMediaStream is undefined. It contains only following properties: > Object.keys(AdapterJS) < ["options", "VERSION", "onwebrtcready", "_onwebrtcreadies", "webRTCReady", "WebRTCPlugin", "onwebrtcreadyDone", "maybeThroughWebRTCReady", "TEXT", "_iceConnectionStates", "_iceConnectionFiredStates", "isDefined", "parseWebrtcDetectedBrowser", "maybeFixConfiguration", "addEvent", "renderNotificationBar"]

Regarding Webpack - it just packages the modules from npm (and bower) to single bundle.js. If a module uses global variables, they are treaten as module-local variables and are not visible to other modules. But assigning to window.attachMediaStream should work. Also it is possible to tell Webpack to bind module-local variables of specific module to window so I can work it around. They say it is for "broken" modules.

johache commented 8 years ago

oops, I missed the not... attachMediaStream is NOT an attribute of AdapterJS.

you should be able to access it globally.

zdila commented 8 years ago

OK, I can. But I must tell webpack to treat AdapterJS as "broken" module.

To "fix" it there may be more alternatives. One of:

johache commented 8 years ago

@ncurrier I'm fine with setting window.attachMediaStream = attachMediaStream Any opinion on this ?

zdila commented 8 years ago

It was hard but my workaround (using Webpack) is:

const ajs = require("imports?module=>false!exports?AdapterJS&attachMediaStream&getUserMedia&RTCPeerConnection&createIceServer!adapterjs");
const {AdapterJS, attachMediaStream, getUserMedia, RTCPeerConnection, createIceServer} = {...ajs};
johache commented 8 years ago

I'll add attachMediaStream in AdapterJS and in window either today or tomorrow

johache commented 8 years ago

PR open : https://github.com/Temasys/AdapterJS/pull/165

johache commented 8 years ago

PR merged and released: https://github.com/Temasys/AdapterJS/releases/tag/0.13.1

I am closing this issue, please feel free to re-open it if relevant.

zdila commented 8 years ago

All public properties should be added to window or better - should be exported from the module.

My current working solution (workaround) for webpack is to import adapterjs like this:

require('!script!adapterjs/publish/adapter.debug.js');

johache commented 8 years ago

I agree. @letchoo we should do that in the near-ish future.