GitHub: "Error: A custom element with name 'toggle-switch' has already been defined."

[Vangelis66]

Browser: Serpent 52.9.0 (2022-08-05) (32-bit) Extension version: Latest master HEAD (palefill-v1.20-2-git-20220907-g5c251f5)

Description

Over the last few days, when browsing any GitHub page, I couldn't help noticing the below Web Console error:

Error: A custom element with name 'toggle-switch' has already been defined.

The error is generated by GH script:

https://github.githubassets.com/assets/compat-838cedbb.js

Screengrab attached:

Is this something to worry about? Can it be eliminated?

Thanks for your stupendous, hard, work! 👍

[SeaHOH]

      customElements.define = function (name, cls) {
--      asNames.add(name);
        cls.prototype.addEventListener = addCEEventListener;
        cls.prototype.removeEventListener = removeCEEventListener;
++      if (asNames.has(name)) return;
++      asNames.add(name);
        oldCED.call(customElements, name, cls);
      };

[Vangelis66]

Thanks @SeaHOH :+1: ; your suggested patch (in file ./lib/polyfills.js) appears to "take care" of the reported error successfully! 🎉

--- polyfills.js.OLD    2022-09-07 23:00:16.000000000 +0300
+++ polyfills.js.NEW    2022-09-13 17:53:07.303486000 +0300
@@ -406,9 +406,10 @@
       rel.call(target, type, listener, options);
     }
     customElements.define = function (name, cls) {
-      asNames.add(name);
       cls.prototype.addEventListener = addCEEventListener;
       cls.prototype.removeEventListener = removeCEEventListener;
+      if (asNames.has(name)) return;
+      asNames.add(name);
       oldCED.call(customElements, name, cls);
     };
   }

[martok]

Very busy with work currently, work here will be slow for the forseeable future.

Can someone check the specification what the correct behavior should be? Obviously that patch suppresses the errors, but doing that when the original (or polyfill) actively throws seems wrong to me. Maybe something was changed and it isn't an error anymore, so frameworks just do it?

[martok]

So it was a standards issue, but in a different way than I expected.

The official polyfill throws errors as instances of the JS SyntaxError and Error classes. This is wrong, they should be DOMException subtypes. Something deeeeeep in Github's framework relies on that to detect and handle errors. This is the correct way, but doesn't work with the polyfill.

I'm fixing that on the attachShadow side even though it technically shouldn't be there since many more pages provide local copies of the customElements polyfill, so it's easiest to catch where we often attach anyway.