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

Do not produce side-effects #2594

Open
romulof opened this issue Feb 7, 2024 · 2 comments
Open

Do not produce side-effects #2594

romulof opened this issue Feb 7, 2024 · 2 comments
Labels
enhancement New feature or request

Comments

@romulof
Copy link

romulof commented Feb 7, 2024

NPM package is producing side-effects, injecting itself in window.DD_RUM at module level:
https://github.com/DataDog/browser-sdk/blob/main/packages/rum/src/entries/main.ts#L39

Just by importing this library it changes global namespace, which is something not desired when using a module system.

Is your feature request related to a problem? Please describe.

I have a big micro-frontend application where each part of the application was handling the initialization of v4 from this NPM package. I'm migrating it to a single global initialization using v5.

I could disable the initialization of the v4 in a central place while I gradually clean up, but, due to this issue, just by these parts of the application importing v4 it already messes up my v5 initialization. I'll have to first do a huge cleanup, losing monitoring in the process, only then to enable my global v5 init.

Describe the solution you'd like

I'd like this NPM package to not change the global scope.

As bonus it could even declare in its sideEffects: false in its package.json, which can increase the bundlers efficiency (although I don't expect improvements in this case). More info here: https://webpack.js.org/guides/tree-shaking/#mark-the-file-as-side-effect-free

Describe alternatives you've considered

I tried to use the code bellow with my v5 initialization, to prevent it from being overwritten, but then the v4 imports start failing.

Object.defineProperty(window, 'DD_RUM', {
  configurable: false,
  enumerable: true,
  writable: false,
  value: window.DD_RUM,
});
@romulof romulof added the enhancement New feature or request label Feb 7, 2024
@amortemousque
Copy link
Contributor

Hello @romulof ,
We don't support several instances of the SDK on the same page. Maybe you could first move to single global initialization using v4 and then upgrade to v5.

@romulof
Copy link
Author

romulof commented Feb 26, 2024

I do get that my project has issues 😬, but it would be really good if Datadog didn't use global namespace. It could even remove the limitation of having multiple SDKs instances on the same page which could aid big projects to upgrade to new major versions of your library.

Mixpanel SDK does this. NPM version can only be accessed by module reference, and the one they deploy on a CDN uses window.mixpanel.

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