Resolution error on missing optional peerDependency

[marvinhagemeister]

The npm package ws has an optional peerDependency on bufferutil. In their code they check for this via:

if (!process.env.WS_NO_BUFFER_UTIL) {
  try {
    const bufferUtil = require('bufferutil');
   // ...
  } catch (e) {
    // Continue regardless of the error.
  }
}

But since the @deno/loader tries to initialize all packages up front it will error out.

Steps to reproduce

1. Clone https://github.com/marvinhagemeister/deno-loader-optional-bug
2. Run deno install
3. Run deno run -A main.ts

Output:

$ deno run -A main.ts
error: Uncaught (in promise) Error: Could not find package 'bufferutil' from referrer 'file:///Users/marvinhagemeister/Library/Caches/deno/npm/registry.npmjs.org/ws/8.18.3/node_modules/ws/lib/buffer-util.js' (ws@8.18.3).
  const ret = new Error(getStringFromWasm0(arg0, arg1));
              ^
    at __wbindgen_error_new (https://jsr.io/@deno/loader/0.1.3/src/lib/rs_lib.internal.js:978:15)
    at <anonymous> (wasm://wasm/013c5e56:1:4115117)
    at <anonymous> (wasm://wasm/013c5e56:1:3914089)
    at <anonymous> (wasm://wasm/013c5e56:1:3978417)
    at DenoLoader.resolve (https://jsr.io/@deno/loader/0.1.3/src/lib/rs_lib.internal.js:318:24)
    at Loader.resolve (https://jsr.io/@deno/loader/0.1.3/src/mod.ts:194:31)
    at file:///Users/marvinhagemeister/dev/test/deno-loader-optional/main.ts:14:31

Original reported at denoland/fresh#3078

[dsherret]

Not sure this one is actually a bug. The behaviour is expected because calling loader.resolve("bufferutil", ...) from that file will error because it can't resolve it.

[marvinhagemeister]

Fair point. I'll try to just catch that error in the esbuild plugin.

[dsherret]

I kind of wonder what's a good way to handle these kind of errors though.

Does the bundler give information that the require is in a try/catch? Or maybe it should be up to deno_loader to include the information that the package was marked as optional in the package.json?

I'm also wondering if errors should all have a code associated with them so they're easier to check for instead of looking at the message.

[nestarz]

@dsherret It would be nice to add another method to the loader to call for example when an error occured (or before) to check if a specifier is a peer dependency - and return if it's available in the loader context. Or throw a custom error that is easily parsable so we can know if it's a peer dependency and act accordingly.

A current hack I use:

const pkgJsonCache = new Map<string, PkgJson | null>();

interface PkgJson {
  optionalDependencies?: { [k: string]: string };
  peerDependencies?: { [k: string]: string };
}
const findNearestPackageJson = async (
  referrerUrl: URL,
  debug?: boolean,
): Promise<PkgJson | null> => {
  let currentUrl = new URL(".", referrerUrl);

  while (true) {
    const pkgJsonUrl = new URL("package.json", currentUrl);
    if (pkgJsonCache.has(pkgJsonUrl.href)) {
      return pkgJsonCache.get(pkgJsonUrl.href) ?? null;
    }
    try {
      const response = await fetch(pkgJsonUrl);
      if (response.ok) {
        const pkgJson = await response.json();
        pkgJsonCache.set(pkgJsonUrl.href, pkgJson);
        return pkgJson;
      }
    } catch (error: unknown) {
      if (
        !(error instanceof TypeError && error.message.includes("Not found"))
      ) {
        const err = error instanceof Error ? error.message : error;
        if (debug) console.warn(`Warning fetching ${pkgJsonUrl.href}:`, err);
      } else console.error(error);
    }
    pkgJsonCache.set(pkgJsonUrl.href, null);
    const parentUrl = new URL("..", currentUrl);
    if (parentUrl.href === currentUrl.href) break;
    currentUrl = parentUrl;
  }
  return null;
};

export async function safeResolve(
  specifier: string,
  referrer: string,
  resolveFn: () => Promise<string> | string,
  debug?: boolean,
): Promise<string | null> {
  try {
    return await resolveFn();
  } catch (error) {
    if (
      error instanceof Error &&
      error.message.startsWith("Could not find package")
    ) {
      const pkgJson = await findNearestPackageJson(new URL(referrer), debug);
      const isOptional = pkgJson?.optionalDependencies?.[specifier];
      const isPeer = pkgJson?.peerDependencies?.[specifier];
      if (isOptional || isPeer) {
        console.log(specifier, "is peer or optional; ignoring;");
        return null;
      }
    }
    throw error;
  }
}

// --- Demonstration ---
import { ResolutionMode, Workspace } from "jsr:@deno/loader";

const referrer =
  "file:///Users/x/Library/Caches/deno/npm/registry.npmjs.org/framer-motion/12.23.3/dist/es/render/dom/utils/filter-props.mjs";
const specifier = "@emotion/is-prop-valid";

const workspace = new Workspace({ noConfig: true });
const loader = await workspace.createLoader({
  entrypoints: ["npm:framer-motion"],
});

const resolvedId = await safeResolve(
  specifier,
  referrer,
  // The callback provides the action to be tried
  () => loader.resolve(specifier, referrer, ResolutionMode.Require),
);

console.log(resolvedId);

[marvinhagemeister]

+1 on associating errors with code or using distinct error classes to make them distinguishable.

Thinking some more about this, maybe just erroring is actually the correct approach. The dependency is technically not installed, hence it cannot be resolved. It's fair to expect integrations to wrap resolution with a try/catch statement.

[dsherret]

This now has the information on ResolveError, which is thrown in this case when resolving: https://jsr.io/@deno/loader/doc/~/ResolveError#property_isoptionaldependency