capricorn86 / happy-dom

A JavaScript implementation of a web browser without its graphical user interface
MIT License
3.42k stars 207 forks source link

incompatibility with axe due to field missing in prototype #978

Open cdaringe opened 1 year ago

cdaringe commented 1 year ago

Describe the bug

axe-core accessibility tooling polyfills Node API(s) (tbd why they do this), which yields

TypeError: Cannot set property isConnected of [object Object] which has only a getter

...when creating a Node.

isConnected is a readonly field the ulitmately code-gens something like this in happy-dom:

function Node() {
  this.isConnected = false
}

axe checks the Node prototype , which adds a getter on field isConnected if its not found in the prototype:

    if (window.Node && !('isConnected' in window.Node.prototype)) {
      Object.defineProperty(window.Node.prototype, 'isConnected', {
        get: function get() {
          return !this.ownerDocument || !(this.ownerDocument.compareDocumentPosition(this) & this.DOCUMENT_POSITION_DISCONNECTED);
        }
      });
    }

...which ultimately prevents happy-dom from doing this.isConnected = false

To Reproduce

Hopefully the above enough is clear. This is more of a discussion.

Expected behavior

Node.prototype.isConnected, at least in chrome, does exist in browser. Should we put attributes on prototypes, rather just flatly on object instances, to improve browser compatibility/mirroring? One could argue that axe shouldn't be polyfilling, 🤷 .

It would be great if we could coach the TS compiler to take some class attributes and hoist them to the prototype. I'm not sure there's a good way to do that.

Screenshots n/a

Device: n/a

Additional context I'm trying to wire up axe to happy-dom. analzye(html: string) => Promise<axe.Result>

cdaringe commented 1 year ago

workaround is trivial, but tracing was a bit wonky! https://github.com/cdaringe/axe-html/blob/main/src/mod.ts#L18-L19

productdevbook commented 1 year ago
 FAIL  packages/components/arrow/src/arrow.test.ts [ packages/components/arrow/src/arrow.test.ts ]
TypeError: Cannot set property isConnected of [object Object] which has only a getter
 ❯ new Node node_modules/.pnpm/happy-dom@10.10.4/node_modules/happy-dom/src/nodes/node/Node.ts:58:29
 ❯ new Element node_modules/.pnpm/happy-dom@10.10.4/node_modules/happy-dom/lib/nodes/element/Element.js:24:9
 ❯ new HTMLElement node_modules/.pnpm/happy-dom@10.10.4/node_modules/happy-dom/lib/nodes/html-element/HTMLElement.js:17:9
 ❯ new HTMLTemplateElement node_modules/.pnpm/happy-dom@10.10.4/node_modules/happy-dom/lib/nodes/html-template-element/HTMLTemplateElement.js:12:9
 ❯ HTMLDocument.createElementNS node_modules/.pnpm/happy-dom@10.10.4/node_modules/happy-dom/src/nodes/document/Document.ts:815:19
 ❯ HTMLDocument.createElement node_modules/.pnpm/happy-dom@10.10.4/node_modules/happy-dom/src/nodes/document/Document.ts:784:15
 ❯ ../../../node_modules/.pnpm/@vue+runtime-dom@3.3.4/node_modules/@vue/runtime-dom/dist/runtime-dom.esm-bundler.js packages/core/utils/dist/index.mjs:15632:52
    15630|     svgNS = "http://www.w3.org/2000/svg";
    15631|     doc = typeof document !== "undefined" ? document : n…
    15632|     templateContainer = doc && /* @__PURE__ */ doc.creat…
       |                                                    ^
    15633|     nodeOps = {
    15634|       insert: (child, parent, anchor) => {
 ❯ __init packages/core/utils/dist/index.mjs:8:56
 ❯ ../../../node_modules/.pnpm/vue@3.3.4/node_modules/vue/dist/vue.cjs.js packages/core/utils/dist/index.mjs:16410:23
 ❯ __require packages/core/utils/dist/index.mjs:11:50

same problem vitest + axe

nocive commented 9 months ago

2024 :tada:

ninrich commented 4 months ago

@capricorn86 can you please provide some insight into whether this is going to be fixed in a future release? Does the fix mentioned by @cdaringe work for you? Thank you.