search

daily-co / daily-js

https://docs.daily.co/reference/daily-js
BSD 2-Clause "Simplified" License
103 stars 33 forks source link

Minimal global namespace pollution #248

Open slavchev opened 8 months ago

slavchev commented 8 months ago

Feature request

We use daily-js in Cordova app and during initial prototyping we got the following error (sorry about the bad formatting)

Object = $1

action: "error"

error: Object

details: Object

bundleUrl: "https://c.daily.co/call-machine/versioned/0.60.0/static/call-machine-object-bundle.js"

on: "load"

sourceError: Error: Failed to load call object bundle https://c.daily.co/call-machine/versioned/0.60.0/static/call-machine-object-bundle.js: TypeError: window.store.getState is not a function. (In 'window.store.getState()', 'window.store.getState' is undefined)

column: 137770

line: 1

message: "Failed to load call object bundle https://c.daily.co/call-machine/versioned/0.60.0/static/call-machine-object-bundle.js: TypeError…"

sourceURL: "https://unpkg.com/@daily-co/daily-js@0.60.0/dist/daily.js"

stack: "@https://unpkg.com/@daily-co/daily-js@0.60.0/dist/daily.js:1:137770↵t@https://unpkg.com/@daily-co/daily-js@0.60.0/dist/daily.js:1:8933…"

Error Prototype

Object Prototype

msg: "Failed to load call object bundle."

type: "connection-error"

Object Prototype

errorMsg: "Failed to load call object bundle https://c.daily.co/call-machine/versioned/0.60.0/static/call-machine-object-bundle.js: TypeError…"

Object Prototype

The error was due to our use of https://www.npmjs.com/package/cordova-plugin-purchase which uses default clobber store. While it was trivial to fix it, it was unexpected and at least to my knowledge undocumented (compared to cordova-plugin-purchase which is well documented). Also it is hard to reason over call-machine-object-bundle.js as it is minified. So far it seems placeDailyContextOnWindow() sets the following global properties 

window.rtcpeers = this.rtcpeers,
window.dispatcher = this.dispatcher,
window.store = this.store,
window.sigChannel = this.sigChannel

As my current understanding is that call-machine-object-bundle.js is an internal library I would suggest to keep global namespace as less polluted as possible, maybe use double underscore prefix or just a single object, e.g. __dailyjs that holds all needed properties.

Why you need this

This feature will provide better software compatibility.

Alternatives you've considered

Maybe providing a simple documentation what global properties daily-js uses will be enough.

Additional context

mattieruth commented 8 months ago

Thank you for the report. We are currently undertaking this very issue and working to remove all dependencies on the window so that these will go away. It is a sizeable project that we hope to have publicly available in the next quarter.