Support URLs such as https://domain/@scope/package #53
Conversation
registry.ts
Outdated
| version(): string { | ||
| const { version } = this.parts(); | ||
| if (version === undefined) { | ||
| throw Error(`Unable to find version in ${this.url}`); |
There was a problem hiding this comment.
won't this be thrown already in defaultParts? https://github.com/hayd/deno-udd/pull/53/files#diff-58f42611a053bb45add25a11c10ed01c89fa3d17a917ecea2de4fad160568ff8R173
There was a problem hiding this comment.
Oh, that's right. I should have paid more attention.
Thank you for your pointing. I'll fix it.
registry.ts
Outdated
| at(version: string): RegistryUrl { | ||
| const { parts, packageName } = this.parts(); | ||
| parts[4] = `${packageName}@${version}`; | ||
| return new UnpkgScope(parts.join("/")); |
There was a problem hiding this comment.
To decrease some boilerplate perhaps:
function defaultScopeAt(that: RegistryUrl): string {
const { parts, packageName } = defaultParts(this);
parts[4] = `${packageName}@${version}`;
return parts.join("/");
}
could be pulled out or something?
There was a problem hiding this comment.
Looks good!
I consider whether to merge defaultScopeAt into defaultAt like this:
export function defaultAt(
that: RegistryUrl,
version: string,
hasScope = false,
): string {
if (!hasScope) return that.url.replace(/@(.*?)(\/|$)/, `@${version}/`);
const { parts, packageName } = defaultParts(that);
parts[4] = `${packageName}@${version}`;
return parts.join("/");
}but this refactoring merely makes registry.ts little shorter, so I'll use the code you suggested.
|
Wow, this is great. Slightly agree with your comment about boilerplate but this functionality will be fantastic to have. (That said, it might be good to test some erroring cases for the parser - i.e. things that throw. But.. looking at test file there aren't any for any of the REGISTRIES! 😳 ) |
|
I'm sorry for late reply. I appreciate your review! |
registry.ts
Outdated
| packageName: string; | ||
| version: string; | ||
| } | ||
| function defaultParts(that: RegistryUrl): PackageInfo { |
There was a problem hiding this comment.
nit: this should probably have been defaultInfo etc ? 😆
but it's fine.
registry.ts
Outdated
| version: string, | ||
| hasScope = false, | ||
| ): string { | ||
| if (!hasScope) return that.url.replace(/@(.*?)(\/|$)/, `@${version}/`); |
There was a problem hiding this comment.
This is probably too clever :), can you delete _defaultAt (unused I think) ?
There was a problem hiding this comment.
Oops! I forgot to delete this. Thanks!
(I wonder if there is such a lint feature which tells me unused export functions and values...)
|
Thanks @takker99 , If you've tested this on a project or two I am happy to merge! 💪 |
|
Thanks! I've fixed them! |
|
Fantastic! Thanks for this @takker99 , this is a very valuable feature. |
close #6