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: Add patch dom mutator #146

Merged
merged 22 commits into from
Dec 10, 2024
Merged

fix: Add patch dom mutator #146

merged 22 commits into from
Dec 10, 2024

Conversation

hthr-lee
Copy link
Contributor

@hthr-lee hthr-lee commented Dec 10, 2024

Summary

Nested HTML editing doesn't work with the current implementation of dom-mutator. They just released a version 0.7.1 that we should probably leverage as that has some rate limiting with the number of mutations which run a minute. That being said, it doesn't directly address the issues that we're seeing. Here's the custom dom-mutator changes that we are keying off of: https://github.com/amplitude/dom-mutator/pull/7/files. To address the infinite loop issue, the lines of interest have to do with _textTimeout where if you setTimeout and pause the global mutation observer, then resume it by queueing something quickly in the JS event loop using setTimeout, we batch the changes together so that an element isn't being changed back and forth clobbering each other.

Note

This only works because we don't expect customers to install this package directly. If they do, it'll throw an error because the repo this change points to is private. We only want to use experiment-tag as a project to generate the experiment-script.min.js template that is used by our dynamic script service, and this is called locally from our machines, which do have access to this project.

Checklist

  • Does your PR title have the correct title format?
  • Does your PR have a breaking change?: no

TODO

  • I need to update the github runners to have a ssh key to pull from this repository if we take this approach instead of the patching approach.

@hthr-lee hthr-lee changed the title Add patch dom mutator fix: Add patch dom mutator Dec 10, 2024
@hthr-lee hthr-lee marked this pull request as ready for review December 10, 2024 01:25
@tyiuhc tyiuhc self-requested a review December 10, 2024 18:10
@hthr-lee hthr-lee merged commit a1afcfa into main Dec 10, 2024
8 checks passed
@hthr-lee hthr-lee deleted the add-patch-dom-mutator branch December 10, 2024 18:55
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