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 feedback and support #284

Open
stefanhoelzl opened this issue Dec 17, 2023 · 23 comments
Open

Svelte 5 feedback and support #284

stefanhoelzl opened this issue Dec 17, 2023 · 23 comments

Comments

@stefanhoelzl
Copy link

stefanhoelzl commented Dec 17, 2023

Hello, @mcous has hijacked this post! We've launched experimental Svelte 5 support in @testing-library/svelte. See the Svelte 5 section in the README for setup instructions.

If you find bugs or have other Svelte 5 testing feedback, please add it to this thread! Svelte 5 is still changing rapidly at the time of writing, so feedback is appreciated!

Original post below


Hi! 👋

Firstly, thanks for your work on this project! 🙂

Today I used patch-package to patch @testing-library/svelte@4.0.5 for the project I'm working on to use svelte 5.

This patches some breaking changes but there might be more work to do for a proper PR (are tests still working? support svelte 4 and 5, maybe more)

But for everyone who likes to try out svelte 5 this patch should be a good starting point.

Here is the diff that solved my problem:

diff --git a/node_modules/@testing-library/svelte/src/pure.js b/node_modules/@testing-library/svelte/src/pure.js
index 04d3cb0..2f041e0 100644
--- a/node_modules/@testing-library/svelte/src/pure.js
+++ b/node_modules/@testing-library/svelte/src/pure.js
@@ -3,7 +3,7 @@ import {
   getQueriesForElement,
   prettyDOM
 } from '@testing-library/dom'
-import { tick } from 'svelte'
+import { tick, createRoot } from 'svelte'
 
 const containerCache = new Set()
 const componentCache = new Set()
@@ -54,18 +54,15 @@ const render = (
     return { props: options }
   }
 
-  let component = new ComponentConstructor({
+  let component = createRoot(ComponentConstructor, {
     target,
-    ...checkProps(options)
+    ...checkProps(options),
+    ondestroy: () => componentCache.delete(component)
   })
 
   containerCache.add({ container, target, component })
   componentCache.add(component)
 
-  component.$$.on_destroy.push(() => {
-    componentCache.delete(component)
-  })
-
   return {
     container,
     component,
@@ -73,18 +70,14 @@ const render = (
     rerender: (options) => {
       if (componentCache.has(component)) component.$destroy()
 
-      // eslint-disable-next-line no-new
-      component = new ComponentConstructor({
+      component = createRoot(ComponentConstructor, {
         target,
-        ...checkProps(options)
+        ...checkProps(options),
+        ondestroy: () => componentCache.delete(component)
       })
 
       containerCache.add({ container, target, component })
       componentCache.add(component)
-
-      component.$$.on_destroy.push(() => {
-        componentCache.delete(component)
-      })
     },
     unmount: () => {
       if (componentCache.has(component)) component.$destroy()

This issue body was partially generated by patch-package.

@jsorb84
Copy link

jsorb84 commented Dec 20, 2023

Thank you for this, this works flawlessly, you should def. make a pull request , cheers my friend.

@jakebottrall
Copy link

Thanks @stefanhoelzl, this is working a treat! Any ideas on how to test snippets? So far I haven't managed to figure out how to provide a snippet to the props API. So it's back to the old slots hack of making a wrapping component for tests.

@sureshjoshi
Copy link

Anyone have any luck with snippets? The wrapper components are boilerplate-laden - was hoping to not have to do that.

If anyone has any pointers, I'm happy to give it a shot myself.

@sureshjoshi
Copy link

sureshjoshi commented Feb 6, 2024

EDIT: Ignore this part - after a LOT of debugging, this seems to be a problem when using happy-dom instead of jsdom - which is insane, since I use happy-dom in all of my other projects... But that's a problem for another day :)

Also, @yanick - for this patch, does screen.debug() work anymore? I can't seem to even get the most basic Svelte component to work due to:

PrettyFormatPluginError: Cannot read properties of null (reading 'toLowerCase')

And that's even with a component which is just <div></div>

@mcous
Copy link
Collaborator

mcous commented Feb 12, 2024

I've been messing around with Svelte 5 in our existing test suite, and have encountered the following issues so far. I didn't see any of these explicitly called out in the change log, but I'm assuming at the moment that they're all intentional.

  • onMount and onDestroy are attached / called asynchronously, outside of the component creation / DOM mount
    • Judicious addition of act() can get the tests passing; if you render then unmount immediately, neither onMount nor onDestroy will be called
    • If we want to guarantee that render will call onMount, we'll need to make render async
  • anchor option has been removed from createRoot
  • Element.animate seems to be required to run animations at all, which will require faking something in jsdom/happy-dom to test components with animations
  • Weird happy-dom issue mentioned in comments above

@sysmat
Copy link

sysmat commented Feb 16, 2024

  • in which version of this library is this?
  • @testing-library/svelte: 4.1.0, "svelte": "5.0.0-next.55", tests not working
Instantiating a component with `new` is no longer valid in Svelte 5

@mcous
Copy link
Collaborator

mcous commented Feb 16, 2024

Rudimentary support for Svelte 5 has been released on the next tag

npm install —-save-dev @testing-library/svelte@next

Since Svelte 5 itself is not ready, expect issues and breaking changes if you try it out!

@sysmat
Copy link

sysmat commented Feb 16, 2024

@mcous thx, so @testing-library/svelte@next is "@testing-library/svelte": "^4.2.0",

with this version it works

@mcous
Copy link
Collaborator

mcous commented Feb 19, 2024

FYI for those following along: as of the latest svelte prerelease, our own Svelte 5 support will no longer work due to sveltejs/svelte#10516, which removed the API we were using to mount components. Will do our best to adjust accordingly

@yanick
Copy link
Collaborator

yanick commented Feb 20, 2024

as of the latest svelte prerelease, our own Svelte 5 support will no longer work due to sveltejs/svelte#10516

:darth vader big NOOO:

@sureshjoshi
Copy link

@mcous If you have any suggestions for directed ways to help, I'm all ears :) Though, less familiar with the internals of the lib.

Is the issue that they removed the API call we use? And that it needs to be migrated to the new one? Or is the new mount API a no-go?

@pboling
Copy link

pboling commented Mar 31, 2024

Anyone know of any updates?

@mcous
Copy link
Collaborator

mcous commented Apr 2, 2024

Experimental Svelte v5 support is available on the next dist tag

npm install --save-dev @testing-library/svelte@next

In your test files:

import { render } from '@testing-library/svelte/svelte5'

@yanick how would you feel about bumping the next branch to 5 (to account for breaking changes semantic-release missed) and merging into main? Are there any more breaks - e.g. dropping svelte@3 - that you'd like to collect on next first?

The svelte@5 support is appropriately marked as experimental and it seems like something we can promote in a minor release whenever svelte@5 goes into production

@pboling
Copy link

pboling commented Apr 2, 2024

Your previous update noted that Svelte 5 support did not work at all. It isn't clear from this update if it has been fixed.

@yanick
Copy link
Collaborator

yanick commented Apr 2, 2024

Fwiw, I'm kinda busy today, but I'll try to turn my attention to deployment tomorrow.

@mcous
Copy link
Collaborator

mcous commented Apr 2, 2024

Your previous update noted that Svelte 5 support did not work at all. It isn't clear from this update if it has been fixed.

It has been fixed for now, it may be broken tomorrow. With the very-in-progress nature of Svelte v5, the best way to find out if it's currently working will be to try it yourself

@sureshjoshi
Copy link

sureshjoshi commented Apr 2, 2024

When I tried testing-lib out a week or so ago, it was working - but also note that there are still bugs depending on whether you're using JSDom vs Happy-Dom, or from svelte itself, or any related libraries.

I've been working on a Svelte 5 headless UI port, and Svelte5 + interop is a bit messy at the moment (to say the least) :)

@mcous
Copy link
Collaborator

mcous commented Apr 18, 2024

Experimental Svelte 5 support has landed in main! We'll do our best to stay on top of things, but here there be dragons 🐉

npm install --save-dev @testing-library/svelte@latest now includes Svelte v5 support

@mcous mcous changed the title Patch for svelte 5 support Svelte 5 feedback and support Apr 24, 2024
@mcous mcous pinned this issue Apr 24, 2024
@MatthewTFarley
Copy link

MatthewTFarley commented Apr 28, 2024

When using svelteTesting Vitest setup function with Svelte 5, the autoCleanup option does not work because the function imports the Svelte 4 cleanup function by way of vitest.js.

I have also run into the Element.animate issue @mcous identified. Here's an example repository demonstrating it. It also has some comments mentioning the cleanup issue in the vite.config.ts file. Using vitest globals: true is a workaround since it will cause the correct cleanup to run here.

@mcous
Copy link
Collaborator

mcous commented Apr 29, 2024

When using svelteTesting Vitest setup function with Svelte 5, the `autoCleanup option does not work because the function imports the Svelte 4 cleanup function by way of vitest.js.

...Using vitest globals: true is a workaround...

@MatthewTFarley auto-cleanup via plugin works if you use the suggested Svelte 5 alias setup from the README! Otherwise, globals mode is probably your best bet. This awkwardness will go away once Svelte 5 is out of prerelease and the svelte5 import is no longer needed.

export default defineConfig({
  plugins: [svelte(), svelteTesting()],
  test: {
    alias: {
      '@testing-library/svelte': '@testing-library/svelte/svelte5',
    },
  },
})

I have also run into the Element.animate issue

I wonder what's going to happen here! The animations work in the browser, but neither jsdom nor happy-dom support Element.animate. I don't know if Svelte's intention is to require Element.animate or include some sort of fallback.

We may have to start thinking about shipping our own Element.animate stub, but faking out browser APIs in this library gets into some iffy-mocking-practices territory

@pboling
Copy link

pboling commented Apr 30, 2024

@mcous I don't know enough to know if what I have should be working, or is expected to be broken. I am on latest versions as of this second of all packages, and latest "next" versions where possible (e.g. Svelte 5 next 118). I have some basic tests working, following the patterns of the internal tests of this actual library (testing-library/svelte-testing-library). I'm trying to ensure my config remains working as Svelte5 nears release, and as I add more complex tests to my test suite, so I am following this issue.

When I use the alias mentioned above and in the readme, my test suite doesn't run:

 FAIL  src/lib/components/forms/input.test.ts [ src/lib/components/forms/input.test.ts ]
Error: Missing "./svelte5/vitest" specifier in "@testing-library/svelte" package
 ❯ e node_modules/.pnpm/vite@5.2.10_@types+node@20.12.7/node_modules/vite/dist/node/chunks/dep-DkOS1hkm.js:47596:25

My config (after adding the alias):

import { sveltekit } from '@sveltejs/kit/vite';
import { defineConfig } from 'vitest/config';
import { paraglide } from '@inlang/paraglide-sveltekit/vite';
import { svelteTesting } from '@testing-library/svelte/vite';
export default defineConfig(({ mode }) => ({
	plugins: [
		paraglide({
			// recommended
			project: './project.inlang',
			outdir: './src/lib/paraglide',
		}),
		sveltekit(),
		svelteTesting(),
	],
	resolve: {
		conditions: mode === 'test' ? ['browser'] : [],
	},
	test: {
		include: ['src/**/*.{test,spec}.{js,ts}'],
		environment: 'jsdom',
		setupFiles: ['./vitest-setup.ts'],
		alias: {
			'@testing-library/svelte': '@testing-library/svelte/svelte5',
		},
	},
}));

When I remove the alias property from test I am able to run my test suite. Hopefully this is helpful feedback!

@mcous
Copy link
Collaborator

mcous commented Apr 30, 2024

@pboling Looks like maybe our alias needs to be more specific; it's replacing imports it shouldn't. I'll experiment with this a little and update the README shortly. Upon further inspection, I think the problem might lie with a couple updates you should make to your config now that you're using the new svelteTesting plugin.

  • Remove import '@testing-library/svelte/vitest' from your ./vitest-setup.ts file
  • Remove the manual configuration of resolve.conditions

Both of these jobs the plugin now handles for you. I find that this configuration - as listed in the README - behaves well with Svelte 5:

// vite.config.ts
// ...
export default defineConfig({
  plugins: [sveltekit(), svelteTesting()],
  test: {
    include: ['src/**/*.{test,spec}.{js,ts}'],
    environment: "jsdom",
    alias: {
      "@testing-library/svelte": "@testing-library/svelte/svelte5",
    },
    setupFiles: ["./vitest-setup.ts"],
  },
});
// vitest-setup.ts
// Only needed to configure `@testing-library/jest-dom`
import '@testing-library/jest-dom/vitest`

If this doesn't work, you could try using a more specific alias. In my own projects, this is necessary only if I'm not using the svelteTesting plugin.

{
  // ...
  test: {
    // ...
    alias: [
      {
        find: /^@testing-library\/svelte$/,
        replacement: '@testing-library/svelte/svelte5',
      },
    ],
  // ...
  },
}

(All this being said, something feels off to me here. I think having a separate Svelte 5 import path is probably causing more trouble than it's worth. I don't think the situation will stick around too much longer, though)

@pboling
Copy link

pboling commented May 1, 2024

@mcous The top suggestions you gave are working! I did not need the more specific alternative alias.

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

No branches or pull requests

9 participants