Tests are failing after updating to Vite 4.1.0

[yuliankarapetkov]

Describe the bug

I'm using SvelteKit with Vitest. I updated SvelteKit and Vitest to their latest versions - 1.5.0 and 0.28.4 respectfully. However, updating Vite from 4.0.4 to 4.1.0 caused 15% of my tests to fail (60/400).

Most of the errors seem unreasonable. For example, removing a DOM element throws a Failed to execute removeChild on Node error. Or Found multiple elements with the text: ... where there's just a single element containing that exact copy; Error: Unable to fire a "click" event - please provide a DOM element., Error: unable to find element with text, etc.

Reproduction

Here's a simple StackBlitz example.

1. Open the link
2. Wait for the installation to complete
3. Stop the execution in the terminal (Cmd + C)
4. Run npm run test
5. The test fails

Then do the following:

1. Open package.json
2. Change the version of vite from 4.1.0 to 4.0.4
3. Update the dependencies
4. Open the terminal again and run npm run test
5. The test succeeds

The example explained

It's a simple component with an if statement:

<script lang="ts">
  import { onMount } from "svelte"

  let showMessage = false

  onMount(() => {
    showMessage = true
  })
</script>

{#if showMessage}
  <div>Hello</div>
{/if}

Test file:

import { render } from "@testing-library/svelte"
import Hello from "../Hello.svelte"

describe("Hello component", () => {
  it("should show message", async () => {
    const { getByText } = render(Hello)
    expect(getByText("Hello")).toBeInTheDocument()
  })
})

Result with Vite 4.1.0:

TestingLibraryElementError: Unable to find an element with the text: Hello. This could be because the text is broken up by multiple elements. In this case, you can provide a function for your text matcher to make your matcher more flexible.

This is just a single example but similar issues occur where there's some sort of an asynchronous code used (or conditionals), like promises, setTimeout, useFakeTimers() and advanceTimersByTime(), TestingLIbrary's waitFor() function, etc.

More dependencies & config

See StackBlitz example for more info

package.json

"@sveltejs/kit": "^1.5.0",
"@testing-library/jest-dom": "^5.16.5",
"@testing-library/svelte": "^3.2.2",
"jsdom": "^21.1.0",

vite.config.ts

export default defineConfig({
  plugins: [sveltekit()],
  test: {
    include: ['src/**/*.{test,spec}.{js,ts}'],
    globals: true,
    environment: "jsdom",
    setupFiles: ["src/setupTest.js"],
  }
});

setupTest.js

import "@testing-library/jest-dom"

System Info

System:
    OS: macOS 13.1
    CPU: (16) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 1.84 GB / 16.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 19.2.0 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 8.13.1 
  Browsers:
    Brave Browser: 109.1.47.186
    Chrome: 109.0.5414.119
    Firefox: 105.0.1
    Safari: 16.2
  npmPackages:
    vite: 4.1.0 => 4.1.0 
    vitest: ^0.28.4 => 0.28.4

Used Package Manager

npm

Validations

• [X] Follow our Code of Conduct
• [X] Read the Contributing Guidelines.
• [X] Read the docs.
• [X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• [X] Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
• [X] The provided reproduction is a minimal reproducible example of the bug.

[github-actions[bot]]

Hello @yuliankarapetkov. Please provide a minimal reproduction using a GitHub repository or StackBlitz. Issues marked with need reproduction will be closed if they have no activity within 3 days.

[yuliankarapetkov]

Hey @sheremet-va, I just updated my comment with a StackBlitz example, along with some further explanation and config. Hope that gives a bit more context of the issue.

[AriPerkkio]

Test passes with following:

<script lang="ts">
-	import { onMount } from "svelte"
+	import { onMount } from "svelte/internal"

 RERUN  src/lib/Hello.svelte x1

 ✓ src/lib/__test__/Hello.test.ts (1)

 Test Files  1 passed (1)

Maybe related to https://github.com/sveltejs/svelte/issues/5534? I'm not sure why this passes when vite version is lowered though.

[sheremet-va]

Test passes with following:

<script lang="ts">

-	import { onMount } from "svelte"

+	import { onMount } from "svelte/internal"

 RERUN  src/lib/Hello.svelte x1



 ✓ src/lib/__test__/Hello.test.ts (1)



 Test Files  1 passed (1)

Maybe related to https://github.com/sveltejs/svelte/issues/5534?

I'm not sure why this passes when vite version is lowered though.

Vitest should still work with import from svelte here. We need to figure out what breaks module resolution here

[yuliankarapetkov]

@sheremet-va, nice catch! How did you find that?

I can confirm that this was the cause of my bug since after changing all onMount and onDestroy imports in the relevant files, all 60 tests that were failing succeed now.

[AriPerkkio]

Started to rollback local vite commit by commit. Test works after reverting https://github.com/vitejs/vite/pull/11595 (:wave: @bluwy).

It seems that entrypoint for Svelte is resolved to svelte/ssr.mjs instead of svelte/index.mjs. In [email protected] the decision was made based on package.json's module field but in [email protected] it's done based on exports.

[email protected]:
/Users/x/y/vitest-example-project/node_modules/.pnpm/[email protected]/node_modules/svelte/index.mjs

[email protected]:
/Users/x/y/vitest-example-project/node_modules/.pnpm/[email protected]/node_modules/svelte/ssr.mjs

Parameters used here below: https://github.com/vitejs/vite/blob/d953536aae448e2bea0f3a7cb3c0062b16d45597/packages/vite/src/node/plugins/resolve.ts#L1108-L1112

{
  key: '.',
  browser: false,
  require: false,
  conditions: [ 'development', 'module', 'svelte', 'node' ]
}
-> Result = './ssr.mjs'

Svelte's entrypoints: https://github.com/sveltejs/svelte/blob/82d2982845df188631993db6b18c2842e3613acf/package.json#L23-L87

It does make sense that ssr.mjs is imported in Node. I guess the onMount intentionally does not run on SSR? Not sure what should be done here. Maybe vite-plugin-svelte should detect test environment and flip the import? Or if Svelte intentionally does not run onMount on SSR, maybe it should do it when it's run in tests. 😕

Another work-around for projects for now so that test code does not need changes:

import { defineConfig } from "vitest/config";

export default defineConfig({
  test: {
    alias: [
      {
        find: /svelte\/ssr.mjs/,
        replacement: "svelte/index.mjs",
      },
    ],

[sheremet-va]

It seems that entrypoint for Svelte is resolved to svelte/ssr.mjs instead of svelte/index.mjs. In [email protected] the decision was made based on package.json's module field but in [email protected] it's done based on exports.

I think Svelte team wants Vitest to use browser condition (resolve.conditions in config) here. The problem is that other packages will not work with it, if we enable it by default (one of the most popular things to do with jest and jsdom right now is adding node condition overriding default browser one, because package authors don’t expect this code to run inside a node with emulated browser environment).

[TorstenDittmann]

Just wanna leave a hack here that worked for us, without having to change all the imports.

Create or add this to your test setup file

import * as svelteinternal from 'svelte/internal';
import { vi } from 'vitest';

beforeAll(() => {
    vi.mock('svelte', () => svelteinternal);
});

[AriPerkkio]

I think Svelte team wants Vitest to use browser condition (resolve.conditions in config) here.

Using resolve.conditions seems to work. Maybe Vitest should expose conditions in configuration API, so that setting that one would not affect production builds? Similar as test.alias is done.

The problem is that other packages will not work with it, if we enable it by default

I guess the conditions cannot be set for specific packages only. One option would be to enable browser condition when Vitest detects test environment to be jsdom or happydom. That would at least limit the damage.

[bluwy]

Yeah the Vite PR change is intentional as it was previously incorrect (though many tooling relied on it). If setting browser condition doesn't cause any other issues, I think it's the best way as long as you have jsdom/happydom to shim the environment.

(Though usually I prefer testing UI components with a real/headless browser)

[sheremet-va]

For now I recommend using a workaround with conditions.

This is much deeper problem than Svelte issue. Vitest processes svelte/vue/... files with web mode, and processes it with SSR transforms afterwards. But typescript and javascript files are processed with SSR mode enabled (plugins receive ssr flag).

This creates an issue, if code is imported from the same package in two different modes, - especially if code depends on a singleton. Currently to bypass this Vitest hardcodes node condition, but this is not reliable of course (as we can see here Svelte expects Svelte to always run in SSR mode, if it's running inside node, which is not true for Vitest - it's always running in Node, not in browser).

One of the reason why Vitest doesn't process everything with web mode is performance - Vite is (at least, was) really slow to transform a single ts/js file. The other issue is that library authors also expect browser condition to be run in a browser, so they don't follow node ESM spec (file in browser condition doesn't have correct extension).

[jgarplind]

I'm facing a similar issue and would be interested to know if the mentioned conditions workaround could help.

Would you mind expanding a little bit on what should be done to implement said workaround?

[sheremet-va]

Would you mind expanding a little bit on what should be done to implement said workaround?

Add "browser" to your resolve.conditions property in the config file.

[vekunz]

If I set resolve.conditions to browser in vite.config.js

resolve: {
  conditions: [
    ...process.env.VITEST ? ['browser'] : []
  ]
}

I get another error multiple times, when running the unit tests

Error: require() of ES Module C:\...\node_modules\jose\dist\browser\index.js from C:\...\node_modules\openid-clien
t\lib\client.js not supported.
Instead change the require of index.js in C:\...\node_modules\openid-client\lib\client.js to a dynamic import() which is available in all CommonJS module
s.

[sheremet-va]

If I set resolve.conditions to browser in vite.config.js

Yes, conditions work for both require and import calls. So, if we define the custom condition, it should always be in the same format (or just cjs, since import can handle it) as the file that imports it. @dominikg, you might want to keep this in mind 👀 It is also possible to define it with two types:

{
  "browser": {
    "import": "./path.mjs"
    "require": "./path.cjs"
  }
}

But this doesn't really make sense for library authors, because "browser" condition should be executed in the browser, and it doesn't support "require". This is a very complicated issue, and I hope we can discuss it with the Vite team when we start moving vite-node into Vite core.

Right now you can use hardcoded aliases for this instead of browser condition (but some svelte libraries might not work!):

export default defineConfig({
  test: {
    alias: {
      svelte: "svelte/internal"
    }
  }
})

As you can see, the state of ESM/CJS is a mess 😄

[dominikg]

setting the browser condition via config directly can be a problem too, as vite resolve.conditions is treated a bit differently than other values by vite config merging.

To make sure the browser condition is added in the front and you don't modify conditions when vitest is not active, use a plugin with config hook:

// ...
const vitestBrowserConditionPlugin = {
  name: 'vite-plugin-vitest-browser-condition',
  config({resolve}) {
    if(process.env.VITEST) {
      resolve.conditions.unshift('browser'); 
    }
  }
}
export default defineConfig({
  // ...
  plugins: [vitestBrowserConditionPlugin,svelte()]
})

[dominikg]

Unfortunately that won't help with @vekunz case, openid-client depends on jose and jose package.json has an interesting exports map. for deno and bun they expose the same export as for browser, but for generic import/require, they export a node specific file.

so with adding the browser condition you're changing what gets imported from jose and that doesn't work inside node. oops.

[sheremet-va]

for deno and bun they expose the same export as for browser, but for generic import/require, they export a node specific file.

I think they have somewhat correct exports map. They don't expect their code to be imported with require and "browser" condition at the same time, which is a fair assumption to make.

I think the problem comes from what developers consider a "browser" condition. Should it be used, when "browser-like" environment is initiated (code has access to global "window" and "document" for example), or when the environment itself is a browser (meaning, the code will be processed by build tools)? I think most people consider it to be the second one, and that's why I am hesitant to add this condition in Vitest by default. For me, the biggest issue is that require will choose this export and fail (what happened in @vekunz and will happen to many other people).

[dominikg]

Agree that what jose does works in regular environments and usecases, still interesting that bun/deno get the browser file via their own conditions whereas node gets a node file via "import"/"require".

The trick with alias above is a more targeted way to ensure svelte is resolved to what it exports for browsers, so in situations where there are competing needs it can be a workaround, though this might become a problem if svelte decided to change its exports structure in a future major release (you'll know from the release notes and your tests crashing so not too bad)

[vekunz]

Right now you can use hardcoded aliases for this instead of browser condition (but some svelte libraries might not work!):

export default defineConfig({
  test: {
    alias: {
      svelte: "svelte/internal"
    }
  }
})

Unfortunately, this does not work for me. If I set this configuration, a get a new error:

Error: Missing "./internal/internal" specifier in "svelte" package

[bluwy]

I guess it needs to be:

export default defineConfig({
  test: {
    alias: [
      { find: /^svelte$/, replacement: "svelte/internal" }
    ]
  }
})

to be more accurate.

[josdejong]

Any news on this issue?

[sheremet-va]

This should be fixed in the latest version. If you still have problems, considered enabling deps.experimentalOptimizer.

[Iuriy-Budnikov]

@sheremet-va it still has an issue. experimentalOptimizer doesn't help

[Iuriy-Budnikov]

 alias: [
      { find: /^svelte$/, replacement: "svelte/internal" }
    ]

It works. Thank you!

[josdejong]

The issue is indeed still there. The workaround works for me.

Not sure if this is helpful, but to reproduce the issue with the project that I'm working o:

1. Clone the project

   $ git clone https://github.com/josdejong/svelte-jsoneditor.git
   $ cd svelte-jsoneditor
   $ npm install
   

2. Open the file vitest.config.js, and remove the line:

   alias: [{ find: /^svelte$/, replacement: 'svelte/internal' }],
   

3. Run the tests:

   $ npm test
   

   This throws an error:

   FAIL  src/lib/components/JSONEditor.test.ts > JSONEditor > render text mode
   TestingLibraryElementError: Unable to find an element with the text: "Joe".
   

So, for some reason, the Svelte component with these contents is not rendered during the test. All tests pass with the alias workaround (and with Vite 4.0.x).

[james-camilleri]

Just chipping in that this issue is still present as of vitest 0.34.6.

[sibiraj-s]

Another option would be to add browser to the resolve conditions

import { sveltekit } from '@sveltejs/kit/vite';
import { defineConfig } from 'vitest/config';

export default defineConfig({
  plugins: [sveltekit()],
  // see https://github.com/testing-library/svelte-testing-library/issues/222
  resolve: {
    ...(process.env.VITEST
      ? {
          conditions: ['browser', 'import', 'module', 'default']
        }
      : null),
  },
});

So far, in my project with a simple setup, it works regardless of the conditions order, as long as browser is added to the list.

[josdejong]

@sibiraj-s thanks for sharing. I tried your workaround but that doesn't work in my case (the alias workaround works fine though).

[sibiraj-s]

Oh, Not sure about the reason, but here is the link for the code that I used. https://github.com/sibiraj-s/svelte-tiptap/blob/3cc167aa67aeb3af1bcd74bbaa68d6011b1d6324/vite.config.ts#L8