[scoped-custom-element-registry] Improve issues related to formAssociated
#581
Conversation
|
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). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
| @@ -11,12 +11,22 @@ | |||
| * subject to an additional IP rights grant found at | |||
| * http://polymer.github.io/PATENTS.txt | |||
| */ | |||
| if (!ShadowRoot.prototype.createElement) { | |||
| if (!(ShadowRoot.prototype.createElement && ShadowRoot.prototype.importNode)) { | |||
There was a problem hiding this comment.
Should this feature check be here at all?
There was a problem hiding this comment.
Reverted for now. Will have more to change/discuss to potentially better support Chrome's partial implementation in a follow-on.
There was a problem hiding this comment.
Based on dicussion with @justinfagnani and suggestion in #549, check was removed.
Will revisit in follow-on.
sorvell
changed the title
formAssociatedformAssociated
| } | ||
|
|
||
| // Polyfill helper object | ||
| (window as Window)['CustomElementRegistryPolyfill'] = { |
There was a problem hiding this comment.
Why is this global?
If it is global, should we check for its existence before writing? Something like:
(window as Window)['CustomElementRegistryPolyfill'] ??= {};
(window as Window)['CustomElementRegistryPolyfill']['formAssociated'] ??= new Set();
There was a problem hiding this comment.
Done and added comment.
| @@ -19,6 +19,22 @@ | |||
| * types, as though they were declared in a `declare global` in a module. | |||
| */ | |||
|
|
|||
| interface Window { | |||
There was a problem hiding this comment.
Prefer declare interface for google3 reasons (asks jscompiler not to rename)
Also, let's call it like, PolyfillWindow to not shadow the global Window interface
| // Patch attachShadow to set customElements on shadowRoot when provided | ||
| const nativeAttachShadow = Element.prototype.attachShadow; | ||
| Element.prototype.attachShadow = function ( | ||
| init: ShadowRootInitWithSettableCustomElements |
There was a problem hiding this comment.
I think we should keep a ...args here to pass through to the native, for future compat.
Fixes #546, #547, #580, #570, #549.
Note, bulk of changes in this commit.
The setting for a defined class'
formAssociatedcannot be properly polyfilled per registry. To address this, previously the polyfill always set the stand-in class to beformAssociated == true. However, this caused a series of subtle issues.Instead, it is now set to whatever the first defined tag name's
formAssociatedis set. It's also possible to reserve a given name to always be formAssociated by adding it tocustomElementRegistryPolyfill.formAssociated.add(tagName).Unconditionally installs polyfill.
Adds
registryto ShadowRootInit