fix: call `resolveId('vitest')` after `buildStart` by hi-ogawa · Pull Request #5646 · vitest-dev/vitest

hi-ogawa · 2024-05-01T00:53:41Z

Description

closes resolveId is called before buildStart with enforce: 'pre' #5616

ctx.distPath is only used by pool runTests, so we can move resolveId from ctx.setServer to ctx.start.

todo

test with vscode extension
- verified locally with pnpm.overrides

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:.

…itialization

netlify · 2024-05-01T00:54:01Z

✅ Deploy Preview for fastidious-cascaron-4ded94 ready!

Name	Link
🔨 Latest commit	`74a9636`
🔍 Latest deploy log	https://app.netlify.com/sites/fastidious-cascaron-4ded94/deploys/6632d17a16139c000844d7fe
😎 Deploy Preview	https://deploy-preview-5646--fastidious-cascaron-4ded94.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

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

sheremet-va · 2024-05-01T08:34:39Z

packages/vitest/src/node/core.ts


+    // if Vitest is running globally, then we should still import local vitest if possible
+    const projectVitestPath = await this.vitenode.resolveId('vitest')
+    const vitestDir = projectVitestPath ? resolve(projectVitestPath.id, '../..') : rootDir


vs code extension doesn't call start at all

Oh, really...
I had createRequire approach before 1037511. Would this work better?

Or we could delay it further to ctx.runFiles. Let me think...

The createRequire approach is wrong because we need Vite to process it. Having it in runFiles is fine. We can have a guard to not check this path if it is resolved, and reset it in configureServer

Do we need Vite's resolution? If the purpose is only to find local Vitest, would createRequire with require.resolve('vitest', { paths: [config.root] }) do the same?

This was the approach used for package installer:

vitest/packages/vitest/src/node/packageInstaller.ts

Lines 15 to 18 in 04df286

if (process.versions.pnp) {

const targetRequire = createRequire(__dirname)

try {

targetRequire.resolve(dependency, { paths: [root, __dirname] })

We need Vite's resolution because files importing vitest are processed by Vite. It's possible that Vitest is called globally and resolved id is not correct, or there is an alias or resolveId interception

test/cli/fixtures/plugin/vitest.config.ts

fix: use resolve.resolve('vitest') instead of resolveId during in…

606fe8c

…itialization

hi-ogawa added 2 commits May 1, 2024 10:35

test: add test

0db4486

wip: just delay til Vitest.start?

1037511

hi-ogawa changed the title ~~fix: use require.resolve('vitest') instead of resolveId during init~~ fix: call resolveId('vitest') after buildStart May 1, 2024

hi-ogawa marked this pull request as ready for review May 1, 2024 01:51

sheremet-va requested changes May 1, 2024

View reviewed changes

hi-ogawa added 5 commits May 1, 2024 18:52

fix: delay initializeDistPath + test createVitest

04df286

test: simplify

997566a

refactor: minor

32ded40

fix: reset distPath

4296dea

Merge branch 'main' into fix-avoid-early-resolveId

74a9636

sheremet-va approved these changes May 2, 2024

View reviewed changes

sheremet-va merged commit f5faf42 into vitest-dev:main May 2, 2024

hi-ogawa mentioned this pull request May 2, 2024

chore: add setup-overrides script #5650

Closed

6 tasks

hi-ogawa deleted the fix-avoid-early-resolveId branch May 2, 2024 09:30

renovate bot referenced this pull request in slipmatio/toolbelt May 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Comments

fix: call `resolveId('vitest')` after `buildStart`#5646

fix: call `resolveId('vitest')` after `buildStart`#5646
sheremet-va merged 8 commits intovitest-dev:mainfrom
hi-ogawa:fix-avoid-early-resolveId

hi-ogawa commented May 1, 2024 •

edited

Loading

Uh oh!

netlify bot commented May 1, 2024 •

edited

Loading

Uh oh!

sheremet-va May 1, 2024

Uh oh!

hi-ogawa May 1, 2024

Uh oh!

hi-ogawa May 1, 2024

Uh oh!

sheremet-va May 1, 2024

Uh oh!

hi-ogawa May 1, 2024

Uh oh!

sheremet-va May 1, 2024

Uh oh!

Uh oh!

	if (process.versions.pnp) {
	const targetRequire = createRequire(__dirname)
	try {
	targetRequire.resolve(dependency, { paths: [root, __dirname] })

Uh oh!

Comments

Conversation

hi-ogawa commented May 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

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

Tests

Documentation

Changesets

Uh oh!

netlify bot commented May 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for fastidious-cascaron-4ded94 ready!

Uh oh!

sheremet-va May 1, 2024

Choose a reason for hiding this comment

Uh oh!

hi-ogawa May 1, 2024

Choose a reason for hiding this comment

Uh oh!

hi-ogawa May 1, 2024

Choose a reason for hiding this comment

Uh oh!

sheremet-va May 1, 2024

Choose a reason for hiding this comment

Uh oh!

hi-ogawa May 1, 2024

Choose a reason for hiding this comment

Uh oh!

sheremet-va May 1, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hi-ogawa commented May 1, 2024 •

edited

Loading

netlify bot commented May 1, 2024 •

edited

Loading