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.

Sign up for GitHub

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

Jump to bottom

fix: two folders are created when the project name contains spaces #11630

Merged
merged 3 commits into from
Feb 1, 2023
Merged

fix: two folders are created when the project name contains spaces #11630

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c51d861
fix: two folders are created when the project name contains spaces
btea Jan 8, 2023
375ec2c
fix: update args
btea Jan 20, 2023
5b31cd5
fix: update cd projectName
btea Jan 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions packages/create-vite/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,9 @@ async function init() {
})

const [command, ...args] = fullCustomCommand.split(' ')
if (targetDir.includes(' ')) {
args.splice(2, args.length - 2, targetDir)
}
Copy link
Member

Choose a reason for hiding this comment

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

I think simply replacing args instead of replacing the fullCustomCommand would be more easy to understand.

// we replace TARGET_DIR here because targetDir may include a space
let replacedArgs = args.map(arg => arg.replace('TARGET_DIR', targetDir))

Would you also add a test here?
https://github.com/vitejs/vite/blob/main/packages/create-vite/__tests__/cli.spec.ts

Copy link
Collaborator Author

@btea btea Jan 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The code has been updated, but there seems to be something wrong with my device, could you help to add a test?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

When I add the test, I found that if projectName contains spaces, the value received by progress.argv will be split. Therefore, targetDir can only get the characters before the spaces. How to deal with this scense? 🤔

Copy link
Member

Choose a reason for hiding this comment

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

In that case, I think the user should quote the project name (e.g. npm init vite@latest "project name with spaces").

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, However, when executing pnpm create vite, the input Project name may contain spaces. I think that after the template is created, if the project name contains spaces, the project name should be wrapped in quotes in the cd projectName output from the terminal.

Copy link
Member

Choose a reason for hiding this comment

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

So you're suggest updating this line? I think that makes sense.

vite/packages/create-vite/src/index.ts

Line 381 in 697dd00

console.log(` cd ${path.relative(cwd, root)}`)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I think it should be updated here. And the previous modification is also necessary. But I don't know how to write this test case?

Copy link
Member

Choose a reason for hiding this comment

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

I tried writing the test but it seems to be quite complex. I think we can go without tests.

If we replace this execaCommandSync with execaSync, we can test the case of passing the project name from argv.

vite/packages/create-vite/__tests__/cli.spec.ts

Line 16 in 2ff5930

return execaCommandSync(`node ${CLI_PATH} ${args.join(' ')}`, options)

But for the case of typing the project name, we need to feed the text from stdin and the test seems to hang.

const { status } = spawn.sync(command, args, {
stdio: 'inherit',
})
Expand Down