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

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

Closed
andrewthebold opened this issue Feb 1, 2024 · 7 comments
Closed

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

andrewthebold opened this issue Feb 1, 2024 · 7 comments

Comments

@andrewthebold
Copy link

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

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
Copy link

sureshjoshi commented Feb 6, 2024

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
Copy link
Member

dummdidumm commented Mar 5, 2024

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 capricorn86/happy-dom#1288 to request a fix.

@dummdidumm
Copy link
Member

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

@sureshjoshi
Copy link

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
Copy link

mcous commented Apr 6, 2024

Hello from testing-library/svelte-testing-library#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
Copy link

mcous commented Apr 7, 2024

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
Copy link
Member

Thank you for pushing this forward!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants