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

Minimal global namespace pollution #248

Open
slavchev opened this issue Mar 13, 2024 · 1 comment
Open

Minimal global namespace pollution #248

slavchev opened this issue Mar 13, 2024 · 1 comment
Labels
enhancement New feature or request

Comments

@slavchev
Copy link

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/[email protected]/dist/daily.js"

stack: "@https://unpkg.com/@daily-co/[email protected]/dist/daily.js:1:137770↵t@https://unpkg.com/@daily-co/[email protected]/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

@slavchev slavchev added the enhancement New feature or request label Mar 13, 2024
@mattieruth
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants