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

Don't respect conditions on Vite 6 #6992

Open
6 tasks done
sxzz opened this issue Nov 30, 2024 · 17 comments
Open
6 tasks done

Don't respect conditions on Vite 6 #6992

sxzz opened this issue Nov 30, 2024 · 17 comments
Labels
breaking change p3-significant High priority enhancement (priority)

Comments

@sxzz
Copy link
Contributor

sxzz commented Nov 30, 2024

Describe the bug

  • resolve.conditions is not respected on Vite 6 with Vitest.
  • It works on Vite 6 without Vitest.
  • It works on Vite 5.
// vitest.config.ts
import { defineConfig } from 'vitest/config'

export default defineConfig({
  resolve: {
    conditions: ['dev'],
  },
})

Reproduction

https://stackblitz.com/~/github.com/sxzz/vitest-conditions-repro

  • Run pnpm run test, and you will see the test has failed.
  • Downgrade to vite@5, remove node_modules and pnpm-lock.yaml, and it works.

System Info

N/A

Used Package Manager

pnpm

Validations

@colinlienard
Copy link

Also experiencing this issue, but I'm not using vite, it's the bump of vitest to v2.1.6 that is causing the issue for me, sticking to v2.1.5 for now

@sxzz
Copy link
Contributor Author

sxzz commented Nov 30, 2024

Vitest uses Vite at its core, and Vitest v2.1.5 uses Vite 5, so this issue can be avoided on v2.1.5.

@sxzz
Copy link
Contributor Author

sxzz commented Nov 30, 2024

Investigating: maybe related to Environment API

@sxzz
Copy link
Contributor Author

sxzz commented Nov 30, 2024

Workaround: Adding environments.ssr.resolve.conditions can fix this issue.

import { defineConfig } from 'vitest/config'

export default defineConfig({
  resolve: {
    conditions: ['dev'],
  },
  environments: {
    ssr: {
      resolve: {
        conditions: ['dev'],
      },
    },
  },
})

sxzz added a commit to vue-macros/vue-macros that referenced this issue Nov 30, 2024
sxzz added a commit to sxzz/vitest that referenced this issue Nov 30, 2024
@sheremet-va
Copy link
Member

sheremet-va commented Nov 30, 2024

This is expected behaviour in Vite 6: https://vite.dev/guide/migration.html#general-changes

The default values for those options are updated to the corresponding values and ssr.resolve.conditions no longer uses resolve.conditions as the default value.

Vitest uses ssr environment in node Vitest environment (previously, ssr field in the config), and client environment in jsdom or happy-dom (previously, the root config)

We can add this to documentation

@sxzz
Copy link
Contributor Author

sxzz commented Nov 30, 2024

IMHO, I don't think Vite 6's documentation is relevant to this issue. The breaking changes in Vite 6 only mention default values, but my reproduction uses a custom dev value instead, without default value.

I also tried setting ssr.resolve.conditions: ['dev'], but it doesn't work.

@sheremet-va
Copy link
Member

sheremet-va commented Nov 30, 2024

The breaking changes in Vite 6 only mention default values, but my reproduction uses a custom dev value instead, without default value.

Vite 6 doesn't inherit resolve.conditions in SSR. Vitest uses SSR environment for the default node environment (from the reproduction). The only bug here is that Vitest doesn't inherit ssr.resolve.conditions. It treats should treat resolve.conditions the same way Vite does.

@sheremet-va
Copy link
Member

You can override transformMode, by the way: https://vitest.dev/config/#testtransformmode (by default, it depends on the environment)

@sxzz
Copy link
Contributor Author

sxzz commented Nov 30, 2024

I see. Maybe Vitest should release a major version due to the breaking change in Vite that significantly affects Vitest.

@sheremet-va
Copy link
Member

I don't think this is part of Vitest API, this is how Vite works. If you don't want to use Vite 6, use Vite 5. We support both versions and just follow the configuration supported by the version of Vite that you are using.

@kolorfilm
Copy link

The breaking changes in Vite 6 only mention default values, but my reproduction uses a custom dev value instead, without default value.

Vite 6 doesn't inherit resolve.conditions in SSR. Vitest uses SSR environment for the default node environment (from the reproduction). The only bug here is that Vitest doesn't inherit ssr.resolve.conditions. It treats should treat resolve.conditions the same way Vite does.

I have the same problem. Can you maybe explain how this could look like in a workaround? Using vitest with happy dom.

Thank you!

@kolorfilm
Copy link

Somehow has the same issue.

Cherry added a commit to Cherry/workers-sdk that referenced this issue Dec 1, 2024
This works around vitest-dev/vitest#6992 being an issue with Vite 6
Cherry added a commit to Cherry/cloudflare-docs that referenced this issue Dec 1, 2024
This works around vitest-dev/vitest#6992 being an issue with Vite 6
@Cherry
Copy link

Cherry commented Dec 1, 2024

I think this needs to be considered a breaking change with vitest 2.1.5 -> 2.1.6. While I understand vitest supports 5.x and 6.x versions of vite now, in projects that don't use vite 6.x is now automatically used over 5.x in a patch version, which can result in drastically different behaviour.

