fix(cli-create): fix scoped creation #6239
Conversation
|
This one works as it should: yarn create @scope/name => @scope/create-name There were problems when name was omitted: Before: yarn create @scope => create- yarn create @scope/ => @scope/create- After: yarn create @scope => @scope/create yarn create @scope/ => @scope/create Fixes yarnpkg#6233
iamstarkov
force-pushed
the
fix/scoped-create
branch
from
636faf9
to
5512472
Compare
|
|
src/cli/commands/create.js
Outdated
| * ## edge cases | ||
| * parseBuilderName('@/name').packageName === 'create-name' | ||
| * parseBuilderName('/name').packageName === 'create-name' | ||
| * parseBuilderName('@/').packageName === 'create' |
There was a problem hiding this comment.
I'd prefer if you could put this in a file called __tests__/commands/create.js (you can just export the parseBuilderName symbol and require the file), so that they are actually ran. Otherwise the comment can just be removed, since it will never be run.
src/cli/commands/create.js
Outdated
| */ | ||
| const parseBuilderName = str => { | ||
| const parts = str.split('/'); | ||
| if (parts.length === 1 && !str.includes('@')) { |
There was a problem hiding this comment.
str.charAt(0) === '@' would be more correct, I think.
src/cli/commands/create.js
Outdated
| * parseBuilderName('@/').packageName === 'create' | ||
| */ | ||
| const parseBuilderName = str => { | ||
| const parts = str.split('/'); |
There was a problem hiding this comment.
It should reject as invalid if there's more than two parts, or if the first part doesn't start with a '@'
There was a problem hiding this comment.
I changed parseBuilderName to parsePackageName and it throws error if first character is . or /. It also throws if scope equals @.
About throwing id there are more than 2 parts: and i want yarn to be robust so I just put that extra part into path field of pkgNameObject¹ and ignore it in rest of create command.
[1]:
'@scope/name/path/to/file.js' => pkgNameObject {
fullName: '@scope/name',
name: 'name',
scope: '@scope',
path: 'path/to/file.js',
full: '@scope/name/path/to/file.js',
}
|
@aracarie updated PR with respect to requested changes |
|
@arcanis had any chance to review it? |
|
Looks great, thanks! 👍 |
This one works as it should:
yarn create @scope/name => @scope/create-name
There were problems when name was omitted:
Before:
yarn create @scope => create-
yarn create @scope/ => @scope/create-
After:
yarn create @scope => @scope/create
yarn create @scope/ => @scope/create
Fixes #6233
Summary
it Is not substantial feature request.
Existing behaviour is a bit odd and not aligned with npm. This PR aims to fix it.
Test plan
Disclaimer:
Couldn't find a place to add test for helper, so I inlined working test assertions.If tests or documentation are required to be added/updated in the same PR please let me know how to do it.