-
Notifications
You must be signed in to change notification settings - Fork 94
Hold references to the pre-patched environment to prevent calling into later-applied polyfills. #102
Conversation
671bf3f
to
aac1395
Compare
src/Environment/EventTarget.js
Outdated
const addEventListenerMethod = | ||
method(descriptors.addEventListener) || | ||
// IE11 | ||
method(getDescriptor(window['Node'].prototype, 'addEventListener')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bicknellr VERY slick! I <3 it.
For reference, this JSBin (forked from this one, produced by @robdodson, to force the polyfills) shows a symptom of the current polyfills' layering problems. In particular, the first element should be able to find the second during its |
@bicknellr this explains so much! Been through this project a bunch of times and have to understand sometimes things are just the way they are just because! |
I didn't really include much description for the PR beyond the title; Rob recently ran across the issue being fixed here and showed me that link so I figured I'd add it here since for more context.
If you find something that isn't explained well, let me know and I'd be happy to answer questions and/or add docs as needed. |
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
CLAs look good, thanks! |
…E11 `isConnected` fast path.
Remaking this PR in the polyfills monorepo! |
Most of this PR is leading whitespace changes:
https://github.com/webcomponents/custom-elements/pull/102/files?w=1