Add support for yarn add jsr:@foo/bar (#6757)

## What's the problem this PR addresses?

My PR at #6752 added support for
the `jsr:` protocol, but in a way that didn't allow for running `yarn
add jsr:@foo/bar` (or even `yarn add @foo/bar@jsr:1.0.0`), because it
was setup as a conversion pass in dependency maps - ie it didn't
integrate well with `suggestUtils`.

## How did you fix it?

I refactored the code to now use a proper resolver & fetcher.

A hack remains (see description of `getResolutionDependencies`); it's a
medium-term solution. I have a better fix in mind, but that will require
some refactoring of our internals so it'll be deferred until the next
major.

## Checklist

<!--- Don't worry if you miss something, chores are automatically
tested. -->
<!--- This checklist exists to help you remember doing the chores when
you submit a PR. -->
<!--- Put an `x` in all the boxes that apply. -->
- [x] I have read the [Contributing
Guide](https://yarnpkg.com/advanced/contributing).

<!-- See
https://yarnpkg.com/advanced/contributing#preparing-your-pr-to-be-released
for more details. -->
<!-- Check with `yarn version check` and fix with `yarn version check
-i` -->
- [x] I have set the packages that need to be released for my changes to
be effective.

<!-- The "Testing chores" workflow validates that your PR follows our
guidelines. -->
<!-- If it doesn't pass, click on it to see details as to what your PR
might be missing. -->
- [x] I will check that all automated PR checks pass before the PR gets
reviewed.

Author: arcanis

.yarn/versions/3749f790.yml

@@ -0,0 +1,35 @@
+releases:
+  "@yarnpkg/cli": minor
+  "@yarnpkg/core": minor
+  "@yarnpkg/plugin-essentials": minor
+
+declined:
+  - "@yarnpkg/plugin-compat"
+  - "@yarnpkg/plugin-constraints"
+  - "@yarnpkg/plugin-dlx"
+  - "@yarnpkg/plugin-exec"
+  - "@yarnpkg/plugin-file"
+  - "@yarnpkg/plugin-git"
+  - "@yarnpkg/plugin-github"
+  - "@yarnpkg/plugin-http"
+  - "@yarnpkg/plugin-init"
+  - "@yarnpkg/plugin-interactive-tools"
+  - "@yarnpkg/plugin-jsr"
+  - "@yarnpkg/plugin-link"
+  - "@yarnpkg/plugin-nm"
+  - "@yarnpkg/plugin-npm"
+  - "@yarnpkg/plugin-npm-cli"
+  - "@yarnpkg/plugin-pack"
+  - "@yarnpkg/plugin-patch"
+  - "@yarnpkg/plugin-pnp"
+  - "@yarnpkg/plugin-pnpm"
+  - "@yarnpkg/plugin-stage"
+  - "@yarnpkg/plugin-typescript"
+  - "@yarnpkg/plugin-version"
+  - "@yarnpkg/plugin-workspace-tools"
+  - "@yarnpkg/builder"
+  - "@yarnpkg/doctor"
+  - "@yarnpkg/extensions"
+  - "@yarnpkg/nm"
+  - "@yarnpkg/pnpify"
+  - "@yarnpkg/sdks"

packages/acceptance-tests/pkg-tests-specs/sources/protocols/jsr.test.ts

@@ -4,6 +4,34 @@ import {tests}         from 'pkg-tests-core';
 
 describe(`Protocols`, () => {
   describe(`jsr:`, () => {
+    test(
+      `is should allow adding a package with "yarn add"`,
+      makeTemporaryEnv(
+        {},
+        async ({path, run, source}) => {
+          await xfs.writeFilePromise(ppath.join(path, `.yarnrc.yml`), JSON.stringify({
+            [`npmScopes`]: {
+              [`jsr`]: {
+                [`npmRegistryServer`]: `${await tests.startPackageServer()}/registry/jsr`,
+              },
+            },
+          }));
+
+          await run(`add`, `jsr:no-deps-jsr`);
+
+          await expect(source(`require('no-deps-jsr')`)).resolves.toMatchObject({
+            name: `@jsr/no-deps-jsr`,
+          });
+
+          await expect(xfs.readJsonPromise(ppath.join(path, `package.json`))).resolves.toMatchObject({
+            dependencies: {
+              [`no-deps-jsr`]: `jsr:^1.0.0`,
+            },
+          });
+        },
+      ),
+    );
+
     test(
       `it should allow installing a package from a jsr registry`,
       makeTemporaryEnv(

packages/plugin-essentials/sources/commands/add.ts

@@ -161,10 +161,23 @@ export default class AddCommand extends BaseCommand {
       ? Infinity
       : 1;
 
+    const jsrToDescriptor = (pseudoDescriptor: string) => {
+      const descriptor = structUtils.tryParseDescriptor(pseudoDescriptor.slice(4));
+      if (!descriptor)
+        return null;
+
+      if (descriptor.range === `unknown`)
+        return structUtils.makeDescriptor(descriptor, `jsr:${structUtils.stringifyIdent(descriptor)}@latest`);
+
+      return structUtils.makeDescriptor(descriptor, `jsr:${descriptor.range}`);
+    };
+
     const allSuggestions = await Promise.all(this.packages.map(async pseudoDescriptor => {
       const request = pseudoDescriptor.match(/^\.{0,2}\//)
         ? await suggestUtils.extractDescriptorFromPath(pseudoDescriptor as PortablePath, {cwd: this.context.cwd, workspace})
-        : structUtils.tryParseDescriptor(pseudoDescriptor);
+        : pseudoDescriptor.startsWith(`jsr:`)
+          ? jsrToDescriptor(pseudoDescriptor)
+          : structUtils.tryParseDescriptor(pseudoDescriptor);
 
       const unsupportedPrefix = pseudoDescriptor.match(/^(https?:|git@github)/);
       if (unsupportedPrefix)

packages/plugin-essentials/sources/suggestUtils.ts

@@ -1,7 +1,7 @@
-import {Cache, DescriptorHash, Descriptor, Ident, Locator, Manifest, Project, ThrowReport, Workspace, FetchOptions, ResolveOptions, Configuration} from '@yarnpkg/core';
-import {formatUtils, structUtils, semverUtils}                                                                                                     from '@yarnpkg/core';
-import {PortablePath, ppath, xfs}                                                                                                                  from '@yarnpkg/fslib';
-import semver                                                                                                                                      from 'semver';
+import {Cache, DescriptorHash, Descriptor, Ident, Locator, Manifest, Project, ThrowReport, Workspace, FetchOptions, ResolveOptions, Configuration, TAG_REGEXP} from '@yarnpkg/core';
+import {formatUtils, structUtils, semverUtils}                                                                                                                 from '@yarnpkg/core';
+import {PortablePath, ppath, xfs}                                                                                                                              from '@yarnpkg/fslib';
+import semver                                                                                                                                                  from 'semver';
 
 const WORKSPACE_PROTOCOL = `workspace:`;
 
@@ -204,17 +204,56 @@ export async function extractDescriptorFromPath(path: PortablePath, {cwd, worksp
   });
 }
 
+type InferenceParameters = {
+  type: `fixed`;
+  range: string;
+} | {
+  type: `resolve`;
+  range: string;
+};
+
+
+function extractInferenceParametersFromRequest(request: Descriptor): InferenceParameters {
+  if (request.range === `unknown`)
+    return {type: `resolve`, range: `latest`};
+
+  if (semverUtils.validRange(request.range))
+    return {type: `fixed`, range: request.range};
+
+  if (TAG_REGEXP.test(request.range))
+    return {type: `resolve`, range: request.range};
+
+  const registryProtocol = request.range.match(/^(?:jsr:|npm:)(.*)/);
+  if (!registryProtocol)
+    return {type: `fixed`, range: request.range};
+
+  let [, range] = registryProtocol;
+
+  // Try to handle the case of "foo@jsr:foo@latest", because otherwise there
+  // would be no way to cleanly add a package from a 3rd-party registry using
+  // a tag (since foo@jsr:latest would refer to "the package named 'latest'").
+  const selfRegistryPrefix = `${structUtils.stringifyIdent(request)}@`;
+  if (range.startsWith(selfRegistryPrefix))
+    range = range.slice(selfRegistryPrefix.length);
+
+  if (semverUtils.validRange(range))
+    return {type: `fixed`, range: request.range};
+
+  if (TAG_REGEXP.test(range))
+    return {type: `resolve`, range: request.range};
+
+  return {type: `fixed`, range: request.range};
+}
+
 export async function getSuggestedDescriptors(request: Descriptor, {project, workspace, cache, target, fixed, modifier, strategies, maxResults = Infinity}: {project: Project, workspace: Workspace, cache: Cache, target: Target, fixed: boolean, modifier: Modifier, strategies: Array<Strategy>, maxResults?: number}): Promise<Results> {
   if (!(maxResults >= 0))
     throw new Error(`Invalid maxResults (${maxResults})`);
 
-  const [requestRange, requestTag] = request.range !== `unknown`
-    ? fixed || semverUtils.validRange(request.range) || !request.range.match(/^[a-z0-9._-]+$/i)
-      ? [request.range, `latest`]
-      : [`unknown`, request.range]
-    : [`unknown`, `latest`];
+  const inference = !fixed || request.range === `unknown`
+    ? extractInferenceParametersFromRequest(request)
+    : {type: `fixed`, range: request.range};
 
-  if (requestRange !== `unknown`) {
+  if (inference.type === `fixed`) {
     return {
       suggestions: [{
         descriptor: request,
@@ -332,7 +371,7 @@ export async function getSuggestedDescriptors(request: Descriptor, {project, wor
               reason: formatUtils.pretty(project.configuration, `(unavailable because enableNetwork is toggled off)`, `grey`),
             });
           } else {
-            const latest = await fetchDescriptorFrom(request, requestTag, {project, cache, workspace, modifier});
+            const latest = await fetchDescriptorFrom(request, inference.range, {project, cache, workspace, modifier});
             if (latest) {
               suggested.push({
                 descriptor: latest,

packages/plugin-jsr/sources/JsrFetcher.ts

@@ -0,0 +1,22 @@
+import {Fetcher, FetchOptions, Locator} from '@yarnpkg/core';
+
+import {JSR_PROTOCOL}                   from './constants';
+import {convertLocatorFromJsrToNpm}     from './helpers';
+
+export class JsrFetcher implements Fetcher {
+  supports(locator: Locator, opts: FetchOptions) {
+    return locator.reference.startsWith(JSR_PROTOCOL);
+  }
+
+  getLocalPath(locator: Locator, opts: FetchOptions) {
+    const nextLocator = convertLocatorFromJsrToNpm(locator);
+
+    return opts.fetcher.getLocalPath(nextLocator, opts);
+  }
+
+  fetch(locator: Locator, opts: FetchOptions) {
+    const nextLocator = convertLocatorFromJsrToNpm(locator);
+
+    return opts.fetcher.fetch(nextLocator, opts);
+  }
+}

packages/plugin-jsr/sources/JsrResolver.ts

@@ -0,0 +1,80 @@
+import {Locator, MinimalResolveOptions, Package, ResolveOptions, Resolver}                     from '@yarnpkg/core';
+import {Descriptor}                                                                            from '@yarnpkg/core';
+
+import {JSR_PROTOCOL}                                                                          from './constants';
+import {convertDescriptorFromJsrToNpm, convertLocatorFromJsrToNpm, convertLocatorFromNpmToJsr} from './helpers';
+
+export class JsrResolver implements Resolver {
+  supportsDescriptor(descriptor: Descriptor, opts: MinimalResolveOptions) {
+    if (!descriptor.range.startsWith(JSR_PROTOCOL))
+      return false;
+
+    return true;
+  }
+
+  supportsLocator(locator: Locator, opts: MinimalResolveOptions) {
+    if (!locator.reference.startsWith(JSR_PROTOCOL))
+      return false;
+
+    return true;
+  }
+
+  shouldPersistResolution(locator: Locator, opts: MinimalResolveOptions) {
+    const nextLocator = convertLocatorFromJsrToNpm(locator);
+
+    return opts.resolver.shouldPersistResolution(nextLocator, opts);
+  }
+
+  bindDescriptor(descriptor: Descriptor, fromLocator: Locator, opts: MinimalResolveOptions) {
+    return descriptor;
+  }
+
+  getResolutionDependencies(descriptor: Descriptor, opts: MinimalResolveOptions) {
+    // This is a hack. Let me explain:
+    //
+    // Imagine we run `yarn add foo@jsr:^1.0.0`. We want to have `"foo": "jsr:1.2.3"` in our lockfile. For
+    // that to happen, suggestUtils need our resolver to return "foo@jsr:1.2.3" as locator; if we were to
+    // return "foo@npm:@jsr/[email protected]" instead, then it'd be unable to store it as-is (unless we hardcoded
+    // this behavior).
+    //
+    // However, if we return "foo@jsr:1.2.3", then we need to also have a fetcher. Since we don't want to
+    // recode the whole fetcher, we just convert our jsr locator into a npm one and recursively call the
+    // fetcher. This works, but with a caveat: since we are "publicly" resolving to foo@jsr:1.2.3 locator
+    // rather than foo@npm:@jsr/[email protected], then foo@npm:@jsr/[email protected] will not be in the lockfile. And it's
+    // in the lockfile that we keep the checksums, so there would be no checksum registration for this package.
+    //
+    // To avoid this, we register a resolution dependency on the converted npm descriptor. This forces it
+    // to be present in the lockfile.
+    //
+    // It's not ideal because nothing guarantees that foo@jsr:^1.0.0 will be locked to the same version as
+    // foo@npm:@jsr/[email protected] (I think it should be the case except if the user manually modifies the
+    // lockfile), but it's the best I can think of for now (I think we'll be able to fix this in the next
+    // major redesign).
+    //
+    // It's almost like we'd need to have locator dependencies.
+    //
+    return {inner: convertDescriptorFromJsrToNpm(descriptor)};
+  }
+
+  async getCandidates(descriptor: Descriptor, dependencies: Record<string, Package>, opts: ResolveOptions) {
+    const nextDescriptor = opts.project.configuration.normalizeDependency(convertDescriptorFromJsrToNpm(descriptor));
+    const candidates = await opts.resolver.getCandidates(nextDescriptor, dependencies, opts);
+
+    return candidates.map(candidate => {
+      return convertLocatorFromNpmToJsr(candidate);
+    });
+  }
+
+  async getSatisfying(descriptor: Descriptor, dependencies: Record<string, Package>, locators: Array<Locator>, opts: ResolveOptions) {
+    const nextDescriptor = opts.project.configuration.normalizeDependency(convertDescriptorFromJsrToNpm(descriptor));
+
+    return opts.resolver.getSatisfying(nextDescriptor, dependencies, locators, opts);
+  }
+
+  async resolve(locator: Locator, opts: ResolveOptions) {
+    const nextLocator = convertLocatorFromJsrToNpm(locator);
+    const resolved = await opts.resolver.resolve(nextLocator, opts);
+
+    return {...resolved, ...convertLocatorFromNpmToJsr(resolved)};
+  }
+}

packages/plugin-jsr/sources/constants.ts

@@ -0,0 +1 @@
+export const JSR_PROTOCOL = `jsr:`;

packages/plugin-jsr/sources/helpers.ts

@@ -0,0 +1,23 @@
+import {Descriptor, Locator, semverUtils} from '@yarnpkg/core';
+import {structUtils}                      from '@yarnpkg/core';
+
+export function convertDescriptorFromJsrToNpm(dependency: Descriptor) {
+  const rangeWithoutProtocol = dependency.range.slice(4);
+
+  if (semverUtils.validRange(rangeWithoutProtocol))
+    return structUtils.makeDescriptor(dependency, `npm:${structUtils.stringifyIdent(structUtils.wrapIdentIntoScope(dependency, `jsr`))}@${rangeWithoutProtocol}`);
+
+  const parsedRange = structUtils.tryParseDescriptor(rangeWithoutProtocol, true);
+  if (parsedRange !== null)
+    return structUtils.makeDescriptor(dependency, `npm:${structUtils.stringifyIdent(structUtils.wrapIdentIntoScope(parsedRange, `jsr`))}@${parsedRange.range}`);
+
+  throw new Error(`Invalid range: ${dependency.range}`);
+}
+
+export function convertLocatorFromJsrToNpm(locator: Locator) {
+  return structUtils.makeLocator(structUtils.wrapIdentIntoScope(locator, `jsr`), `npm:${locator.reference.slice(4)}`);
+}
+
+export function convertLocatorFromNpmToJsr(locator: Locator) {
+  return structUtils.makeLocator(structUtils.unwrapIdentFromScope(locator, `jsr`), `jsr:${locator.reference.slice(4)}`);
+}

packages/plugin-jsr/sources/index.ts

@@ -1,23 +1,9 @@
-import {Descriptor, Locator, Plugin, Project, ResolveOptions, Resolver, Workspace} from '@yarnpkg/core';
-import {structUtils, semverUtils}                                                  from '@yarnpkg/core';
+import {Plugin, Workspace}             from '@yarnpkg/core';
+import {structUtils}                   from '@yarnpkg/core';
 
-function normalizeJsrDependency(dependency: Descriptor) {
-  if (semverUtils.validRange(dependency.range.slice(4)))
-    return structUtils.makeDescriptor(dependency, `npm:${structUtils.wrapIdentIntoScope(dependency, `jsr`)}@${dependency.range.slice(4)}`);
-
-  const parsedRange = structUtils.tryParseDescriptor(dependency.range.slice(4), true);
-  if (parsedRange !== null)
-    return structUtils.makeDescriptor(dependency, `npm:${structUtils.wrapIdentIntoScope(parsedRange, `jsr`)}@${parsedRange.range}`);
-
-
-  return dependency;
-}
-
-function reduceDependency(dependency: Descriptor, project: Project, locator: Locator, initialDependency: Descriptor, {resolver, resolveOptions}: {resolver: Resolver, resolveOptions: ResolveOptions}) {
-  return dependency.range.startsWith(`jsr:`)
-    ? normalizeJsrDependency(dependency)
-    : dependency;
-}
+import {JsrFetcher}                    from './JsrFetcher';
+import {JsrResolver}                   from './JsrResolver';
+import {convertDescriptorFromJsrToNpm} from './helpers';
 
 const DEPENDENCY_TYPES = [`dependencies`, `devDependencies`, `peerDependencies`];
 
@@ -27,7 +13,7 @@ function beforeWorkspacePacking(workspace: Workspace, rawManifest: any) {
       if (!descriptor.range.startsWith(`jsr:`))
         continue;
 
-      const normalizedDescriptor = normalizeJsrDependency(descriptor);
+      const normalizedDescriptor = convertDescriptorFromJsrToNpm(descriptor);
 
       // Ensure optional dependencies are handled as well
       const identDescriptor = dependencyType === `dependencies`
@@ -45,9 +31,14 @@ function beforeWorkspacePacking(workspace: Workspace, rawManifest: any) {
 
 const plugin: Plugin = {
   hooks: {
-    reduceDependency,
     beforeWorkspacePacking,
   },
+  resolvers: [
+    JsrResolver,
+  ],
+  fetchers: [
+    JsrFetcher,
+  ],
 };
 
 // eslint-disable-next-line arca/no-default-export

packages/yarnpkg-core/sources/structUtils.ts

@@ -641,12 +641,26 @@ export function stringifyIdent(ident: Ident) {
 
 export function wrapIdentIntoScope(ident: Ident, scope: string) {
   if (ident.scope) {
-    return `@${scope}/${ident.scope}__${ident.name}`;
+    return structUtils.makeIdent(scope, `${ident.scope}__${ident.name}`);
   } else {
-    return `@${scope}/${ident.name}`;
+    return structUtils.makeIdent(scope, ident.name);
   }
 }
 
+export function unwrapIdentFromScope(ident: Ident, scope: string) {
+  if (ident.scope !== scope)
+    return ident;
+
+  const underscoreUnderscore = ident.name.indexOf(`__`);
+  if (underscoreUnderscore === -1)
+    return makeIdent(null, ident.name);
+
+  const innerScope = ident.name.slice(0, underscoreUnderscore);
+  const innerName = ident.name.slice(underscoreUnderscore + 2);
+
+  return makeIdent(innerScope, innerName);
+}
+
 /**
  * Returns a string from a descriptor (eg. `@types/lodash@^1.0.0`).
  */