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

fix: avoid using window in module exports #477

Merged
merged 1 commit into from
Oct 8, 2024
Merged

Conversation

louis-bompart
Copy link
Contributor

@louis-bompart louis-bompart commented Oct 2, 2024

CDX-1593 🚀

Proposed changes:

dist/browser.mjs is the exported file in module.

The module can be imported by both NodeJS & browsers alike nowadays and as a library.

For example, we recently encountered an issue consuming the library after migrating Headless to an ESM-first package, as we export both ESM for NodeJS and browsers.

To solve that non-breakingly, I propose using the isomorphic way to access the global object instead of runtime specific, such as window or process.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/globalThis

From our experience in Headless, this is 'good enough', even if other places of the code uses window

How to test

Checklist:

  • todo Unit tests and/or functional tests are written and cover the proposed changes ❗
  • Unusure about how to tests, 🤔
  • The proposed change can't affect any current customer implementation that could lead to events being rejected from our APIs or events being miscategorized by UA.
  • Potentially no: If the library supports IE11, this change is not IE11 compatible, meaning events wouldn't be sent.

`dist/browser.mjs` is the exported file in `module`.

The module can be imported by both NodeJS & browsers alike nowadays and as a library.

For example, we recently encountered an issue consuming the library after migrating Headless to an ESM-first package, as we export both ESM for NodeJS & browsers alike.
@louis-bompart louis-bompart merged commit 21dc1cb into master Oct 8, 2024
4 checks passed
@louis-bompart louis-bompart deleted the fix/CDX-1593 branch October 8, 2024 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants