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

Patch the registry class only when polyfilling #524

Merged
merged 2 commits into from
Sep 15, 2022

Conversation

rictic
Copy link
Contributor

@rictic rictic commented Sep 15, 2022

Currently we always write/overwrite window.CustomElementsRegistry when the polyfill loads, even if we don't patch anything else, including window.customElements

If the browser has a native implementation of custom elements, this means that customElements isn't an instance of CustomElementsRegistry, so patching CustomElementsRegistry.prototype, as redefine_custom_elements does, does not effect how customElements behaves. This leads to fun and mysterious bugs!

Better to only write to CustomElementsRegistry when we're doing other patching, and otherwise leave it alone.

Fixes #340.

Currently we always write/overwrite window.CustomElementsRegistry when the polyfill loads, even if we don't patch anything else, including window.customElements

If the browser has a native implementation of custom elements, this means that customElements isn't an instance of CustomElementsRegistry, so patching CustomElementsRegistry.prototype, as redefine_custom_elements does, does not effect how customElements behaves. This leads to fun and mysterious bugs!

Better to only write to CustomElementsRegistry when we're doing other patching, and otherwise leave it alone.
@bicknellr
Copy link
Collaborator

LGTM with a changelog entry, thanks!

@rictic rictic merged commit 64da220 into master Sep 15, 2022
@rictic rictic deleted the write-registry-only-when-polyfilling branch September 15, 2022 19:21
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.

window.CustomElementRegistry is always overwritten, even when the polyfill isn't needed
2 participants