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

fix: don't change the working directory when loading workspace projects #6811

Merged
merged 5 commits into from
Nov 5, 2024

Conversation

sheremet-va
Copy link
Member

@sheremet-va sheremet-va commented Oct 29, 2024

Description

Closes #6706
Fixes #6832

This behaviour is inconsistent with how the CWD works everywhere else (global setup files keep the original one; the plugin callbacks will also keep the original CWD). The team decided to revert it.

We might explore loading projects in a separate process with a different CWD in the future.

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time. If the feature is substantial or introduces breaking changes without a discussion, PR might be closed.
  • Ideally, include a test that fails without this PR but passes with it.
  • Please, don't make changes to pnpm-lock.yaml unless you introduce a new test example.

Tests

  • Run the tests with pnpm test:ci.

Documentation

  • If you introduce new functionality, document it. You can run documentation with pnpm run docs command.

Changesets

  • Changes in changelog are generated from PR name. Please, make sure that it explains your changes in an understandable manner. Please, prefix changeset messages with feat:, fix:, perf:, docs:, or chore:.

Copy link

netlify bot commented Oct 29, 2024

Deploy Preview for vitest-dev ready!

Name Link
🔨 Latest commit 9ba07d2
🔍 Latest deploy log https://app.netlify.com/sites/vitest-dev/deploys/6729e9c6cd7c760008e44869
😎 Deploy Preview https://deploy-preview-6811--vitest-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@unional
Copy link

unional commented Nov 5, 2024

I think setting the cwd to the project folder is the right approach. vite plugins probable expect cwd to be that.

I just experienced something probably related to this.
The tailwindcss+postcss is not able to scan the content:

warn - The `content` option in your Tailwind CSS configuration is missing or empty.
warn - Configure your content sources or your generated CSS will be missing styles.
warn - https://tailwindcss.com/docs/content-configuration

⎯⎯⎯⎯ Unhandled Rejection ⎯⎯⎯⎯⎯
TypeError: Cannot read properties of undefined (reading 'imports')
 ❯ node_modules/.pnpm/vite@5.4.10_patch_hash=xgqt7nmj25gqyng6hwpkq4rvp4_@types+node@22.8.1/node_modules/vite/dist/node/chunks/dep-BWSbWtLw.js:50832:71
 ❯ process.processTicksAndRejections node:internal/process/task_queues:105:5

https://github.com/repobuddy/storybook-addon-vis/actions/runs/11680177019/job/32522639763

where the tailwind.config.js is:

/** @type {import('tailwindcss').Config} */
export default {
	content: ['./src/**/*.tsx'],
	darkMode: 'class',
	theme: {
		extend: {},
	},
	plugins: [],
}

It works when running vitest within the project (testcase/themes in this case),
but fail (no such message) at the root.

@sheremet-va
Copy link
Member Author

sheremet-va commented Nov 5, 2024

This happens because the cwd was changed during the config resolution and then reverted back because projects are initiated i. the same process.

Plugins may reference cwd in hooks and it will be different from the one that the plugin was initiated with.

To fix your issue, cwd needs to always be the same which this PR achieves.

@hi-ogawa
Copy link
Contributor

hi-ogawa commented Nov 5, 2024

Do we have root for browser server? It looks like we enforce it for workspace server, but it appears missing here.

const vite = await createViteServer({

const config: ViteInlineConfig = {
...options,
root,

@sheremet-va
Copy link
Member Author

sheremet-va commented Nov 5, 2024

Do we have root for browser server? It looks like we enforce it for workspace server, but it appears missing here.

I don't think it should be specified there because it will override user root, although it should always be the same as the project since the config is the same 🤔

Anyway, let's do this in a separate PR.

@sheremet-va sheremet-va merged commit f0aeaca into vitest-dev:main Nov 5, 2024
17 checks passed
@sheremet-va sheremet-va deleted the fix/remove-chdir branch November 5, 2024 11:53
renovate bot added a commit to mmkal/eslint-plugin-mmkal that referenced this pull request Nov 13, 2024
##### [v2.1.5](https://github.com/vitest-dev/vitest/releases/tag/v2.1.5)

#####    🚀 Features

-   **experimental**: Expose vite on the test project as a separate field  -  by [@sheremet-va](https://github.com/sheremet-va) [<samp>(bcc59)</samp>](vitest-dev/vitest@bcc59088)

#####    🐞 Bug Fixes

-   `dangerouslyIgnoreUnhandledErrors` without base reporter  -  by [@AriPerkkio](https://github.com/AriPerkkio) in vitest-dev/vitest#6808 [<samp>(0bf0a)</samp>](vitest-dev/vitest@0bf0ab00)
-   Capture `unhandledRejection` even when base reporter is not used  -  by [@AriPerkkio](https://github.com/AriPerkkio) in vitest-dev/vitest#6812 [<samp>(8878b)</samp>](vitest-dev/vitest@8878b04d)
-   Don't change the working directory when loading workspace projects  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6811 [<samp>(f0aea)</samp>](vitest-dev/vitest@f0aeaca8)
-   Remove `sequence.concurrent` from the `RuntimeConfig` type  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6880 [<samp>(6af73)</samp>](vitest-dev/vitest@6af73d93)
-   Stop the runner before restarting, restart on workspace config change  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6859 [<samp>(b01df)</samp>](vitest-dev/vitest@b01df47d)
-   Don't rerun on Esc or Ctrl-C during watch filter  -  by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6895 [<samp>(98f76)</samp>](vitest-dev/vitest@98f76ea7)
-   Print ssrTransform error  -  by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6885 [<samp>(4c96c)</samp>](vitest-dev/vitest@4c96cce7)
-   Throw an error and a warning if `.poll`, `.element`, `.rejects`/`.resolves`, and `locator.*` weren't awaited  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6877 [<samp>(93b67)</samp>](vitest-dev/vitest@93b67c24)
-   **browser**:
    -   Don't process the default css styles  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6861 [<samp>(0d67f)</samp>](vitest-dev/vitest@0d67f04b)
    -   Support non US key input  -  by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6873 [<samp>(5969d)</samp>](vitest-dev/vitest@5969d8da)
    -   Stop the browser rpc when the pool is closed  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6858 [<samp>(9a0c9)</samp>](vitest-dev/vitest@9a0c93d7)
    -   Init browsers eagerly when tests are running  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6876 [<samp>(417bd)</samp>](vitest-dev/vitest@417bdb42)
-   **coverage**:
    -   Report uncovered files when re-run by `enter` or `'a'`  -  by [@AriPerkkio](https://github.com/AriPerkkio) in vitest-dev/vitest#6848 [<samp>(487c8)</samp>](vitest-dev/vitest@487c80ae)
-   **ui**:
    -   Remove crossorigin attributes for same origin assets  -  by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6883 [<samp>(6e793)</samp>](vitest-dev/vitest@6e793c64)
-   **vite-node**:
    -   Improve esm check to decide external  -  by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6816 [<samp>(7e1fa)</samp>](vitest-dev/vitest@7e1faf3d)
    -   Top-level throw in module is not reported properly  -  by [@vanaigr](https://github.com/vanaigr) and [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6840 [<samp>(cf0cb)</samp>](vitest-dev/vitest@cf0cbf6a)
-   **vitest**:
    -   Use fetch() implementation from happy-dom  -  by [@capricorn86](https://github.com/capricorn86) in vitest-dev/vitest#6879 [<samp>(3c56f)</samp>](vitest-dev/vitest@3c56feb4)

#####    🏎 Performance

-   Limit workspace project concurrency to available cores  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6869 [<samp>(f4c04)</samp>](vitest-dev/vitest@f4c04ab6)

#####     [View changes on GitHub](vitest-dev/vitest@v2.1.4...v2.1.5)
renovate bot added a commit to mmkal/eslint-plugin-mmkal that referenced this pull request Nov 13, 2024
##### [v2.1.5](https://github.com/vitest-dev/vitest/releases/tag/v2.1.5)

#####    🚀 Features

-   **experimental**: Expose vite on the test project as a separate field  -  by [@sheremet-va](https://github.com/sheremet-va) [<samp>(bcc59)</samp>](vitest-dev/vitest@bcc59088)

#####    🐞 Bug Fixes

-   `dangerouslyIgnoreUnhandledErrors` without base reporter  -  by [@AriPerkkio](https://github.com/AriPerkkio) in vitest-dev/vitest#6808 [<samp>(0bf0a)</samp>](vitest-dev/vitest@0bf0ab00)
-   Capture `unhandledRejection` even when base reporter is not used  -  by [@AriPerkkio](https://github.com/AriPerkkio) in vitest-dev/vitest#6812 [<samp>(8878b)</samp>](vitest-dev/vitest@8878b04d)
-   Don't change the working directory when loading workspace projects  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6811 [<samp>(f0aea)</samp>](vitest-dev/vitest@f0aeaca8)
-   Remove `sequence.concurrent` from the `RuntimeConfig` type  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6880 [<samp>(6af73)</samp>](vitest-dev/vitest@6af73d93)
-   Stop the runner before restarting, restart on workspace config change  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6859 [<samp>(b01df)</samp>](vitest-dev/vitest@b01df47d)
-   Don't rerun on Esc or Ctrl-C during watch filter  -  by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6895 [<samp>(98f76)</samp>](vitest-dev/vitest@98f76ea7)
-   Print ssrTransform error  -  by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6885 [<samp>(4c96c)</samp>](vitest-dev/vitest@4c96cce7)
-   Throw an error and a warning if `.poll`, `.element`, `.rejects`/`.resolves`, and `locator.*` weren't awaited  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6877 [<samp>(93b67)</samp>](vitest-dev/vitest@93b67c24)
-   **browser**:
    -   Don't process the default css styles  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6861 [<samp>(0d67f)</samp>](vitest-dev/vitest@0d67f04b)
    -   Support non US key input  -  by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6873 [<samp>(5969d)</samp>](vitest-dev/vitest@5969d8da)
    -   Stop the browser rpc when the pool is closed  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6858 [<samp>(9a0c9)</samp>](vitest-dev/vitest@9a0c93d7)
    -   Init browsers eagerly when tests are running  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6876 [<samp>(417bd)</samp>](vitest-dev/vitest@417bdb42)
-   **coverage**:
    -   Report uncovered files when re-run by `enter` or `'a'`  -  by [@AriPerkkio](https://github.com/AriPerkkio) in vitest-dev/vitest#6848 [<samp>(487c8)</samp>](vitest-dev/vitest@487c80ae)
-   **ui**:
    -   Remove crossorigin attributes for same origin assets  -  by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6883 [<samp>(6e793)</samp>](vitest-dev/vitest@6e793c64)
-   **vite-node**:
    -   Improve esm check to decide external  -  by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6816 [<samp>(7e1fa)</samp>](vitest-dev/vitest@7e1faf3d)
    -   Top-level throw in module is not reported properly  -  by [@vanaigr](https://github.com/vanaigr) and [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6840 [<samp>(cf0cb)</samp>](vitest-dev/vitest@cf0cbf6a)
-   **vitest**:
    -   Use fetch() implementation from happy-dom  -  by [@capricorn86](https://github.com/capricorn86) in vitest-dev/vitest#6879 [<samp>(3c56f)</samp>](vitest-dev/vitest@3c56feb4)

#####    🏎 Performance

-   Limit workspace project concurrency to available cores  -  by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6869 [<samp>(f4c04)</samp>](vitest-dev/vitest@f4c04ab6)

#####     [View changes on GitHub](vitest-dev/vitest@v2.1.4...v2.1.5)
@unional
Copy link

unional commented Nov 22, 2024

I think 2.1.5 is indeed breaking. Now the storybook integration crashes:

> vitest


⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Startup Error ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
SB_CORE-SERVER_0006 (MainFileMissingError): No configuration files have been found in your configDir: /<repository-root>/.storybook.
Storybook needs a "main.js" file, please add it.

it should be looking at /<project-root>/.storybook

@sheremet-va
Copy link
Member Author

sheremet-va commented Nov 22, 2024

I think 2.1.5 is indeed breaking. Now the storybook integration crashes:

> vitest


⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Startup Error ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
SB_CORE-SERVER_0006 (MainFileMissingError): No configuration files have been found in your configDir: /<repository-root>/.storybook.
Storybook needs a "main.js" file, please add it.

it should be looking at /<project-root>/.storybook

This is intentional. process.cwd() always equals the working directory from where vitest was running even in different workspace projects. While it makes running vitest from different folders behave differently, it is more consistent during the run (cwd in config() hook is the same as cwd in transform() hook). To support different cwd, we need to spawn child_process for every project which is out of scope for now

@sheremet-va sheremet-va restored the fix/remove-chdir branch November 22, 2024 09:49
@sheremet-va sheremet-va deleted the fix/remove-chdir branch November 22, 2024 09:50
@ivancuric
Copy link

ivancuric commented Dec 6, 2024

This change breaks testing in my monorepo setup. Previously I could test via the workspace both in the monorepo root task (single process) and via the VScode plugin, now I can only run tests from each package directory in its own process.

Some of the packages have build artifacts which aren't importable and need to be moved to other packages' /public directories during the buildStart hooks. For that I require the cwd to work as I need to resolve the dependent packages.

For now I'll rely on turborepo's task spawning and skip the VScode extension.

@unional
Copy link

unional commented Dec 6, 2024

To me, this completely broke workspace support. I'm waiting for a working solution before I try the workspace feature again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants