Allow "catalog" and "catalogs" in top-level package.json if it wasn't provided in "workspaces" object (#21009)

docs/install/catalogs.md

@@ -52,6 +52,8 @@ In your root-level `package.json`, add a `catalog` or `catalogs` field within th
 }
 ```
 
+If you put `catalog` or `catalogs` at the top level of the `package.json` file, that will work too.
+
 ### 2. Reference Catalog Versions in Workspace Packages
 
 In your workspace packages, use the `catalog:` protocol to reference versions:

src/install/lockfile/CatalogMap.zig

@@ -113,9 +113,11 @@ pub fn parseAppend(
     source: *const logger.Source,
     expr: Expr,
     builder: *Lockfile.StringBuilder,
-) OOM!void {
+) OOM!bool {
+    var found_any = false;
     if (expr.get("catalog")) |default_catalog| {
         const group = try this.getOrPutGroup(lockfile, .empty);
+        found_any = true;
         switch (default_catalog.data) {
             .e_object => |obj| {
                 for (obj.properties.slice()) |item| {
@@ -171,6 +173,7 @@ pub fn parseAppend(
     }
 
     if (expr.get("catalogs")) |catalogs| {
+        found_any = true;
         switch (catalogs.data) {
             .e_object => |catalog_names| {
                 for (catalog_names.properties.slice()) |catalog| {
@@ -234,6 +237,8 @@ pub fn parseAppend(
             else => {},
         }
     }
+
+    return found_any;
 }
 
 pub fn sort(this: *CatalogMap, lockfile: *const Lockfile) void {

src/install/lockfile/Package.zig

@@ -1958,8 +1958,19 @@ pub const Package = extern struct {
         // This function depends on package.dependencies being set, so it is done at the very end.
         if (comptime features.is_main) {
             try lockfile.overrides.parseAppend(pm, lockfile, package, log, source, json, &string_builder);
+            var found_any_catalog_or_catalog_object = false;
+            var has_workspaces = false;
             if (json.get("workspaces")) |workspaces_expr| {
-                try lockfile.catalogs.parseAppend(pm, lockfile, log, source, workspaces_expr, &string_builder);
+                found_any_catalog_or_catalog_object = try lockfile.catalogs.parseAppend(pm, lockfile, log, source, workspaces_expr, &string_builder);
+                has_workspaces = true;
+            }
+
+            // `"workspaces"` being an object instead of an array is sometimes
+            // unexpected to people. therefore if you also are using workspaces,
+            // allow "catalog" and "catalogs" in top-level "package.json"
+            // so it's easier to guess.
+            if (!found_any_catalog_or_catalog_object and has_workspaces) {
+                _ = try lockfile.catalogs.parseAppend(pm, lockfile, log, source, json, &string_builder);
             }
         }
 

test/cli/install/catalogs.test.ts

@@ -15,21 +15,32 @@ afterAll(() => {
 });
 
 describe("basic", () => {
-  async function createBasicCatalogMonorepo(packageDir: string, name: string) {
-    const packageJson = {
-      name,
-      workspaces: {
-        packages: ["packages/*"],
-        catalog: {
-          "no-deps": "2.0.0",
-        },
-        catalogs: {
-          a: {
-            "a-dep": "1.0.1",
-          },
+  async function createBasicCatalogMonorepo(packageDir: string, name: string, inTopLevelKey: boolean = false) {
+    const catalogs = {
+      catalog: {
+        "no-deps": "2.0.0",
+      },
+      catalogs: {
+        a: {
+          "a-dep": "1.0.1",
         },
       },
     };
+    const packageJson = !inTopLevelKey
+      ? {
+          name,
+          workspaces: {
+            packages: ["packages/*"],
+            ...catalogs,
+          },
+        }
+      : {
+          name,
+          ...catalogs,
+          workspaces: {
+            packages: ["packages/*"],
+          },
+        };
 
     await Promise.all([
       write(join(packageDir, "package.json"), JSON.stringify(packageJson)),
@@ -47,26 +58,29 @@ describe("basic", () => {
 
     return packageJson;
   }
-  test("both catalog and catalogs", async () => {
-    const { packageDir } = await registry.createTestDir();
 
-    await createBasicCatalogMonorepo(packageDir, "catalog-basic-1");
+  for (const isTopLevel of [true, false]) {
+    test(`both catalog and catalogs ${isTopLevel ? "in top-level" : "in workspaces"}`, async () => {
+      const { packageDir } = await registry.createTestDir();
 
-    await runBunInstall(bunEnv, packageDir);
+      await createBasicCatalogMonorepo(packageDir, "catalog-basic-1", isTopLevel);
 
-    expect(await file(join(packageDir, "node_modules", "no-deps", "package.json")).json()).toEqual({
-      name: "no-deps",
-      version: "2.0.0",
+      await runBunInstall(bunEnv, packageDir);
+
+      expect(await file(join(packageDir, "node_modules", "no-deps", "package.json")).json()).toEqual({
+        name: "no-deps",
+        version: "2.0.0",
+      });
+
+      expect(await file(join(packageDir, "node_modules", "a-dep", "package.json")).json()).toEqual({
+        name: "a-dep",
+        version: "1.0.1",
+      });
+
+      // another install does not save the lockfile
+      await runBunInstall(bunEnv, packageDir, { savesLockfile: false });
     });
-
-    expect(await file(join(packageDir, "node_modules", "a-dep", "package.json")).json()).toEqual({
-      name: "a-dep",
-      version: "1.0.1",
-    });
-
-    // another install does not save the lockfile
-    await runBunInstall(bunEnv, packageDir, { savesLockfile: false });
-  });
+  }
 
   for (const binaryLockfile of [true, false]) {
     test(`detect changes (${binaryLockfile ? "bun.lockb" : "bun.lock"})`, async () => {
@@ -122,7 +136,7 @@ describe("basic", () => {
       });
 
       // update catalogs
-      packageJson.workspaces.catalogs.a["a-dep"] = "1.0.10";
+      packageJson.workspaces!.catalogs!.a["a-dep"] = "1.0.10";
       await write(join(packageDir, "package.json"), JSON.stringify(packageJson));
       ({ err } = await runBunInstall(bunEnv, packageDir, { savesLockfile: true }));
       expect(err).toContain("Saved lockfile");