fix: preserve this inside addEventListener

[esjs]

What is it?

• Feature / enhancement
• Bug
• Docs / tests

Description

This will update addEventListener method to always have correct this value, even when it's called without context.

Use cases and why

While updating OneTrust service integration on our website to use Partytown, I've noticed that they are using addEventListener alongside window.attachEvent to support older versions of browsers.

The problem with the code they've written is that when addEventListener is called inside of WebWorker's addEventListener it has no context (this === undefined) and we get an error

Looks like we can easily update Partytown to support such cases by simply updating addEventListener to use arrow function (which preserves context).

P.S. postMessage method in this class has same problem, but I'm not sure whether we need to update as well, although I don't see any problems in doing so. I can update this PR to make it use arrow function if you think that it's a good idea.

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

[vercel]

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	Nov 7, 2023 8:06pm

[gioboa]

@esjs Out of curiosity, could I know your application stack? what third party scripts are you using and which framework?

[esjs]

@gioboa Sure, it's not a big secret.

We have a PHP backend. On frontend we don't use any frameworks, it's just a good old jQuery (although we have a plan to get rid of it), everything is bundled together using custom Webpack build.

In terms of 3rd party code, it's really hard to give you definitive list, because we have a lot of editions for different languages.
We use Google Ad Manager to serve our ads, but there is an auction and list of ad providers may change and depends on edition as well.
We also use OneTrust to handle 3rd party consent collection and Piano to handle custom user behaviors.

As I said previously it's really hard to give you a definitive list of all 3rd party services we use, but you can check yourself here https://www.motorsport.com/. Keep in mind that there is more than one edition, full list looks like this

[gioboa]

OneTrust and Piano are working fine with PartyTown?

[esjs]

OneTrust seems to be working fine with this fix in place.
I haven't checked Piano yet (PartyTown integration is still in progress) , but it serves it's content inside of an iframe (there is just small initialization script) so should be just fine, but of course need to be tested for compatibility before we can enable it all on production servers.

[gioboa]

Thanks for sharing 😉

[spacedawwwg]

@esjs - How did you get OneTrust working with party town? Did you need to forward anything?

This is the error I'm getting trying to use it:

[gioboa]

@spacedawwwg let's wait @esjs response 😉
In the error I can see: "reading length..."
can you tell me the variable name pls? you should see this .length

[spacedawwwg]

@gioboa I think we diagnosed this one as the OneTrust script trying to access a DOM element (a button that we have to add to every page) that does not exist (as it's trying to find it in the partytown iframe, not the 'real' DOM) - So I'm not 100% sure on a solution for this as we need to the script to bind to that button.

My colleague @ajwhitehead88 thinks we need to be able to access window.OneTrust and window.OnetrustActiveGroups from the partytown iframe to be able to make it work as well.

<sarcasm>All fun and games these big enterprise third party scripts</sarcasm> 😆

[esjs]

@spacedawwwg I cannot check this right now (maybe something changed and it doesn't work anymore), but it worked for me with all changes in this fork (as far as I remember there were 3 of them in total).

[gioboa]

Thanks @esjs
@spacedawwwg can you open a specific issue for OneTrust with your information pls?

[esjs]

@gioboa All of changes in fork mentioned above have merge request for main repo, so there is no need to check what is the difference if it would work.
I just created it, because I need a way to check how all of the changes work together, while I was waiting for approval on main repo.

[spacedawwwg]

@gioboa Absolutely, will do 👍🏻 It'd be good discuss the nuances of working with Partytown and what to look out for/configure with more complicated third party scripts like OneTrust.

@esjs It might just be that OneTrust code is different per implementation and our scripts are doing something different 🤷🏻‍♂️