For example, with Cloudflare's vitest-pool-workers which is used for testing Cloudflare Workers, they rely heavily on setting conditions: https://github.com/cloudflare/workers-sdk/blob/ff8f5ae03cd4a71bf7b468fcf3cd28bf064059ab/packages/vitest-pool-workers/src/config/index.ts#L110-L113

Most Cloudflare Workers don't use vite to build though, so don't have vite pinned to 5.x, resulting in breaking behaviour when upgrading from vitest 2.1.5 to 2.1.6, which just happened to me and led me down this rabbit hole.

@sheremet-va sheremet-va added p2-to-be-discussed Enhancement under consideration (priority) and removed pending triage labels Dec 1, 2024
@sheremet-va sheremet-va moved this to P2 - 5 in Team Board Dec 1, 2024
sheremet-va added a commit that referenced this issue Dec 2, 2024
@sheremet-va
Copy link
Member

sheremet-va commented Dec 2, 2024

https://github.com/vitest-dev/vitest/releases/tag/v2.1.7 reverted support for VIte 6 for now. We will discuss with the team on how to introduce it without breaking changes.

renovate bot added a commit to mmkal/eslint-plugin-mmkal that referenced this issue Dec 2, 2024
##### [v2.1.7](https://github.com/vitest-dev/vitest/releases/tag/v2.1.7)

#####    🐞 Bug Fixes

-   Revert support for Vite 6  -  by [@sheremet-va](https://github.com/sheremet-va) [<samp>(fbe5c)</samp>](vitest-dev/vitest@fbe5c39d)
    -   This introduced some breaking changes (vitest-dev/vitest#6992). We will enable support at a later time. In the meantime, you can still use `pnpm.overrides` or yarn resolutions to override the `vite` version in the `vitest` package - the APIs are compatible.

#####     [View changes on GitHub](vitest-dev/vitest@v2.1.6...v2.1.7)
penalosa pushed a commit to Cherry/workers-sdk that referenced this issue Dec 2, 2024
This works around vitest-dev/vitest#6992 being an issue with Vite 6
@sheremet-va
Copy link
Member

We will be releasing Vitest 3 in January with this change instead of Vitest 2.2. Both Vite 5 and Vite 6 will be supported, but the behaviour will depend on the version installed by your package manager.

@WtfJoke
Copy link

WtfJoke commented Dec 5, 2024

Does this also means vitest 3 (beside < vitest 2.17) will be the first version which supports vite 6 or will you also release a vitest 2.x version which supports vite 6?

@sheremet-va
Copy link
Member

sheremet-va commented Dec 5, 2024

Does this also means vitest 3 (beside < vitest 2.17) will be the first version which supports vite 6 or will you also release a vitest 2.x version which supports vite 6?

There will be no more Vitest 2 versions. Vitest 3 is the first version that supports Vite 6 (excluding 2.1.6)

@sheremet-va sheremet-va moved this from P2 - 5 to Approved in Team Board Dec 5, 2024
@sheremet-va sheremet-va added breaking change p3-significant High priority enhancement (priority) and removed p2-to-be-discussed Enhancement under consideration (priority) labels Dec 5, 2024
penalosa pushed a commit to Cherry/workers-sdk that referenced this issue Dec 5, 2024
This works around vitest-dev/vitest#6992 being an issue with Vite 6
penalosa pushed a commit to cloudflare/workers-sdk that referenced this issue Dec 5, 2024
* chore: bump vitest in templates

* chore: pin to vitest 2.1.5

This works around vitest-dev/vitest#6992 being an issue with Vite 6

* Create eighty-lies-sort.md

* chore: update to vitest 2.1.8

* Bump package vitest version to 2.1.8
penalosa added a commit to cloudflare/workers-sdk that referenced this issue Dec 6, 2024
commit 094699c6a218caa1c0ed5dcfb8f75598bc1f503c
Merge: 21bfe2a5b 5ea9d8e
Author: Samuel Macleod <smacleod@cloudflare.com>
Date:   Fri Dec 6 00:36:04 2024 +0000

    Merge remote-tracking branch 'origin/main' into penalosa/extension-scaffolding

commit 21bfe2a5b2f43c4d505cde795a2958964c4a8b0a
Author: Samuel Macleod <smacleod@cloudflare.com>
Date:   Fri Dec 6 00:36:00 2024 +0000

    main

commit 5ea9d8e
Author: emily-shen <69125074+emily-shen@users.noreply.github.com>
Date:   Thu Dec 5 19:01:47 2024 +0000

    remove stray await (#7453)

commit 9ede45b
Author: Pete Bacon Darwin <pete@bacondarwin.com>
Date:   Thu Dec 5 18:45:35 2024 +0000

    fix: relax validation of unsafe configuration to allow an empty object (#7461)

    The types, the default and the code in general support an empty object for this config setting.

    So it makes sense to avoid erroring when validating the config.

commit 55ec38a
Author: James Ross <james@jross.me>
Date:   Thu Dec 5 17:26:12 2024 +0000

    chore: bump vitest in templates (#7384)

    * chore: bump vitest in templates

    * chore: pin to vitest 2.1.5

    This works around vitest-dev/vitest#6992 being an issue with Vite 6

    * Create eighty-lies-sort.md

    * chore: update to vitest 2.1.8

    * Bump package vitest version to 2.1.8

commit f2045be
Author: Pete Bacon Darwin <pete@bacondarwin.com>
Date:   Thu Dec 5 17:06:31 2024 +0000

    refactor: Ensure that unstable type exports are all prefixed with `Unstable_` rather than just `Unstable` (#7454)

commit bc0a980
Author: Samuel Macleod <smacleod@cloudflare.com>
Date:   Thu Oct 3 18:10:07 2024 +0100

    fix buikd

commit 76afe01
Author: Samuel Macleod <smacleod@cloudflare.com>
Date:   Thu Oct 3 17:58:21 2024 +0100

    packaging & icon

commit cc00903
Author: Samuel Macleod <smacleod@cloudflare.com>
Date:   Thu Oct 3 17:30:21 2024 +0100

    support new workers

commit 472b15d
Author: Samuel Macleod <smacleod@cloudflare.com>
Date:   Thu Oct 3 17:27:18 2024 +0100

    lockfile

commit adb2f57
Author: Samuel Macleod <smacleod@cloudflare.com>
Date:   Mon Sep 23 13:45:06 2024 +0100

    WIP types

commit 72f2f99
Author: Samuel Macleod <smacleod@cloudflare.com>
Date:   Fri Sep 6 18:18:16 2024 +0100

    import -> require

commit 853e030
Author: Samuel Macleod <smacleod@cloudflare.com>
Date:   Fri Sep 6 18:04:30 2024 +0100

    better comment

commit 7dc9c53
Author: Samuel Macleod <smacleod@cloudflare.com>
Date:   Fri Sep 6 18:00:15 2024 +0100

    better comment

commit b8c8e32
Author: Samuel Macleod <smacleod@cloudflare.com>
Date:   Fri Sep 6 17:52:58 2024 +0100

    build wrangler

commit 6fd3a45
Author: Samuel Macleod <smacleod@cloudflare.com>
Date:   Fri Sep 6 17:35:42 2024 +0100

    vsce deps

commit 5e0df56
Author: Samuel Macleod <smacleod@cloudflare.com>
Date:   Fri Sep 6 17:12:11 2024 +0100

    fix types

commit 9e8960d
Author: Samuel Macleod <smacleod@cloudflare.com>
Date:   Mon Sep 2 12:03:39 2024 +0100

    sdk

commit 7ec1eed
Author: Samuel Macleod <smacleod@cloudflare.com>
Date:   Fri Aug 30 22:33:20 2024 +0100

    api

commit c6462ec
Author: Samuel Macleod <smacleod@cloudflare.com>
Date:   Fri Aug 9 20:51:13 2024 +0100

    fix build

commit 1bc79d4
Author: Samuel Macleod <smacleod@cloudflare.com>
Date:   Fri Aug 9 20:44:17 2024 +0100

    fix lint

commit f544b2a
Author: Samuel Macleod <smacleod@cloudflare.com>
Date:   Fri Aug 9 20:36:21 2024 +0100

    Resource provisioning

commit eb48c7c
Author: Samuel Macleod <smacleod@cloudflare.com>
Date:   Fri Aug 30 20:33:39 2024 +0100

    a bit more

commit d2eef9f
Author: Samuel Macleod <smacleod@cloudflare.com>
Date:   Thu Aug 29 19:00:55 2024 +0100

    add caching

commit 85aebb2
Author: Samuel Macleod <smacleod@cloudflare.com>
Date:   Thu Aug 29 18:59:38 2024 +0100

    path

commit fd031f6
Author: Samuel Macleod <smacleod@cloudflare.com>
Date:   Thu Aug 29 18:56:55 2024 +0100

    json

commit 5c2836e
Author: Samuel Macleod <smacleod@cloudflare.com>
Date:   Thu Aug 29 18:51:45 2024 +0100

    version number

commit 1999b27
Author: Samuel Macleod <smacleod@cloudflare.com>
Date:   Thu Aug 29 18:41:24 2024 +0100

    update lockfile

commit c67eba1
Author: Samuel Macleod <smacleod@cloudflare.com>
Date:   Thu Aug 29 18:31:43 2024 +0100

    first pass
GregBrimble added a commit to cloudflare/cloudflare-docs that referenced this issue Dec 11, 2024
* fix: update supported vitest version

* chore: pin vitest to 2.1.5

This works around vitest-dev/vitest#6992 being an issue with Vite 6

* chore: update vitest to 2.1.8

* chore: update vitest to 2.1.8

* Update src/content/docs/workers/testing/vitest-integration/get-started/write-your-first-test.mdx

---------

Co-authored-by: Greg Brimble <developer@gregbrimble.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change p3-significant High priority enhancement (priority)
Projects
Status: Approved
Development

Successfully merging a pull request may close this issue.

6 participants