feat(install): warn when installing deprecated packages #26443
Conversation
WalkthroughNPM package manifests now capture deprecation messages; lockfile package metadata records deprecation state; installer emits deprecation warnings when resolving packages with deprecation messages; regression tests added to verify warnings for deprecated packages and no warnings for non-deprecated packages. Changes
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/install/lockfile/Package/Meta.zig`:
- Around line 31-40: Rename the private padding field `_padding_integrity` to
`#padding_integrity` in the Package.Meta struct and update all references to
that symbol (e.g., any code that reads or writes `_padding_integrity`) to use
`#padding_integrity`; verify the Zig compiler version in use accepts
`#`-prefixed private field names inside extern/packed structs—if the version
disallows `#` on extern fields, keep the underscore and add a comment explaining
why. Ensure the enum `is_deprecated` and surrounding layout are unchanged and
run the project build to confirm no layout or symbol errors occur.
In `@src/install/npm.zig`:
- Around line 2200-2204: The string buffer is undersized because deprecation
texts are only appended in the second pass; update the first-pass counting logic
(the count() used to drive string_builder.allocate()) to detect
prop.value.asProperty("deprecated") and, when
deprecated.expr.asString(allocator) yields a string, add its length to the count
so string_builder.allocate() reserves enough capacity; keep the existing
second-pass append using string_builder.append(ExternalString, msg) in the
package_version.deprecated path.
In `@src/install/PackageManager/PackageManagerEnqueue.zig`:
- Around line 1420-1429: The deprecation warning printed in the
PackageManagerEnqueue code uses Output.warn without a trailing newline, so add a
newline to the message format (or use the Output method variant that appends a
newline) for the Output.warn call that formats package_name,
find_result.version.fmt(manifest.string_buf), and deprecated_msg (referencing
find_result.package.deprecated) so the deprecation message ends with a newline
and subsequent terminal output is not concatenated on the same line.
| // Print deprecation warning if package is deprecated | ||
| if (!find_result.package.deprecated.isEmpty()) { | ||
| const package_name = this.lockfile.str(&name); | ||
| const deprecated_msg = manifest.str(&find_result.package.deprecated); | ||
| Output.warn("<b>{s}@{f}<r><d>:<r> {s}", .{ | ||
| package_name, | ||
| find_result.version.fmt(manifest.string_buf), | ||
| deprecated_msg, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Missing newline in deprecation warning output.
The Output.warn call does not include a trailing newline, which may cause the next output to appear on the same line or cause formatting issues in the terminal.
Proposed fix
// Print deprecation warning if package is deprecated
if (!find_result.package.deprecated.isEmpty()) {
const package_name = this.lockfile.str(&name);
const deprecated_msg = manifest.str(&find_result.package.deprecated);
- Output.warn("<b>{s}@{f}<r><d>:<r> {s}", .{
+ Output.warn("<b>{s}@{f}<r><d>:<r> {s}\n", .{
package_name,
find_result.version.fmt(manifest.string_buf),
deprecated_msg,
});
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Print deprecation warning if package is deprecated | |
| if (!find_result.package.deprecated.isEmpty()) { | |
| const package_name = this.lockfile.str(&name); | |
| const deprecated_msg = manifest.str(&find_result.package.deprecated); | |
| Output.warn("<b>{s}@{f}<r><d>:<r> {s}", .{ | |
| package_name, | |
| find_result.version.fmt(manifest.string_buf), | |
| deprecated_msg, | |
| }); | |
| } | |
| // Print deprecation warning if package is deprecated | |
| if (!find_result.package.deprecated.isEmpty()) { | |
| const package_name = this.lockfile.str(&name); | |
| const deprecated_msg = manifest.str(&find_result.package.deprecated); | |
| Output.warn("<b>{s}@{f}<r><d>:<r> {s}\n", .{ | |
| package_name, | |
| find_result.version.fmt(manifest.string_buf), | |
| deprecated_msg, | |
| }); | |
| } |
🤖 Prompt for AI Agents
In `@src/install/PackageManager/PackageManagerEnqueue.zig` around lines 1420 -
1429, The deprecation warning printed in the PackageManagerEnqueue code uses
Output.warn without a trailing newline, so add a newline to the message format
(or use the Output method variant that appends a newline) for the Output.warn
call that formats package_name, find_result.version.fmt(manifest.string_buf),
and deprecated_msg (referencing find_result.package.deprecated) so the
deprecation message ends with a newline and subsequent terminal output is not
concatenated on the same line.
There was a problem hiding this comment.
Hmmm, it looked like to me that Output.warn uses prettyErrorln internally which already appends newline ?(src/output.zig:1039)
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the implementation of Output.warn in src/output.zig
# Looking around line 1039 and the warn function definition
# First, find the warn function definition
rg -n -A 15 'pub fn warn\(' src/output.zig
# Also check what prettyErrorln does
rg -n -A 5 'pub fn prettyErrorln\(' src/output.zig
Length of output: 217
🏁 Script executed:
#!/bin/bash
# Search for the warn function more broadly
rg -n -B 2 -A 10 'fn warn\(' src/output.zig
# Also search for "pub fn warn" or "pub inline fn warn"
rg -n -B 2 -A 10 '(pub |pub inline )fn warn' src/output.zig
Length of output: 991
@matthew2564 You're absolutely right! I apologize for the incorrect review comment.
Looking at src/output.zig:1140-1142, Output.warn does indeed call prettyErrorln, which appends a newline (the "ln" suffix). So your deprecation warning code is correct as-is and doesn't need the extra \n.
Thank you for catching that! 🙏
matthew2564
force-pushed
the
deprecation-warning-6883
branch
from
f1930fb to
176434e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/install/lockfile/Package/Meta.zig`:
- Around line 31-40: Change the is_deprecated enum default from .old to .false
and update the lockfile migration/check logic so .old is treated as a “needs
update” state; specifically, modify the Meta.is_deprecated declaration to
default to .false and update the needsUpdate() implementation to return true
when is_deprecated == .old (in addition to any other existing conditions),
ensuring freshly written lockfiles never persist .old while existing .old values
trigger migration.
♻️ Duplicate comments (1)
src/install/PackageManager/PackageManagerEnqueue.zig (1)
1420-1429: Missing newline in deprecation warning output.The
Output.warncall does not include a trailing newline, which may cause subsequent output to appear on the same line or cause formatting issues in the terminal.Proposed fix
// Print deprecation warning if package is deprecated if (!find_result.package.deprecated.isEmpty()) { const package_name = this.lockfile.str(&name); const deprecated_msg = manifest.str(&find_result.package.deprecated); - Output.warn("<b>{s}@{f}<r><d>:<r> {s}", .{ + Output.warn("<b>{s}@{f}<r><d>:<r> {s}\n", .{ package_name, find_result.version.fmt(manifest.string_buf), deprecated_msg, }); }
matthew2564
force-pushed
the
deprecation-warning-6883
branch
from
176434e to
c07fbb0
Compare
Parse the `deprecated` field from npm registry responses and display warnings during `bun install` when a package version is deprecated. Changes: - Add `deprecated` field to PackageVersion struct in npm.zig - Add `is_deprecated` flag to lockfile Meta struct for persistence - Print deprecation warnings when resolving new packages Example output: warn: request@2.88.2: request has been deprecated, see request/request#3142 warn: har-validator@5.1.5: this library is no longer supported
matthew2564
force-pushed
the
deprecation-warning-6883
branch
from
c07fbb0 to
4201220
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/install/npm.zig (1)
872-885: Bump the manifest cache version after changingPackageVersionlayout.Adding
deprecatedchanges the serializedPackageVersionsize; old manifest caches will be parsed with the new element size, which can truncatepackage_versionsand desync arrays. Please incrementPackageManifest.Serializer.versionto invalidate old caches.🛠️ Proposed fix
- pub const version = "bun-npm-manifest-cache-v0.0.7\n"; + pub const version = "bun-npm-manifest-cache-v0.0.8\n";
1 participant
Summary
Fixes #6883
Parses the
deprecatedfield from npm registry responses and displays warnings duringbun installwhen a package version is deprecated. This matches the behavior of npm, pnpm, and yarn.Example output:
Changes:
deprecatedfield toPackageVersionstruct innpm.zigto parse deprecation messagesis_deprecatedflag to lockfileMetastruct for persistenceTest plan
test/regression/issue/06883.test.tscovering: