Skip to content

fix(create-vite): fix installing dependencies#20826

Merged
sapphi-red merged 5 commits intovitejs:mainfrom
nicolo-ribaudo:patch-1
Sep 23, 2025
Merged

fix(create-vite): fix installing dependencies#20826
sapphi-red merged 5 commits intovitejs:mainfrom
nicolo-ribaudo:patch-1

Conversation

@nicolo-ribaudo
Copy link
Copy Markdown
Contributor

@nicolo-ribaudo nicolo-ribaudo commented Sep 23, 2025

Description

Fixes #20823, #20824

This PR does not add tests. There seems to be already tests for this (

test('accepts immediate flag', () => {
const { stdout } = run([projectName, '--template', 'vue', '--immediate'], {
cwd: __dirname,
})
expect(stdout).not.toContain('Install and start now?')
expect(stdout).toContain(`Scaffolding project in ${genPath}`)
expect(stdout).toContain('Installing dependencies')
})
test('accepts immediate flag and skips install prompt', () => {
const { stdout } = run([projectName, '--template', 'vue', '--no-immediate'], {
cwd: __dirname,
})
expect(stdout).not.toContain('Install and start now?')
expect(stdout).not.toContain('Installing dependencies')
expect(stdout).toContain(`Scaffolding project in ${genPath}`)
), and I do not understand why they were passing. It seems like often this type of changes is done without tests (e.g. #20821), so hopefully somebody else can add some separately in a follow up :)

@nicolo-ribaudo nicolo-ribaudo changed the title fix(create-vite): Fix installing dependencies fix(create-vite): fix installing dependencies Sep 23, 2025
Comment thread packages/create-vite/src/index.ts Outdated
Copy link
Copy Markdown
Member

@bluwy bluwy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit nitpicky, but I think I'd modify this part as something like this instead:

run(agent, getInstallCommand(agent), {
stdio: 'inherit',
cwd: root,
})

const [pm, ...args] = getInstallCommand(agent)
run(pm, args, ...)

Comment thread packages/create-vite/src/index.ts Outdated
@sapphi-red sapphi-red added feat: create-vite create-vite package p4-important Violate documented behavior or significantly improves performance (priority) labels Sep 23, 2025
@sapphi-red
Copy link
Copy Markdown
Member

I do not understand why they were passing.

It's probably because the install itself is skipped in the tests 🙈

if (process.env._VITE_TEST_CLI) {
prompts.log.step(
`Installing dependencies with ${agent}... (skipped in test)`,
)
return
}

@nicolo-ribaudo
Copy link
Copy Markdown
Contributor Author

Uh linting is failing, give me a second... I was editing through GitHub rather than locally 😅

@sapphi-red
Copy link
Copy Markdown
Member

Ah, I've fixed it 🫡 (saw the comment after doing it)

Copy link
Copy Markdown
Member

@sapphi-red sapphi-red left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR 💚

@sapphi-red sapphi-red merged commit 01ae663 into vitejs:main Sep 23, 2025
16 checks passed
@nicolo-ribaudo nicolo-ribaudo deleted the patch-1 branch September 23, 2025 14:17
@nicolo-ribaudo
Copy link
Copy Markdown
Contributor Author

Thanks for fixing it up!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feat: create-vite create-vite package p4-important Violate documented behavior or significantly improves performance (priority)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unknown command: npm

3 participants