Skip to content

fix: create-vite when targetDir is a valid packageName#4247

Merged
patak-cat merged 3 commits intovitejs:mainfrom
spencer17x:main
Jul 14, 2021
Merged

fix: create-vite when targetDir is a valid packageName#4247
patak-cat merged 3 commits intovitejs:mainfrom
spencer17x:main

Conversation

@spencer17x
Copy link
Copy Markdown
Contributor

Description

fix create-vite without packageName

Additional context

isValidPackageName judgment


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the Commit Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@patak-cat
Copy link
Copy Markdown
Member

The current code is doing what we intended, if the targetDir is a valid packageName, then the next question is ignored because it will be reused. We are only using that prompt if it is invalid.

@spencer17x would you create an issue to describe what problem did you run into?

@spencer17x
Copy link
Copy Markdown
Contributor Author

@patak-js Now when it is created, although the project name is valid, the name of package.json is still empty. When the type is null, the problem is skipped directly, resulting in packageName being undefined

@patak-cat
Copy link
Copy Markdown
Member

Good catch @spencer17x. The condition is still fine, I think we should modify this line

pkg.name = packageName
to fallback to targetDir if packageName is undefined.

@spencer17x
Copy link
Copy Markdown
Contributor Author

@patak-js I think so too, but considering that if the user expects the name of package.json to be inconsistent with the directory, this method requires the user to manually modify the name of package.json, so you think the method is better

@patak-cat
Copy link
Copy Markdown
Member

@patak-js I think so too, but considering that if the user expects the name of package.json to be inconsistent with the directory, this method requires the user to manually modify the name of package.json, so you think the method is better

We discussed this with the team last time, and it was decided that we should enable people to chose a directory name that is not a valid package.json name. We also don't want to ask too many prompts, only the questions that are strictly needed. So the current approach is fine, but you found a bug. You could use this PR if you would like to fix it as we discussed 👍🏼

@spencer17x
Copy link
Copy Markdown
Contributor Author

@patak-js So that's it, I understand, I modified it

@Shinigami92 Shinigami92 added the p3-minor-bug An edge case that only affects very specific usage (priority) label Jul 14, 2021
@patak-cat patak-cat changed the title fix: fix create-vite without packageName fix: create-vite when targetDir is a valid packageName Jul 14, 2021
@patak-cat patak-cat merged commit 02e244d into vitejs:main Jul 14, 2021
aleclarson pushed a commit to aleclarson/vite that referenced this pull request Nov 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

p3-minor-bug An edge case that only affects very specific usage (priority)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants