Conditional exports can't be active when key is browser

[NWYLZW]

Describe the bug

When I use 'browser' as an optional condition for exporting, I cannot correctly import the module.

Reproduction

https://github.com/NWYLZW/issue-vite-conditional-exports

Steps to reproduce

Run pnpm install followed by pnpm run test

System Info

npx envinfo --system --npmPackages '{vite,@vitejs/*,rollup}' --binaries --browsers

  System:
    OS: macOS 14.1.1
    CPU: (10) arm64 Apple M1 Pro
    Memory: 154.44 MB / 32.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 18.20.4 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 10.7.0 - /usr/local/bin/npm
    pnpm: 8.12.1 - ~/Library/pnpm/pnpm
  Browsers:
    Safari: 17.1

Used Package Manager

pnpm

Logs

Click to expand!

~/codes/projects/issue-vite-conditional-exports git:[master]
pnpm run test

> issue-vite-conditional-exports@1.0.0 test /Users/yijie/codes/projects/issue-vite-conditional-exports
> vitest


The CJS build of Vite's Node API is deprecated. See https://vitejs.dev/guide/troubleshooting.html#vite-cjs-node-api-deprecated for more details.
[ 'browser' ]

 DEV  v2.0.5 /Users/yijie/codes/projects/issue-vite-conditional-exports

 ❯ tests/index.spec.ts (1)
   ❯ foo (1)
     × should be true

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Failed Tests 1 ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯

 FAIL  tests/index.spec.ts > foo > should be true
AssertionError: expected 'foo.node' to be 'foo.browser' // Object.is equality

Expected: "foo.browser"
Received: "foo.node"

 ❯ tests/index.spec.ts:7:15
      5|   it('should be true', () => {
      6|     const f = foo()
      7|     expect(f).toBe('foo.browser')
       |               ^
      8|     expectTypeOf(f).toEqualTypeOf<'foo.browser'>()
      9|     expectTypeOf(f).not.toEqualTypeOf<'foo.node'>()

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/1]⎯

 Test Files  1 failed (1)
      Tests  1 failed (1)
   Start at  18:23:27
   Duration  175ms (transform 20ms, setup 0ms, collect 14ms, tests 5ms, environment 0ms, prepare 38ms)


 FAIL  Tests failed. Watching for file changes...

Validations

• Follow our Code of Conduct
• Read the Contributing Guidelines.
• Read the docs.
• Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• Make sure this is a Vite issue and not a framework-specific issue. For example, if it's a Vue SFC related bug, it should likely be reported to vuejs/core instead.
• Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
• The provided reproduction is a minimal reproducible example of the bug.

[NWYLZW]

The example for reproduction uses Vitest, but in reality, it is not closely related to Vitest; the main issue lies with vite-node.

vite/packages/vite/src/node/plugins/resolve.ts

Line 177 in 5679d81

const targetWeb = !ssr || ssrTarget === 'webworker'

vite/packages/vite/src/node/plugins/resolve.ts

Line 1113 in 5679d81

browser: targetWeb && !additionalConditions.has('node'),

Based on my debugging results, the handling here seems a bit too tightly coupled with SSR. For other users encountering this issue, you can try setting the ssrTarget parameter to 'webworker'. However, I believe this area should take into account that the user has already actively declared the conditional.

[hi-ogawa]

While I agree Vitest's resolution is confusing at times (other issue I remember is module condition cannot be disabled on Vitest #16631), is it possible that the exports entries should have browser before node for this specific case? something like:

  "exports": {
    "./foo": {
      "browser": "./src/foo.browser.ts",
      "node": "./src/foo.node.ts"
    }
  },

With this, resolve.exports picks up browser instead of node since internally they set up conditions = ["default", "browser", "import", "node"] and it can match browser entry first https://github.com/lukeed/resolve.exports/blob/294192309b7276bc0913f65a7ee73721611ae383/src/utils.ts#L15-L20

Vitest/Vite automatically adding node condition doesn't seem entirely wrong since it's guarded for ssr usage as you pointed out. The behavior is probably similar to how node --conditions browser -e 'import("issue-vite-conditional-exports/foo")' would resolve exports.

[NWYLZW]

With this, resolve.exports picks up browser instead of node since internally they set up conditions = ["default", "browser", "import", "node"] and it can match browser entry first https://github.com/lukeed/resolve.exports/blob/294192309b7276bc0913f65a7ee73721611ae383/src/utils.ts#L15-L20

I also noticed this point, and it was precisely because of this that I made some modifications to the corresponding browser condition and initiated the related PR.

However, based on the latest code, it seems that the issue here has been exacerbated, as users need a configuration that is used in more places to disable this logic.