search

vitest-dev / vitest

Next generation testing framework powered by Vite.
https://vitest.dev
MIT License
13.08k stars 1.17k forks source link

Error in Browser mode x Node APIs #6293

Closed Jinjiang closed 1 week ago

Jinjiang commented 3 months ago

Describe the bug

It got error when run Vitest via Node APIs with config like this:

const viteConfig = {
  configFile: false,
  envFile: false,
  plugins: [vue()],
  test: {
    root,
    watch: false,
    browser: {
      enabled: true,
      provider: 'playwright',
      name: 'chromium',
      headless: true,
    }
  }
};

startVitest('test', undefined, undefined, viteConfig);

Error log:

11:58:25 AM [vite] Pre-transform error: Failed to parse source for import analysis because the content contains invalid JS syntax. Install @vitejs/plugin-vue to handle .vue files.
11:58:25 AM [vite] Internal server error: Failed to parse source for import analysis because the content contains invalid JS syntax. Install @vitejs/plugin-vue to handle .vue files.
  Plugin: vite:import-analysis
  File: /xxx/reproductions/App.vue:2:18
  1  |  <template>
  2  |    <h1>My App</h1>
     |                   ^
  3  |  </template>
      at TransformPluginContext._formatError (file:///xxx/reproductions/node_modules/.pnpm/vite@5.3.5_@types+node@22.1.0/node_modules/vite/dist/node/chunks/dep-mCdpKltl.js:48945:41)
      at TransformPluginContext.error (file:///xxx/reproductions/node_modules/.pnpm/vite@5.3.5_@types+node@22.1.0/node_modules/vite/dist/node/chunks/dep-mCdpKltl.js:48940:16)
      at TransformPluginContext.transform (file:///xxx/reproductions/node_modules/.pnpm/vite@5.3.5_@types+node@22.1.0/node_modules/vite/dist/node/chunks/dep-mCdpKltl.js:63667:14)
      at async PluginContainer.transform (file:///xxx/reproductions/node_modules/.pnpm/vite@5.3.5_@types+node@22.1.0/node_modules/vite/dist/node/chunks/dep-mCdpKltl.js:48786:18)
      at async loadAndTransform (file:///xxx/reproductions/node_modules/.pnpm/vite@5.3.5_@types+node@22.1.0/node_modules/vite/dist/node/chunks/dep-mCdpKltl.js:51608:27)
      at async viteTransformMiddleware (file:///xxx/reproductions/node_modules/.pnpm/vite@5.3.5_@types+node@22.1.0/node_modules/vite/dist/node/chunks/dep-mCdpKltl.js:61557:24)
 ❯ App.vue.spec.ts (0)

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Failed Suites 1 ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯

 FAIL  App.vue.spec.ts [ App.vue.spec.ts ]
TypeError: Failed to fetch dynamically imported module: http://localhost:5173/xxx/reproductions/App.vue.spec.ts?import&browserv=1723003105187
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/1]⎯

 Test Files  1 failed (1)
      Tests  no tests
   Start at  11:58:24
   Duration  1.31s (transform 0ms, setup 0ms, collect 0ms, tests 0ms, environment 0ms, prepare 268ms)

 ELIFECYCLE  Command failed with exit code 1.

Reproduction

https://github.com/Jinjiang/reproductions/tree/try-vitest-browser-mode-20240807

pnpm install

# it works
pnpm test
# it doesn't work
pnpm test:debug

System Info

System:
    OS: macOS 15.0
    CPU: (8) arm64 Apple M1
    Memory: 53.38 MB / 8.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.11.1 - ~/.local/share/mise/installs/node/20/bin/node
    Yarn: 1.22.18 - ~/.yarn/bin/yarn
    npm: 10.2.4 - ~/.local/share/mise/installs/node/20/bin/npm
    pnpm: 9.6.0 - ~/Library/pnpm/pnpm
    bun: 1.0.29 - ~/.local/share/mise/installs/bun/latest/bin/bun
  Browsers:
    Safari: 18.0
  npmPackages:
    @vitejs/plugin-vue: ^5.1.2 => 5.1.2 
    @vitest/browser: ^2.0.5 => 2.0.5 
    vite: ^5.3.5 => 5.3.5 
    vitest: ^2.0.5 => 2.0.5

Used Package Manager

pnpm

Validations

hi-ogawa commented 3 months ago

It looks like a duplicate of https://github.com/vitest-dev/vitest/issues/5015 Vitest uses a separate Vite server for browser mode (or any workspace project) while viteConfig passed to startVitest only affects a main vite server.

Currently it doesn't look possible to setup workspace project level config from Node API like startVitest, so you might need an actual config file to pass vue() to browser mode project.

hi-ogawa commented 3 months ago

Looking at the reproduction, I just noticed you have vite config already, so I'm wondering if there's any reason you're disabling configFile? I think you can comment out these and it would work more naturally as it loads project vite.config.ts:

const viteConfig = {
  // configFile: false,
  // envFile: false,
  // plugins: [vue()],
  test: {
    // root: resolve(__dirname, '../'),
    watch: false,
    browser: {
      enabled: true,
      provider: 'playwright',
      name: 'chromium',
      headless: true,
    }
  }
};
Jinjiang commented 3 months ago

@hi-ogawa thanks for the informaiton.

The background is I'm integrating vitest into @teambit in which there is no vite config file to resolve. The vite.config.ts in this reproduction is just for comparison between CLI command and Node API calls.

It would be great if we can support it for browser server in any way.

hi-ogawa commented 3 months ago

The background is I'm integrating vitest into @teambit in which there is no vite config file to resolve.

Can you elaborate more on this? For example, is it the assumption that users might not be even using Vite for their app/lib but want to use Vitest for testing?

Jinjiang commented 3 months ago

Sure.

In short, Bit is a component-based workspace. In Bit, every component is based on a certain dev env. And every dev env (such as React env, Vue env, Angular env, etc) consists of multiple separate dev services like compiler, linter, formatter, tester, docs preview. By design, the dev env author can pick up any tools from the open source community and integrate them into the env.

e.g. for this vue env: https://bit.cloud/bitdev/vue/envs/vue-modern-env it picks up Vite as the docs preview and Vitest as the tester. You can also replace Vite into webpack as the docs preview but still use Vitest as the tester if you want. So the answer to your question is yes.

All the dev services are called via Node APIs in Bit. And the file structure of a workspace is slightly different from a normal workspace. So for flexibility, we usually prefer Node API calls and inline configs rather than CLI command and config files. That also means there is no vite.config.ts in each component dir. They are all configured in the corresponding dev env via inline config.

If you are interested, this is the source code that we currently resolve config for Vitest dev service: https://bit.cloud/teambit/vite/vitest-tester/~code/utils.ts#l103 and we make the call here to run Vitest with the inline config: https://bit.cloud/teambit/vite/vitest-tester/~code/vitest-tester.ts#l98

Let me know if you need any other information.

hi-ogawa commented 3 months ago

e.g. for this vue env: https://bit.cloud/bitdev/vue/envs/vue-modern-env it picks up Vite as the docs preview and Vitest as the tester. You can also replace Vite into webpack as the docs preview but still use Vitest as the tester if you want. So the answer to your question is yes.

Thanks, this context is useful to know :+1:

For this use case, you'll need a way to pass plugins to browser project server via startVitest but currently that's not possible as in https://github.com/vitest-dev/vitest/issues/5015.

Not sure, but maybe we can support something like inline test.workspace config, which goes like:

// run.js
import { startVitest, defineWorkspace } from 'vitest/node';
import vue from '@vitejs/plugin-vue';

await startVitest('test', undefined, undefined, {
  test: {
    // not supported
    workspace: defineWorkspace([
      {
        plugins: [vue()],
        test: {
          browser: {
            enabled: true,
            provider: 'playwright',
            name: 'chromium',
          }
        }
      }
    ])
  }
});

Currently test.workspace only supports a file path https://vitest.dev/config/#workspace, so the configuration needs to be split like:

//// run.js
import { startVitest } from 'vitest/node';
import vue from '@vitejs/plugin-vue';

await startVitest('test', undefined, undefined, {
  test: {
    workspace: "workspace.js"
  }
});

//// workspace.js
import { defineWorkspace } from 'vitest/node';
import vue from '@vitejs/plugin-vue';

export default defineWorkspace([
  {
    plugins: [vue()],
    test: {
      browser: {
        enabled: true,
        provider: 'playwright',
        name: 'chromium',
      }
    }
  }
])

Or instead of going through workspace, we could simply allow sneaking in plugins from test.browser.plugins config. :thinking:

hi-ogawa commented 3 months ago

@Jinjiang One question. What is this vitest config appearing under vue-modern-env? https://bit.cloud/bitdev/vue/envs/vue-modern-env/~code/config/vitest.config.mjs Isn't this something vitest-tester can pick up automatically as vitest config?

hi-ogawa commented 3 months ago

I think the answer to that might be yes, but probably it gets merged as inline config instead of used as configFile option https://bit.cloud/teambit/vite/vitest-tester/~code/utils.ts#l160 I wonder what's the reason to keep configFile: false instead of passing configFile: configPath directly there.

Jinjiang commented 3 months ago

@hi-ogawa Thanks so much for the explanation. Passing the plugins through sounds good and intuitive to me. Looking forward to it.

For the vitest.config.mjs in the env, yes, in this case it does use a config file. However, that's just a way we provide to the env author to custom something easily in the last mile. And it eventually comes to inline config and will be called by startVitest() programatically. So I understand the issue is still the same.

The extra config file is not in the root of the component dir. I understand in such a case configFile: true still doesn't work, right? And the config file could be anywhere in the dev env or another dependency package. So to avoid unexpected effects we disabled configFile and envFile by default.

hi-ogawa commented 3 months ago

The extra config file is not in the root of the component dir. I understand in such a case configFile: true still doesn't work, right?

@Jinjiang I was thinking of doing configFile: configPath at L117 https://bit.cloud/teambit/vite/vitest-tester/~code/utils.ts#l117 and removing mergeConfig below since configPath: string seems always defined. Would it change something too much?

sheremet-va commented 3 months ago

Passing the plugins through sounds good and intuitive to me.

I just want to say that it seems very counterintuitive to me. The config file with browser.enabled: true is the config with plugins field already.

sheremet-va commented 3 months ago

But I think it would make sense if we applied viteOverrides to the browser project config if the workspace project reuses root config (meaning, there no custom workspaces)

hi-ogawa commented 3 months ago

But I think it would make sense if we applied viteOverrides to the browser project config if the workspace project reuses root config (meaning, there no custom workspaces)

That might be okay, but the concern is same as a previous issue since that would cause plugins instances to be passed to two vite servers and that can break plugin hook convention (like buildStart runs multiple times).

Jinjiang commented 3 months ago

The extra config file is not in the root of the component dir. I understand in such a case configFile: true still doesn't work, right?

@Jinjiang I was thinking of doing configFile: configPath at L117 bit.cloud/teambit/vite/vitest-tester/~code/utils.ts#l117 and removing mergeConfig below since configPath: string seems always defined. Would it change something too much?

Sure. I've tried it. I thought it was just an equivalent change (from load and merge to configFile) but it worked after that. I think we can take this for now. Thanks a lot.

Jinjiang commented 3 months ago

Passing the plugins through sounds good and intuitive to me.

I just want to say that it seems very counterintuitive to me. The config file with browser.enabled: true is the config with plugins field already.

I personally feel the most confusing part to me is, with the same config, a file works and an inline object doesn't. The other approaches are also good to me.

Anyway, good to know the complication behind it through this issue. From the surface I just saw a new test.browser field.

Thanks.

sheremet-va commented 1 week ago

We decided to close this since there doesn't seem to be any demand for this feature anymore. You can specify the config path to achieve the same.