Svelte 5: `happy-dom` test fails with >2 components in a file

andrewthebold commented 7 months ago

Describe the bug

I found that happy-dom-based tests break if a .svelte file has >2 components inside it.

It seems in this line, there is no null-check for node, causing a type error. If I patch it in, the test in the repro passes. I imagine it's either svelte has a bug, or svelte@5 is more strict and happy-dom is erroneously providing a null node.

Notably, the test passes with svelte@4 + happy-dom, so it seems like a svelte bug? However, it also passes if I swap to jsdom, so... I'm not sure.

Reproduction

Svelte 5 (fails): https://stackblitz.com/edit/svelte-5-happy-dom-test-uepmhg?file=src%2Flib%2FApp.svelte&view=editor
Svelte 4 (passes): https://stackblitz.com/edit/svelte-4-happy-dom-test-qdfde2?file=src%2Flib%2FApp.svelte&view=editor

Run pnpm test to run the bug.test.js file with vitest. Comment out the 3rd <Child /> in the svelte 5 example and the test will pass. The only difference between the two is the svelte version and using createRoot instead of new.

Logs

No response

System Info

System:
    OS: Linux 5.0 undefined
    CPU: (8) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 0 Bytes / 0 Bytes
    Shell: 1.0 - /bin/jsh
  Binaries:
    Node: 18.18.0 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 10.2.3 - /usr/local/bin/npm
    pnpm: 8.14.0 - /usr/local/bin/pnpm
  npmPackages:
    svelte: 5.0.0-next.44 => 5.0.0-next.44

Severity

blocking an upgrade

sureshjoshi commented 7 months ago

Ran into a similar problem using svelte-testing-library (and svelte5) - also passes with jsdom, fails with happy-dom - though in my case, a single HTML line in a svelte component causes the prettyDOM formatter to fall over, while two or more lines passes

dummdidumm commented 6 months ago

Tested this - the problem is that happy-dom doesn't seem to support the shorthand for comments, <!>, which browser auto-correct to . We do that to save space. Created https://github.com/capricorn86/happy-dom/issues/1288 to request a fix.

dummdidumm commented 5 months ago

Happy-dom 14.3.6 claims to have it fixed, haven't validatet it yet

sureshjoshi commented 5 months ago

FWIW I still get runtime errors when I use a single line in a component, rather than multiple with happy-dom (but JSDom still works fine).

So, something's going on. I just upgraded, so I'm still debugging.

TypeError: Cannot read properties of null (reading 'includes')
 ❯ HTMLElement.[connectToNode] node_modules/.pnpm/happy-dom@14.3.8/node_modules/happy-dom/src/nodes/html-element/HTMLElement.ts:494:14
 ❯ HTMLElement.[connectToNode] node_modules/.pnpm/happy-dom@14.3.8/node_modules/happy-dom/src/nodes/node/Node.ts:508:48
 ❯ HTMLElement.[connectToNode] node_modules/.pnpm/happy-dom@14.3.8/node_modules/happy-dom/src/nodes/html-element/HTMLElement.ts:598:38
 ❯ Function.removeChild node_modules/.pnpm/happy-dom@14.3.8/node_modules/happy-dom/src/nodes/node/NodeUtility.ts:115:45
 ❯ Function.removeChild node_modules/.pnpm/happy-dom@14.3.8/node_modules/happy-dom/src/nodes/element/ElementUtility.ts:118:15
 ❯ HTMLElement.removeChild node_modules/.pnpm/happy-dom@14.3.8/node_modules/happy-dom/src/nodes/element/Element.ts:517:25
 ❯ Svelte5TestingLibrary.cleanupTarget node_modules/.pnpm/@testing-library+svelte@4.2.2_patch_hash=gyderxtjqjyqcwxm3dw26xb52i_svelte@5.0.0-next.85/node_modules/@testing-library/svelte/src/pure.js:121:21
    119|     const inCache = this.targetCache.delete(target)
    120| 
    121|     if (inCache && target.parentNode === document.body) {
       |                     ^
    122|       document.body.removeChild(target)

TypeError: Cannot read properties of null (reading 'includes')
 ❯ HTMLElement.[connectToNode] node_modules/.pnpm/happy-dom@14.3.8/node_modules/happy-dom/src/nodes/html-element/HTMLElement.ts:494:14
 ❯ Function.insertBefore node_modules/.pnpm/happy-dom@14.3.8/node_modules/happy-dom/src/nodes/node/NodeUtility.ts:205:48
 ❯ Function.insertBefore node_modules/.pnpm/happy-dom@14.3.8/node_modules/happy-dom/src/nodes/element/ElementUtility.ts:208:16
 ❯ DocumentFragment.insertBefore node_modules/.pnpm/happy-dom@14.3.8/node_modules/happy-dom/src/nodes/document-fragment/DocumentFragment.ts:248:25
 ❯ Function.before node_modules/.pnpm/happy-dom@14.3.8/node_modules/happy-dom/src/nodes/child-node/ChildNodeUtility.ts:76:12
 ❯ Comment.before node_modules/.pnpm/happy-dom@14.3.8/node_modules/happy-dom/src/nodes/character-data/CharacterData.ts:210:20
 ❯ Module.insert node_modules/.pnpm/svelte@5.0.0-next.85/node_modules/svelte/src/internal/client/dom/reconciler.js:46:11
     45| 
     46| /**
     47|  * @param {import('#client').Dom} current
       |  ^
     48|  */
     49| export function remove(current) {

mcous commented 5 months ago

Hello from https://github.com/testing-library/svelte-testing-library/issues/319! I found some time to debug this, and it looks to me like the primary problem is that Svelte v5 relies on accessing a few methods through Node.prototype, like cloneNode. This works fine in browsers, but does not work in happy-dom.

It seems like the best path forward would be to fix this in happy-dom, but it looks like a fairly large change on their end based on how they've got all their classes set up

mcous commented 5 months ago

The just released happy-dom@14.7.1 is passing our Svelte 5 suite over in svelte-testing-library, so I think this issue has been resolved

dummdidumm commented 5 months ago

Thank you for pushing this forward!

sveltejs / svelte