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

🐛 Force @percy/dom globalization #237

Merged
merged 3 commits into from
Mar 12, 2021

Conversation

wwilsman
Copy link
Contributor

What is this?

Sometimes modules or exports are polyfilled causing the UMD bundle to not attach the DOM script to the global scope. Since this scope issue seems to keep coming up, let's always set the serialize function globally when the window object is defined.

I also updated the bundle intro to use typeof rather than try-catch.

Sometimes modules or exports are polyfilled causing the UMD bundle to not attach the DOM script to
the global scope. Since this scope issue seems to keep coming up, let's always set the serialize
function globally when the window object is defined.
@wwilsman wwilsman added the 🐛 bug Something isn't working label Mar 12, 2021
@wwilsman wwilsman requested a review from Robdel12 March 12, 2021 19:50
Robdel12
Robdel12 previously approved these changes Mar 12, 2021
Copy link
Contributor

@Robdel12 Robdel12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🏁

@wwilsman wwilsman enabled auto-merge (squash) March 12, 2021 19:52
@Robdel12 Robdel12 self-requested a review March 12, 2021 20:17
@Robdel12 Robdel12 dismissed their stale review March 12, 2021 20:17

Tests failed

// `globalThis` is probably not defined
// works around instances where the context has an incorrect scope
if (typeof window !== 'undefined') {
window.PercyDOM = exports;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test error was because the test bundles use the global PercyDOM to reference this module, however setting this manually to a new object caused it to lose other module properties. Setting it to exports will set it to be the same value as the module exports (including a default property).

Copy link
Contributor

@Robdel12 Robdel12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🏁

@wwilsman wwilsman merged commit 8b8bdf9 into master Mar 12, 2021
@wwilsman wwilsman deleted the ww/force-dom-script-globalization branch March 12, 2021 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants