diff --git a/src/install/PackageManager/PackageJSONEditor.rs b/src/install/PackageManager/PackageJSONEditor.rs index 23322d4adf0c..d962b9a634fe 100644 --- a/src/install/PackageManager/PackageJSONEditor.rs +++ b/src/install/PackageManager/PackageJSONEditor.rs @@ -582,11 +582,30 @@ fn for_each_catalog_object( Ok(()) } +/// Is `name` defined in any `catalog`/`catalogs` group? +fn catalog_defines_package(root_package_json: &Expr, name: &[u8]) -> bool { + let mut found = false; + let _ = for_each_catalog_object(root_package_json, |_catalog_name, catalog_expr| { + if !found { + if let Some(obj) = catalog_expr.data.e_object() { + if obj.has_property(name) { + found = true; + } + } + } + Ok(()) + }); + found +} + /// Records the original version of every catalog entry and, with `--latest`, /// rewrites each to `latest` in memory so the resolver fetches it. +/// `names_filter` restricts recording to the named targets (every group +/// defining a name, as in the interactive updater); `None` records everything. pub(crate) fn edit_catalogs_before_update( manager: &mut PackageManager, root_package_json: &Expr, + names_filter: Option<&[Box<[u8]>]>, ) -> Result { // see note in `edit_update_no_args` — always avoid the store let _guard = ExprDisabler::scope(); @@ -618,6 +637,15 @@ pub(crate) fn edit_catalogs_before_update( continue; } + if let Some(filter) = names_filter { + let key_str = key + .as_utf8_string_literal() + .unwrap_or_else(|| bun_core::out_of_memory()); + if !filter.iter().any(|n| strings::eql_long(n, key_str, true)) { + continue; + } + } + let version_literal = value .as_utf8_string_literal() .unwrap_or_else(|| bun_core::out_of_memory()); @@ -904,6 +932,7 @@ pub(crate) fn edit( let mut i: usize = 0; 'loop_: while i < updates.len() { let request = &mut updates[i]; + let mut matched_catalog_reference = false; // order-insensitive scan: `FOUR` is fine here 'dependency_group: for list in DependencyGroup::FOUR.map(|g| g.prop) { if let Some(query) = current_package_json.as_property(list) { @@ -922,6 +951,11 @@ pub(crate) fn edit( }, ); + // The kept reference points at a catalog + // entry; that entry is the update target. + matched_catalog_reference |= + keep_catalog_reference && options.before_install; + if request.package_id != INVALID_PACKAGE_ID && strings::eql_long(list, dependency_list, true) && !keep_catalog_reference @@ -1070,11 +1104,36 @@ pub(crate) fn edit( } } } + if matched_catalog_reference { + updates[i].is_catalog = true; + } i += 1; } } } + // Catalog named targets are handled by `edit_catalogs_*`, not the append + // block below. Classify only before install: post-install, a root-group + // match takes the `replacing` branch, which leaves `e_string` unset and + // must not be reclassified here. + if manager.subcommand == Subcommand::Update { + for request in updates.iter_mut() { + if options.before_install + && !request.is_catalog + && request.e_string.is_none() + && request.package_id == INVALID_PACKAGE_ID + && catalog_defines_package(current_package_json, request.get_name()) + { + request.is_catalog = true; + } + // A catalog-only target occupies no dependency-group slot; a kept + // `catalog:` reference was already counted when it matched. + if request.is_catalog && request.e_string.is_none() { + remaining -= 1; + } + } + } + if remaining != 0 { let mut new_dependencies: Vec = { let mut dependencies: Vec = Vec::new(); @@ -1149,7 +1208,7 @@ pub(crate) fn edit( }; for request in updates.iter_mut() { - if request.e_string.is_some() { + if request.e_string.is_some() || request.is_catalog { continue; } diff --git a/src/install/PackageManager/UpdateRequest.rs b/src/install/PackageManager/UpdateRequest.rs index 90c124c6fc70..6aa875a2f517 100644 --- a/src/install/PackageManager/UpdateRequest.rs +++ b/src/install/PackageManager/UpdateRequest.rs @@ -33,6 +33,9 @@ pub struct UpdateRequest { pub(crate) package_id: PackageID, pub(crate) is_aliased: bool, pub failed: bool, + /// The target lives only in a `catalog`/`catalogs` map; its catalog entry + /// is updated instead of adding a root dependency. + pub(crate) is_catalog: bool, /// This must be cloned to handle when the AST store resets. /// ARENA-owned (AST `Expr.Data` store) — raw pointer per LIFETIMES.tsv; /// only valid while the store that allocated it is alive. @@ -49,6 +52,7 @@ impl Default for UpdateRequest { package_id: INVALID_PACKAGE_ID, is_aliased: false, failed: false, + is_catalog: false, e_string: None, } } diff --git a/src/install/PackageManager/updatePackageJSONAndInstall.rs b/src/install/PackageManager/updatePackageJSONAndInstall.rs index 850bac548da5..771a3acd88da 100644 --- a/src/install/PackageManager/updatePackageJSONAndInstall.rs +++ b/src/install/PackageManager/updatePackageJSONAndInstall.rs @@ -596,12 +596,28 @@ fn update_package_json_and_install_with_manager_with_updates( .as_deref() .is_none_or(|t| t.iter().any(|w| w.is_root)); + let catalog_request_names: Vec> = manager + .update_requests + .iter() + .filter(|r| r.is_catalog) + .map(|r| Box::<[u8]>::from(r.get_name())) + .collect(); + if subcommand == Subcommand::Update - && manager.update_requests.is_empty() + && (manager.update_requests.is_empty() || !catalog_request_names.is_empty()) && root_is_targeted { + let names_filter = if manager.update_requests.is_empty() { + None + } else { + Some(&catalog_request_names[..]) + }; let root_package_json_root: bun_ast::Expr = root_package_json.root; - if PackageJSONEditor::edit_catalogs_before_update(manager, &root_package_json_root)? { + if PackageJSONEditor::edit_catalogs_before_update( + manager, + &root_package_json_root, + names_filter, + )? { editing_catalogs = true; if manager.options.do_.contains(Do::UPDATE_TO_LATEST) { @@ -671,21 +687,6 @@ fn update_package_json_and_install_with_manager_with_updates( }, )?; } - - if editing_catalogs - && manager.workspace_name_hash.is_none() - && manager.update_target_workspaces.is_none() - { - // running from root: catalogs live in this file. - let _ = PackageJSONEditor::edit_catalogs_after_update( - manager, - &new_package_json, - EditOptions { - exact_versions: manager.options.enable.exact_versions(), - ..Default::default() - }, - )?; - } } else { let mut updates_slice: &mut [UpdateRequest] = &mut updates[..]; PackageJSONEditor::edit( @@ -703,6 +704,21 @@ fn update_package_json_and_install_with_manager_with_updates( }, )?; } + + if editing_catalogs + && manager.workspace_name_hash.is_none() + && manager.update_target_workspaces.is_none() + { + // running from root: catalogs live in this file. + let _ = PackageJSONEditor::edit_catalogs_after_update( + manager, + &new_package_json, + EditOptions { + exact_versions: manager.options.enable.exact_versions(), + ..Default::default() + }, + )?; + } let mut buffer_writer_two = js_printer::BufferWriter::init(); buffer_writer_two.buffer.list.reserve( (source.contents.len() + 1).saturating_sub(buffer_writer_two.buffer.list.len()), diff --git a/src/install/lockfile.rs b/src/install/lockfile.rs index 7d763ecf7f20..a53592db7478 100644 --- a/src/install/lockfile.rs +++ b/src/install/lockfile.rs @@ -727,8 +727,11 @@ impl Lockfile { continue; } let res = resolutions_of_yore[old_resolution as usize]; + // a `catalog:` reference keeps its literal; the + // catalog definition is updated instead if res.tag != ResolutionTag::Npm || update.version.tag != dependency::Tag::DistTag + || dep.version.tag == dependency::Tag::Catalog { continue; } @@ -782,8 +785,11 @@ impl Lockfile { continue; } let res = resolutions_of_yore[old_resolution as usize]; + // a `catalog:` reference keeps its literal; the + // catalog definition is updated instead if res.tag != ResolutionTag::Npm || update.version.tag != dependency::Tag::DistTag + || dep.version.tag == dependency::Tag::Catalog { continue; } diff --git a/test/cli/install/catalogs.test.ts b/test/cli/install/catalogs.test.ts index 08cc2b4044db..82bb49ac32d8 100644 --- a/test/cli/install/catalogs.test.ts +++ b/test/cli/install/catalogs.test.ts @@ -509,6 +509,234 @@ describe("update", () => { "no-deps": "catalog:", "a-dep": "catalog:a", }); + // the referenced catalog entry is what gets updated; untargeted entries stay + const root = await file(join(packageDir, "package.json")).json(); + expect(root.workspaces.catalog).toEqual({ "no-deps": "^2.0.0" }); + expect(root.workspaces.catalogs).toEqual({ a: { "a-dep": "~1.0.1" } }); + expect(exitCode).toBe(0); + }); + + test("update from a workspace updates the root catalog entry it references", async () => { + const { packageDir } = await registry.createTestDir(); + await createUpdateMonorepo(packageDir, "catalog-update-named-from-ws"); + await runBunInstall(bunEnv, packageDir); + + const { err, exitCode } = await runUpdate(join(packageDir, "packages", "pkg1"), "no-deps"); + expect(err).not.toContain("error:"); + + const root = await file(join(packageDir, "package.json")).json(); + expect(root.dependencies).toBeUndefined(); + expect(root.workspaces.catalog).toEqual({ "no-deps": "^1.1.0" }); + expect(root.workspaces.catalogs).toEqual({ a: { "a-dep": "~1.0.1" } }); + expect((await file(join(packageDir, "packages", "pkg1", "package.json")).json()).dependencies).toEqual({ + "no-deps": "catalog:", + "a-dep": "catalog:a", + }); + expect(exitCode).toBe(0); + }); + + test("update updates the catalog entry referenced by the root package.json itself", async () => { + const { packageDir } = await registry.createTestDir(); + await Promise.all([ + write( + join(packageDir, "package.json"), + JSON.stringify({ + name: "catalog-update-named-root-ref", + dependencies: { "no-deps": "catalog:" }, + workspaces: { + packages: ["packages/*"], + catalog: { "no-deps": "^1.0.0" }, + }, + }), + ), + write(join(packageDir, "packages", "pkg1", "package.json"), JSON.stringify({ name: "pkg1" })), + ]); + await runBunInstall(bunEnv, packageDir); + + const { err, exitCode } = await runUpdate(packageDir, "no-deps"); + expect(err).not.toContain("error:"); + + const root = await file(join(packageDir, "package.json")).json(); + // the reference is kept and the referenced entry is updated + expect(root.dependencies).toEqual({ "no-deps": "catalog:" }); + expect(root.workspaces.catalog).toEqual({ "no-deps": "^1.1.0" }); + expect(exitCode).toBe(0); + }); + + // https://github.com/oven-sh/bun/issues/32808 + test("update from root updates the catalog entry instead of adding a root dependency", async () => { + const { packageDir } = await registry.createTestDir(); + await createUpdateMonorepo(packageDir, "catalog-update-named"); + await runBunInstall(bunEnv, packageDir); + + const { err, exitCode } = await runUpdate(packageDir, "no-deps"); + expect(err).not.toContain("error:"); + + const root = await file(join(packageDir, "package.json")).json(); + // no spurious top-level dependency is synthesized for the catalog package + expect(root.dependencies).toBeUndefined(); + // the targeted entry moves within range; the untargeted one is untouched + expect(root.workspaces.catalog).toEqual({ "no-deps": "^1.1.0" }); + expect(root.workspaces.catalogs).toEqual({ a: { "a-dep": "~1.0.1" } }); + + expect((await file(join(packageDir, "packages", "pkg1", "package.json")).json()).dependencies).toEqual({ + "no-deps": "catalog:", + "a-dep": "catalog:a", + }); + expect(await file(join(packageDir, "node_modules", "no-deps", "package.json")).json()).toEqual({ + name: "no-deps", + version: "1.1.0", + }); + expect(exitCode).toBe(0); + }); + + test("update from root targets a named catalog entry", async () => { + const { packageDir } = await registry.createTestDir(); + await createUpdateMonorepo(packageDir, "catalog-update-named-group"); + await runBunInstall(bunEnv, packageDir); + + const { err, exitCode } = await runUpdate(packageDir, "a-dep"); + expect(err).not.toContain("error:"); + + const root = await file(join(packageDir, "package.json")).json(); + expect(root.dependencies).toBeUndefined(); + expect(root.workspaces.catalog).toEqual({ "no-deps": "^1.0.0" }); + expect(root.workspaces.catalogs).toEqual({ a: { "a-dep": "~1.0.10" } }); + expect(exitCode).toBe(0); + }); + + test("update --latest from root bumps the catalog entry past its range, preserving the pin style", async () => { + const { packageDir } = await registry.createTestDir(); + await createUpdateMonorepo(packageDir, "catalog-update-named-latest"); + await runBunInstall(bunEnv, packageDir); + + const { err, exitCode } = await runUpdate(packageDir, "no-deps", "--latest"); + expect(err).not.toContain("error:"); + + const root = await file(join(packageDir, "package.json")).json(); + expect(root.dependencies).toBeUndefined(); + expect(root.workspaces.catalog).toEqual({ "no-deps": "^2.0.0" }); + expect(root.workspaces.catalogs).toEqual({ a: { "a-dep": "~1.0.1" } }); + expect(exitCode).toBe(0); + }); + + test("update --latest updates every catalog group defining the package; unconsumed entries are restored", async () => { + const { packageDir } = await registry.createTestDir(); + await Promise.all([ + write( + join(packageDir, "package.json"), + JSON.stringify({ + name: "catalog-update-named-groups", + workspaces: { + packages: ["packages/*"], + catalog: { + "no-deps": "^1.0.0", + }, + catalogs: { + pinned: { + "no-deps": "1.0.1", + }, + unused: { + "no-deps": "1.0.0", + }, + }, + }, + }), + ), + write( + join(packageDir, "packages", "pkg1", "package.json"), + JSON.stringify({ + name: "pkg1", + dependencies: { "no-deps": "catalog:" }, + }), + ), + write( + join(packageDir, "packages", "pkg2", "package.json"), + JSON.stringify({ + name: "pkg2", + dependencies: { "no-deps": "catalog:pinned" }, + }), + ), + ]); + await runBunInstall(bunEnv, packageDir); + + const { err, exitCode } = await runUpdate(packageDir, "no-deps", "--latest"); + expect(err).not.toContain("error:"); + + const root = await file(join(packageDir, "package.json")).json(); + expect(root.dependencies).toBeUndefined(); + expect(root.workspaces.catalog).toEqual({ "no-deps": "^2.0.0" }); + expect(root.workspaces.catalogs.pinned).toEqual({ "no-deps": "2.0.0" }); + // no workspace consumes this entry, so it cannot resolve and is left as-is + expect(root.workspaces.catalogs.unused).toEqual({ "no-deps": "1.0.0" }); + expect(exitCode).toBe(0); + }); + + test("update prefers a root dependency over a same-named catalog entry", async () => { + const { packageDir } = await registry.createTestDir(); + await Promise.all([ + write( + join(packageDir, "package.json"), + JSON.stringify({ + name: "catalog-update-named-precedence", + dependencies: { "no-deps": "^1.0.0" }, + workspaces: { + packages: ["packages/*"], + catalog: { "no-deps": "^1.0.0" }, + }, + }), + ), + write( + join(packageDir, "packages", "pkg1", "package.json"), + JSON.stringify({ + name: "pkg1", + dependencies: { "no-deps": "catalog:" }, + }), + ), + ]); + await runBunInstall(bunEnv, packageDir); + + const { err, exitCode } = await runUpdate(packageDir, "no-deps"); + expect(err).not.toContain("error:"); + + const root = await file(join(packageDir, "package.json")).json(); + // the direct dependency is rewritten; the catalog entry is left alone + expect(root.dependencies).toEqual({ "no-deps": "^1.1.0" }); + expect(root.workspaces.catalog).toEqual({ "no-deps": "^1.0.0" }); + expect(exitCode).toBe(0); + }); + + test("update bumps an npm: aliased catalog entry, preserving the alias", async () => { + const { packageDir } = await registry.createTestDir(); + await Promise.all([ + write( + join(packageDir, "package.json"), + JSON.stringify({ + name: "catalog-update-named-alias", + workspaces: { + packages: ["packages/*"], + catalogs: { + a: { "my-no-deps": "npm:no-deps@^1.0.0" }, + }, + }, + }), + ), + write( + join(packageDir, "packages", "pkg1", "package.json"), + JSON.stringify({ + name: "pkg1", + dependencies: { "my-no-deps": "catalog:a" }, + }), + ), + ]); + await runBunInstall(bunEnv, packageDir); + + const { err, exitCode } = await runUpdate(packageDir, "my-no-deps"); + expect(err).not.toContain("error:"); + + const root = await file(join(packageDir, "package.json")).json(); + expect(root.dependencies).toBeUndefined(); + expect(root.workspaces.catalogs).toEqual({ a: { "my-no-deps": "npm:no-deps@^1.1.0" } }); expect(exitCode).toBe(0); }); });