From 8877e22a090c5a5ccfd9206122649181ba9c44f9 Mon Sep 17 00:00:00 2001 From: Vladimir Date: Wed, 17 Jan 2024 11:20:43 +0100 Subject: [PATCH] fix(vitest): use development/production conditions when resolving external modules (#4980) --- packages/vitest/src/node/pool.ts | 11 +++- pnpm-lock.yaml | 11 ++++ pnpm-workspace.yaml | 1 + test/config/fixtures/conditions-pkg/index.js | 1 + .../fixtures/conditions-pkg/package.json | 9 +++ .../fixtures/conditions-subpackage/custom.js | 1 + .../fixtures/conditions-subpackage/default.js | 1 + .../conditions-subpackage/development.js | 1 + .../conditions-subpackage/package.json | 13 ++++ .../conditions-subpackage/production.js | 1 + .../conditions-test/conditions.test.js | 6 ++ .../fixtures/conditions-test/vitest.config.ts | 3 + test/config/test/conditions-cli.test.ts | 62 +++++++++++++++++++ test/test-utils/index.ts | 5 +- 14 files changed, 123 insertions(+), 3 deletions(-) create mode 100644 test/config/fixtures/conditions-pkg/index.js create mode 100644 test/config/fixtures/conditions-pkg/package.json create mode 100644 test/config/fixtures/conditions-subpackage/custom.js create mode 100644 test/config/fixtures/conditions-subpackage/default.js create mode 100644 test/config/fixtures/conditions-subpackage/development.js create mode 100644 test/config/fixtures/conditions-subpackage/package.json create mode 100644 test/config/fixtures/conditions-subpackage/production.js create mode 100644 test/config/fixtures/conditions-test/conditions.test.js create mode 100644 test/config/fixtures/conditions-test/vitest.config.ts create mode 100644 test/config/test/conditions-cli.test.ts diff --git a/packages/vitest/src/node/pool.ts b/packages/vitest/src/node/pool.ts index 649f3bed31a6..ca9d3f9bc544 100644 --- a/packages/vitest/src/node/pool.ts +++ b/packages/vitest/src/node/pool.ts @@ -60,7 +60,16 @@ export function createPool(ctx: Vitest): ProcessPool { return getDefaultPoolName(project, file) } - const conditions = ctx.server.config.resolve.conditions?.flatMap(c => ['--conditions', c]) || [] + // in addition to resolve.conditions Vite also adds production/development, + // see: https://github.com/vitejs/vite/blob/af2aa09575229462635b7cbb6d248ca853057ba2/packages/vite/src/node/plugins/resolve.ts#L1056-L1080 + const potentialConditions = new Set(['production', 'development', ...ctx.server.config.resolve.conditions]) + const conditions = [...potentialConditions].filter((condition) => { + if (condition === 'production') + return ctx.server.config.isProduction + if (condition === 'development') + return !ctx.server.config.isProduction + return true + }).flatMap(c => ['--conditions', c]) // Instead of passing whole process.execArgv to the workers, pick allowed options. // Some options may crash worker, e.g. --prof, --title. nodejs/node#41103 diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 2ea530937f4f..42547a1957c1 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1567,6 +1567,12 @@ importers: specifier: workspace:* version: link:../../packages/vitest + test/config/fixtures/conditions-pkg: + dependencies: + subpackage: + specifier: file:../conditions-subpackage + version: file:test/config/fixtures/conditions-subpackage + test/core: devDependencies: '@vitest/expect': @@ -27741,6 +27747,11 @@ packages: resolution: {integrity: sha512-V50KMwwzqJV0NpZIZFwfOD5/lyny3WlSzRiXgA0G7VUnRlqttta1L6UQIHzd6EuBY/cHGfwTIck7w1yH6Q5zUw==} dev: true + file:test/config/fixtures/conditions-subpackage: + resolution: {directory: test/config/fixtures/conditions-subpackage, type: directory} + name: subpackage + dev: false + file:test/env-custom/vitest-environment-custom: resolution: {directory: test/env-custom/vitest-environment-custom, type: directory} name: vitest-environment-custom diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index 5492b888537a..70b0a9eecd11 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -3,3 +3,4 @@ packages: - packages/* - examples/* - test/* + - test/config/fixtures/conditions-pkg diff --git a/test/config/fixtures/conditions-pkg/index.js b/test/config/fixtures/conditions-pkg/index.js new file mode 100644 index 000000000000..3ed313256b6e --- /dev/null +++ b/test/config/fixtures/conditions-pkg/index.js @@ -0,0 +1 @@ +export { default as condition } from 'subpackage' diff --git a/test/config/fixtures/conditions-pkg/package.json b/test/config/fixtures/conditions-pkg/package.json new file mode 100644 index 000000000000..9190ef45d01a --- /dev/null +++ b/test/config/fixtures/conditions-pkg/package.json @@ -0,0 +1,9 @@ +{ + "name": "conditions", + "private": true, + "type": "module", + "main": "index.js", + "dependencies": { + "subpackage": "file:../conditions-subpackage" + } +} \ No newline at end of file diff --git a/test/config/fixtures/conditions-subpackage/custom.js b/test/config/fixtures/conditions-subpackage/custom.js new file mode 100644 index 000000000000..4fd6a6052a87 --- /dev/null +++ b/test/config/fixtures/conditions-subpackage/custom.js @@ -0,0 +1 @@ +export default 'custom' diff --git a/test/config/fixtures/conditions-subpackage/default.js b/test/config/fixtures/conditions-subpackage/default.js new file mode 100644 index 000000000000..32d12c6a5815 --- /dev/null +++ b/test/config/fixtures/conditions-subpackage/default.js @@ -0,0 +1 @@ +throw new Error('Should not be imported') diff --git a/test/config/fixtures/conditions-subpackage/development.js b/test/config/fixtures/conditions-subpackage/development.js new file mode 100644 index 000000000000..e47b8144bfec --- /dev/null +++ b/test/config/fixtures/conditions-subpackage/development.js @@ -0,0 +1 @@ +export default 'development' diff --git a/test/config/fixtures/conditions-subpackage/package.json b/test/config/fixtures/conditions-subpackage/package.json new file mode 100644 index 000000000000..4fbbddc56b70 --- /dev/null +++ b/test/config/fixtures/conditions-subpackage/package.json @@ -0,0 +1,13 @@ +{ + "name": "subpackage", + "private": true, + "type": "module", + "exports": { + ".": { + "custom": "./custom.js", + "development": "./development.js", + "production": "./production.js", + "default": "./default.js" + } + } +} \ No newline at end of file diff --git a/test/config/fixtures/conditions-subpackage/production.js b/test/config/fixtures/conditions-subpackage/production.js new file mode 100644 index 000000000000..207e27b62518 --- /dev/null +++ b/test/config/fixtures/conditions-subpackage/production.js @@ -0,0 +1 @@ +export default 'production' diff --git a/test/config/fixtures/conditions-test/conditions.test.js b/test/config/fixtures/conditions-test/conditions.test.js new file mode 100644 index 000000000000..447a8696379f --- /dev/null +++ b/test/config/fixtures/conditions-test/conditions.test.js @@ -0,0 +1,6 @@ +import { test, expect } from 'vitest'; +import { condition } from '../conditions-pkg'; + +test('condition is correct', () => { + expect(condition).toBe(TEST_CONDITION) +}) diff --git a/test/config/fixtures/conditions-test/vitest.config.ts b/test/config/fixtures/conditions-test/vitest.config.ts new file mode 100644 index 000000000000..abed6b2116e1 --- /dev/null +++ b/test/config/fixtures/conditions-test/vitest.config.ts @@ -0,0 +1,3 @@ +import { defineConfig } from 'vitest/config' + +export default defineConfig({}) diff --git a/test/config/test/conditions-cli.test.ts b/test/config/test/conditions-cli.test.ts new file mode 100644 index 000000000000..7f44397095fa --- /dev/null +++ b/test/config/test/conditions-cli.test.ts @@ -0,0 +1,62 @@ +import { expect, test } from 'vitest' +import { runVitest } from '../../test-utils' + +test('correctly imports external dependencies with a development condition', async () => { + // dev condition is the default + const { stderr } = await runVitest({ + root: 'fixtures/conditions-test', + server: { + deps: { + external: [/conditions-pkg/], + }, + }, + }, [], 'test', { + define: { + TEST_CONDITION: '"development"', + }, + }) + + expect(stderr).toBe('') +}) + +test('correctly imports external dependencies with a production condition', async () => { + // this is the only check in Vite for "isProduction" value + process.env.NODE_ENV = 'production' + + const { stderr } = await runVitest({ + root: 'fixtures/conditions-test', + server: { + deps: { + external: [/conditions-pkg/], + }, + }, + }, [], 'test', { + define: { + TEST_CONDITION: '"production"', + }, + }) + + expect(stderr).toBe('') +}) + +test('correctly imports external dependencies with a custom condition', async () => { + delete process.env.NODE_ENV + + const { stderr } = await runVitest({ + root: 'fixtures/conditions-test', + server: { + deps: { + external: [/conditions-pkg/], + }, + }, + }, [], 'test', { + resolve: { + conditions: ['custom'], + }, + define: { + TEST_CONDITION: '"custom"', + }, + }) + + expect(stderr).toBe('') +}) diff --git a/test/test-utils/index.ts b/test/test-utils/index.ts index 31d33e1a58a6..f0312d64b201 100644 --- a/test/test-utils/index.ts +++ b/test/test-utils/index.ts @@ -2,6 +2,7 @@ import { Console } from 'node:console' import { Writable } from 'node:stream' import fs from 'node:fs' import { fileURLToPath } from 'node:url' +import type { UserConfig as ViteUserConfig } from 'vite' import { type UserConfig, type VitestRunMode, afterEach } from 'vitest' import type { Vitest } from 'vitest/node' import { startVitest } from 'vitest/node' @@ -9,7 +10,7 @@ import { type Options, execa } from 'execa' import stripAnsi from 'strip-ansi' import { dirname, resolve } from 'pathe' -export async function runVitest(config: UserConfig, cliFilters: string[] = [], mode: VitestRunMode = 'test') { +export async function runVitest(config: UserConfig, cliFilters: string[] = [], mode: VitestRunMode = 'test', viteOverrides: ViteUserConfig = {}) { // Reset possible previous runs process.exitCode = 0 let exitCode = process.exitCode @@ -26,7 +27,7 @@ export async function runVitest(config: UserConfig, cliFilters: string[] = [], m watch: false, reporters: ['verbose'], ...config, - }) + }, viteOverrides) } catch (e: any) { return {