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(worker-window): polyfill CSSMediaRule #614

Closed
wants to merge 1 commit into from

Conversation

nicksrandall
Copy link
Contributor

What is it?

  • Feature / enhancement
  • Bug
  • Docs / tests

Description

Include polyfill for CSSMediaRule (used by hubspot tracking js)

Use cases and why

Hubspot references CSSMediaRule in their JS and it is currently failing because it doesn't exist in worker context.
We already have the polyfill for CSSStyleSheet and CSSMediaRule has the same interface so the same polyfill should work!

Checklist:

  • My code follows the developer guidelines of this project
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • Added new tests to cover the fix / functionality

Copy link

vercel bot commented Aug 6, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
partytown ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 7, 2024 0:02am

@nicksrandall
Copy link
Contributor Author

Looks like CI had an issue installing PNPM. Can a maintainer please rerun the CI checks on this PR?

@nicksrandall
Copy link
Contributor Author

@gioboa Do you see any issues with adding this?

@gioboa
Copy link
Member

gioboa commented Aug 10, 2024

@gioboa Do you see any issues with adding this?

It looks good to me but I'm no longer on the core team that maintains the library. I'm sorry.

@nicksrandall
Copy link
Contributor Author

I discovered that microsoft clarity has patched this on their end and I'm not sure this solution makes sense as it only polyfilled the bare minimum of what was needed so I'm going to close for now.

@lampask
Copy link

lampask commented Sep 12, 2024

In the end, this should still go in. It wasn't patched in clarity and having this polyfilled inside partytown is arguably a better solution.

@azurre99
Copy link

Is there any update on this or some kind of workaround ? I'm having issues with GTM including clarity
Screenshot 2024-10-22 at 12 30 21
Screenshot 2024-10-22 at 12 30 42

@AndreAzevedoCoder
Copy link

I'm still having this issue with clarity, I think it's a good idea to merge this

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.

5 participants