ref: Rework DOM Breadcrumbs integration #3208
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
size-limit report
|
|
This looks great! (Sorry, I should have added all of my suggestions as comments under a single review, so you'd see this first.) Other than the stuff above (which is mostly only word-smithing and clarification of comments), my only feedback is that, like we talked about in our retro, it'd be great to have a few sentences in the PR description summarizing the overall problem this fixes and how the changes here solve it. |
Co-authored-by: Katie Byers <[email protected]>
1 task
5 tasks
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added appropriate tests and verified them on BrowserStack.
Fixes #2197
Fixes #2125
Possibly Fixes #2074
==============================
The previous implementation was faulty, as it instantly triggered a handler during listener registration, not when it was actually called in the future. This caused some breadcrumbs to be captured despite no user action having been performed.
Instead of wrapping the listener provided by the user, we register our own listener on a given element for the same event type and use it as the mechanism for triggering the instrumentation handler.
We always register only one instrumentation listener and keep track of how many listeners are attached overall with
refCount. When removing custom listeners, we only detach instrumentation once all references are gone. This way we don't store unnecessary data in the memory once it's not necessary.