From 4b2cdc4fc1564f12dc5d339b5ad6bce44ad38d0b Mon Sep 17 00:00:00 2001 From: Dylan Conway <35280289+dylan-conway@users.noreply.github.com> Date: Fri, 20 Oct 2023 03:27:10 -0700 Subject: [PATCH] respect optional peer dependencies and update docs (#6615) * update docs * optional peer dependencies * rename offset variable name, cache invalidation time * Update install.zig * install more peer dependencies --- docs/cli/bun-install.md | 6 ++-- docs/cli/install.md | 4 +-- docs/guides/install/add-peer.md | 2 +- docs/install/index.md | 4 +-- docs/runtime/bunfig.md | 2 +- src/install/dependency.zig | 4 +++ src/install/install.zig | 49 +++++++++++++++++++++------------ src/install/lockfile.zig | 38 +++++++++++++++++++++++-- src/install/npm.zig | 24 ++++++++-------- 9 files changed, 91 insertions(+), 42 deletions(-) diff --git a/docs/cli/bun-install.md b/docs/cli/bun-install.md index 0cbc21e1d9..c7dfcae5d5 100644 --- a/docs/cli/bun-install.md +++ b/docs/cli/bun-install.md @@ -59,8 +59,8 @@ optional = true # Install local devDependencies (default: true) dev = true -# Install peerDependencies (default: false) -peer = false +# Install peerDependencies (default: true) +peer = true # When using `bun install -g`, install packages here globalDir = "~/.bun/install/global" @@ -170,7 +170,7 @@ bun stores normalized `cpu` and `os` values from npm in the lockfile, along with ## Peer dependencies? -Peer dependencies are handled similarly to yarn. `bun install` does not automatically install peer dependencies and will try to choose an existing dependency. +Peer dependencies are handled similarly to yarn. `bun install` will automatically install peer dependencies. If the dependency is marked optional in `peerDependenciesMeta`, an existing dependency will be chosen if possible. ## Lockfile diff --git a/docs/cli/install.md b/docs/cli/install.md index 27b7e51e3d..e3607e55fa 100644 --- a/docs/cli/install.md +++ b/docs/cli/install.md @@ -31,7 +31,7 @@ $ bun install Running `bun install` will: -- **Install** all `dependencies`, `devDependencies`, and `optionalDependencies`. Bun does not install `peerDependencies` by default. +- **Install** all `dependencies`, `devDependencies`, and `optionalDependencies`. Bun will install `peerDependencies` by default. - **Run** your project's `{pre|post}install` and `{pre|post}prepare` scripts at the appropriate time. For security reasons Bun _does not execute_ lifecycle scripts of installed dependencies. - **Write** a `bun.lockb` lockfile to the project root. @@ -162,7 +162,7 @@ optional = true dev = true # whether to install peerDependencies -peer = false +peer = true # equivalent to `--production` flag production = false diff --git a/docs/guides/install/add-peer.md b/docs/guides/install/add-peer.md index 78d5c86b35..84154e13fd 100644 --- a/docs/guides/install/add-peer.md +++ b/docs/guides/install/add-peer.md @@ -2,7 +2,7 @@ name: Add a peer dependency --- -To add an npm package as a peer dependency, directly modify the `peerDependencies` object in your package.json. Running `bun install` will not install peer dependencies. +To add an npm package as a peer dependency, directly modify the `peerDependencies` object in your package.json. Running `bun install` will install peer dependencies by default, unless marked optional in `peerDependenciesMeta`. ```json-diff { diff --git a/docs/install/index.md b/docs/install/index.md index 11e0d4fd4e..8f0aaf3782 100644 --- a/docs/install/index.md +++ b/docs/install/index.md @@ -39,7 +39,7 @@ On Linux, `bun install` tends to install packages 20-100x faster than `npm insta Running `bun install` will: -- **Install** all `dependencies`, `devDependencies`, and `optionalDependencies`. Bun does not install `peerDependencies` by default. +- **Install** all `dependencies`, `devDependencies`, and `optionalDependencies`. Bun will install `peerDependencies` by default. - **Run** your project's `{pre|post}install` scripts at the appropriate time. For security reasons Bun _does not execute_ lifecycle scripts of installed dependencies. - **Write** a `bun.lockb` lockfile to the project root. @@ -81,7 +81,7 @@ optional = true dev = true # whether to install peerDependencies -peer = false +peer = true # equivalent to `--production` flag production = false diff --git a/docs/runtime/bunfig.md b/docs/runtime/bunfig.md index 6950d8233e..edf83f0c42 100644 --- a/docs/runtime/bunfig.md +++ b/docs/runtime/bunfig.md @@ -213,7 +213,7 @@ Whether to install peer dependencies. Default `true`. ```toml [install] -peer = false +peer = true ``` ### `install.production` diff --git a/src/install/dependency.zig b/src/install/dependency.zig index 813ec9ac52..314e364d9e 100644 --- a/src/install/dependency.zig +++ b/src/install/dependency.zig @@ -1022,6 +1022,10 @@ pub const Behavior = packed struct(u8) { return this.optional and !this.isPeer(); } + pub inline fn isOptionalPeer(this: Behavior) bool { + return this.optional and this.isPeer(); + } + pub inline fn isDev(this: Behavior) bool { return this.dev; } diff --git a/src/install/install.zig b/src/install/install.zig index 276e6f70b8..17c5bc4ee0 100644 --- a/src/install/install.zig +++ b/src/install/install.zig @@ -243,6 +243,19 @@ const NetworkTask = struct { header_builder.count("npm-auth-type", "legacy"); } + // "peerDependencies": { + // "@ianvs/prettier-plugin-sort-imports": "*", + // "prettier-plugin-twig-melody": "*" + // }, + // "peerDependenciesMeta": { + // "@ianvs/prettier-plugin-sort-imports": { + // "optional": true + // }, + // Example case ^ + // `@ianvs/prettier-plugin-sort-imports` is peer and also optional but was not marked optional because + // the offset would be 0 and the current loop index is also 0. + const invalidate_manifest_cache_because_optional_peer_dependencies_were_not_marked_as_optional_if_the_optional_peer_dependency_offset_was_equal_to_the_current_index = 1697871350; + pub fn forManifest( this: *NetworkTask, name: string, @@ -291,8 +304,10 @@ const NetworkTask = struct { var last_modified: string = ""; var etag: string = ""; if (loaded_manifest) |manifest| { - last_modified = manifest.pkg.last_modified.slice(manifest.string_buf); - etag = manifest.pkg.etag.slice(manifest.string_buf); + if (manifest.pkg.public_max_age > invalidate_manifest_cache_because_optional_peer_dependencies_were_not_marked_as_optional_if_the_optional_peer_dependency_offset_was_equal_to_the_current_index) { + last_modified = manifest.pkg.last_modified.slice(manifest.string_buf); + etag = manifest.pkg.etag.slice(manifest.string_buf); + } } var header_builder = HeaderBuilder{}; @@ -3262,7 +3277,7 @@ pub const PackageManager = struct { this.enqueueNetworkTask(network_task); } } else { - if (this.options.do.install_peer_dependencies) { + if (this.options.do.install_peer_dependencies and !dependency.behavior.isOptionalPeer()) { try this.peer_dependencies.append(this.allocator, id); } } @@ -3772,18 +3787,16 @@ pub const PackageManager = struct { fn processPeerDependencyList( this: *PackageManager, ) !void { - if (this.peer_dependencies.items.len > 0) { - for (this.peer_dependencies.items) |peer_dependency_id| { - try this.processDependencyListItem(.{ .dependency = peer_dependency_id }, null, true); - const dependency = this.lockfile.buffers.dependencies.items[peer_dependency_id]; - const resolution = this.lockfile.buffers.resolutions.items[peer_dependency_id]; - try this.enqueueDependencyWithMain( - peer_dependency_id, - &dependency, - resolution, - true, - ); - } + while (this.peer_dependencies.popOrNull()) |peer_dependency_id| { + try this.processDependencyListItem(.{ .dependency = peer_dependency_id }, null, true); + const dependency = this.lockfile.buffers.dependencies.items[peer_dependency_id]; + const resolution = this.lockfile.buffers.resolutions.items[peer_dependency_id]; + try this.enqueueDependencyWithMain( + peer_dependency_id, + &dependency, + resolution, + true, + ); } } @@ -8125,11 +8138,11 @@ pub const PackageManager = struct { } if (manager.options.do.install_peer_dependencies) { - try manager.processPeerDependencyList(); + while (manager.pending_tasks > 0 or manager.peer_dependencies.items.len > 0) { + try manager.processPeerDependencyList(); - manager.drainDependencyList(); + manager.drainDependencyList(); - while (manager.pending_tasks > 0) { try manager.runTasks( *PackageManager, manager, diff --git a/src/install/lockfile.zig b/src/install/lockfile.zig index 3e82c88913..a4a23a5408 100644 --- a/src/install/lockfile.zig +++ b/src/install/lockfile.zig @@ -2749,7 +2749,7 @@ pub const Package = extern struct { .name = name.value, .name_hash = name.hash, .behavior = if (comptime is_peer) - group.behavior.setOptional(package_version.optional_peer_dependencies_len > i) + group.behavior.setOptional(i < package_version.non_optional_peer_dependencies_start) else group.behavior, .version = Dependency.parse( @@ -3740,6 +3740,28 @@ pub const Package = extern struct { var workspace_names = WorkspaceMap.init(allocator); defer workspace_names.deinit(); + var optional_peer_dependencies = std.ArrayHashMap(PackageNameHash, void, ArrayIdentityContext.U64, false).init(allocator); + defer optional_peer_dependencies.deinit(); + + if (json.asProperty("peerDependenciesMeta")) |peer_dependencies_meta| { + if (peer_dependencies_meta.expr.data == .e_object) { + const props = peer_dependencies_meta.expr.data.e_object.properties.slice(); + try optional_peer_dependencies.ensureUnusedCapacity(props.len); + for (props) |prop| { + if (prop.value.?.asProperty("optional")) |optional| { + if (optional.expr.data != .e_boolean or !optional.expr.data.e_boolean.value) { + continue; + } + + optional_peer_dependencies.putAssumeCapacity( + String.Builder.stringHash(prop.key.?.asString(allocator) orelse unreachable), + {}, + ); + } + } + } + } + inline for (dependency_groups) |group| { if (json.asProperty(group.prop)) |dependencies_q| brk: { switch (dependencies_q.expr.data) { @@ -4031,7 +4053,12 @@ pub const Package = extern struct { path, logger.Loc.Empty, logger.Loc.Empty, - )) |dep| { + )) |_dep| { + var dep = _dep; + if (group.behavior.isPeer() and optional_peer_dependencies.contains(external_name.hash)) { + dep.behavior = dep.behavior.setOptional(true); + } + package_dependencies[total_dependencies_count] = dep; total_dependencies_count += 1; @@ -4068,7 +4095,12 @@ pub const Package = extern struct { version, key.loc, value.loc, - )) |dep| { + )) |_dep| { + var dep = _dep; + if (group.behavior.isPeer() and optional_peer_dependencies.contains(external_name.hash)) { + dep.behavior = dep.behavior.setOptional(true); + } + package_dependencies[total_dependencies_count] = dep; total_dependencies_count += 1; } diff --git a/src/install/npm.zig b/src/install/npm.zig index c9964e3ba7..aaedd8330c 100644 --- a/src/install/npm.zig +++ b/src/install/npm.zig @@ -451,7 +451,7 @@ pub const PackageVersion = extern struct { optional_dependencies: ExternalStringMap = ExternalStringMap{}, /// `"peerDependencies"` in [package.json](https://docs.npmjs.com/cli/v8/configuring-npm/package-json#peerdependencies) - /// if `optional_peer_dependencies_len` is > 0, then instead of alphabetical, the first N items are optional + /// if `non_optional_peer_dependencies_start` is > 0, then instead of alphabetical, the first N items are optional peer_dependencies: ExternalStringMap = ExternalStringMap{}, /// `"devDependencies"` in [package.json](https://docs.npmjs.com/cli/v8/configuring-npm/package-json#devdependencies) @@ -466,8 +466,8 @@ pub const PackageVersion = extern struct { engines: ExternalStringMap = ExternalStringMap{}, /// `"peerDependenciesMeta"` in [package.json](https://docs.npmjs.com/cli/v8/configuring-npm/package-json#peerdependenciesmeta) - /// if `optional_peer_dependencies_len` is > 0, then instead of alphabetical, the first N items of `peer_dependencies` are optional - optional_peer_dependencies_len: u32 = 0, + /// if `non_optional_peer_dependencies_start` is > 0, then instead of alphabetical, the first N items of `peer_dependencies` are optional + non_optional_peer_dependencies_start: u32 = 0, man_dir: ExternalString = ExternalString{}, @@ -1346,7 +1346,7 @@ pub const PackageManifest = struct { } } - var peer_dependency_len: usize = 0; + var non_optional_peer_dependency_offset: usize = 0; inline for (dependency_groups) |pair| { if (prop.value.?.asProperty(comptime pair.prop)) |versioned_deps| { @@ -1396,17 +1396,17 @@ pub const PackageManifest = struct { // For optional peer dependencies, we store a length instead of a whole separate array // To make that work, we have to move optional peer dependencies to the front of the array // - if (peer_dependency_len != i) { + if (non_optional_peer_dependency_offset != i) { const current_name = this_names[i]; - this_names[i] = this_names[peer_dependency_len]; - this_names[peer_dependency_len] = current_name; + this_names[i] = this_names[non_optional_peer_dependency_offset]; + this_names[non_optional_peer_dependency_offset] = current_name; const current_version = this_versions[i]; - this_versions[i] = this_versions[peer_dependency_len]; - this_versions[peer_dependency_len] = current_version; - - peer_dependency_len += 1; + this_versions[i] = this_versions[non_optional_peer_dependency_offset]; + this_versions[non_optional_peer_dependency_offset] = current_version; } + + non_optional_peer_dependency_offset += 1; } if (optional_peer_dep_names.items.len == 0) { @@ -1431,7 +1431,7 @@ pub const PackageManifest = struct { var version_list = ExternalStringList.init(version_extern_strings, this_versions); if (comptime is_peer) { - package_version.optional_peer_dependencies_len = @as(u32, @truncate(peer_dependency_len)); + package_version.non_optional_peer_dependencies_start = @as(u32, @truncate(non_optional_peer_dependency_offset)); } if (count > 0 and