-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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.
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
fix(cli-create): fix scoped creation #6239
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,17 +16,50 @@ export function hasWrapper(commander: Object, args: Array<string>): boolean { | |
return true; | ||
} | ||
|
||
const coerceName = (name = '') => (name === '' ? 'create' : `create-${name}`); | ||
const coerceScope = (scope = '') => (scope === '@' ? '' : scope); | ||
const coerceFullName = (scope, name) => [coerceScope(scope), coerceName(name)].filter(Boolean).join('/'); | ||
|
||
/** | ||
* # Tests | ||
* | ||
* ## basic | ||
* parseBuilderName('name').packageName === 'create-name' | ||
* parseBuilderName('@scope/name').packageName === '@scope/create-name' | ||
* | ||
* ## not adding "-" if name is empty | ||
* parseBuilderName('@scope/').packageName === '@scope/create' | ||
* parseBuilderName('@scope').packageName === '@scope/create' | ||
* | ||
* ## edge cases | ||
* parseBuilderName('@/name').packageName === 'create-name' | ||
* parseBuilderName('/name').packageName === 'create-name' | ||
* parseBuilderName('@/').packageName === 'create' | ||
*/ | ||
const parseBuilderName = str => { | ||
const parts = str.split('/'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good idea There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I changed About throwing id there are more than 2 parts: and i want yarn to be robust so I just put that extra part into [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',
} |
||
if (parts.length === 1 && !str.includes('@')) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. right There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed |
||
return { | ||
packageName: coerceName(str), | ||
packageDir: '', | ||
commandName: coerceName(str), | ||
}; | ||
} | ||
return { | ||
packageName: coerceFullName(parts[0], parts[1]), | ||
packageDir: coerceScope(parts[0]), | ||
commandName: coerceName(parts[1]), | ||
}; | ||
}; | ||
|
||
export async function run(config: Config, reporter: Reporter, flags: Object, args: Array<string>): Promise<void> { | ||
const [builderName, ...rest] = args; | ||
|
||
if (!builderName) { | ||
throw new MessageError(reporter.lang('invalidPackageName')); | ||
} | ||
|
||
const packageName = builderName.replace(/^(@[^\/]+\/)?/, '$1create-'); | ||
const packageDir = packageName.replace(/^(?:(@[^\/]+)\/)?.*/, '$1'); | ||
const commandName = packageName.replace(/^@[^\/]+\//, ''); | ||
|
||
const {packageName, packageDir, commandName} = parseBuilderName(builderName); | ||
await runGlobal(config, reporter, {}, ['add', packageName]); | ||
|
||
const binFolder = await getBinFolder(config, {}); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer if you could put this in a file called
__tests__/commands/create.js
(you can just export theparseBuilderName
symbol and require the file), so that they are actually ran. Otherwise the comment can just be removed, since it will never be run.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay, will do
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added tests