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

Cleanup upstreamed patches & fix angular #291

Merged
merged 4 commits into from
Nov 23, 2021
Merged

Cleanup upstreamed patches & fix angular #291

merged 4 commits into from
Nov 23, 2021

Conversation

t2t2
Copy link
Contributor

@t2t2 t2t2 commented Nov 22, 2021

Description

Issues with angular: https://codesandbox.io/s/angular-material-tooltip-fhimx?file=/src/index.html

  1. Tooltip location isn't properly updated as while it waits for ngzone's microtask queue to flush out, the microtask never existed due to us patching Node (as a fix for IE not having EventTarget) vs zone patching EventTarget or Node depending on browser support. Node is before EventTarget in prototype chain so even though zone patches addEventListener later, it will never get called as Node's addEventListener is found first.

    Upstreamed version of the IE fix matches zone's patching = doesn't cause this issue, so fixing by removing our version of the fix.

  2. After angular gets to use zone again there's an issue with passive event listener feature detection where null event listener doesn't work as a key in weakmap used in otel code (TypeError: Invalid value used as weak map key)

    Added a quick check for non-values to addPatchedListener.

    Otel doesn't have this issue as it uses zone for event listener tracing if possible. But it's still worth pr-ing a fix to otel, probably should also check other "fun" ways to make event listeners (I think you can pass a string as a listener too, calling the global function with that name? That's also not valid for weakmap)

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How has this been tested?

  • Added unit tests

@t2t2 t2t2 temporarily deployed to integration November 22, 2021 14:14 Inactive
@t2t2 t2t2 merged commit c4f578b into main Nov 23, 2021
@t2t2 t2t2 deleted the cleanup-angular-fix branch November 23, 2021 08:44
sfishel-splunk pushed a commit to sfishel-splunk/splunk-otel-js-web that referenced this pull request Jun 22, 2022
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.

2 participants