Skip to content

fix(npm): handle boolean-form deprecated field - #48

Merged
andrew merged 1 commit into
git-pkgs:mainfrom
acidghost:fix/npm-deprecated-boolean
Jul 27, 2026
Merged

fix(npm): handle boolean-form deprecated field#48
andrew merged 1 commit into
git-pkgs:mainfrom
acidghost:fix/npm-deprecated-boolean

Conversation

@acidghost

Copy link
Copy Markdown
Contributor

Problem

FetchVersions / FetchPackage fail with a JSON unmarshal error for packages whose versions publish deprecated as a boolean instead of a string:

json: cannot unmarshal bool into Go struct field versionInfo.versions.deprecated of type string

react triggers this — several of its versions publish deprecated: false.

Reproducer

# Public registry: react has versions with deprecated as a boolean
curl -s https://registry.npmjs.org/react \
  | jq -c '.versions | to_entries[] | select(.value.deprecated | type=="boolean") | {version: .key, deprecated: .value.deprecated}'

returns:

{"version":"16.7.0","deprecated":false}
{"version":"16.8.0-alpha.0","deprecated":false}
{"version":"0.14.10","deprecated":false}
{"version":"15.7.0","deprecated":false}
{"version":"16.14.0","deprecated":false}

so FetchVersions(ctx, "react") errors out instead of returning versions.

Root cause

The npm packument spec defines deprecated as a string (the deprecation message), but some packuments emit a boolean instead (false = "not deprecated", true = "deprecated with no message"). versionInfo.Deprecated is typed string, so the boolean form fails the entire packageResponse decode.

Unlike the other polymorphic npm fields, deprecated drives logic:

if v.Deprecated != "" {
    status = core.StatusDeprecated
}

so simply switching to interface{} is not enough.

Fix

Introduce a deprecatedField string type with a custom UnmarshalJSON that accepts a string, boolean, or null and normalizes to the string form the existing logic expects:

  • string message → kept verbatim (non-empty → StatusDeprecated)
  • true"true" (deprecated)
  • false / null"" (not deprecated)

This preserves the non-empty check driving StatusDeprecated, so boolean-false versions stay active while string / true versions stay deprecated.

Testing

Added TestFetchVersions_DeprecatedShapes covering absent, string-message, boolean-false, and boolean-true forms, asserting both the resulting Version.Status and the Version.Metadata["deprecated"] value.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves the npm registry client’s robustness by allowing FetchVersions / FetchPackage to decode npm packuments where a version’s deprecated field is emitted as a boolean (a real-world deviation from the spec), preventing JSON unmarshal failures for packages like react.

Changes:

  • Introduces a deprecatedField type with custom UnmarshalJSON to normalize deprecated values across string/boolean/null shapes.
  • Updates versionInfo.Deprecated to use deprecatedField so packument decoding no longer fails on boolean forms.
  • Adds a unit test covering multiple deprecated shapes and asserting resulting Version.Status and metadata.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
internal/npm/npm.go Adds deprecatedField JSON decoding to accept boolean/null and normalizes to a string-like value used by existing deprecation logic.
internal/npm/npm_test.go Adds test coverage for deprecated field shape variations during FetchVersions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/npm/npm_test.go
Comment on lines +165 to +169
// TestFetchVersions_DeprecatedShapes verifies that the "deprecated" field is
// handled across the shapes npm packuments emit: absent/null, a string
// message, and the legacy boolean form (false == not deprecated,
// true == deprecated with no message). String and true values must mark the
// version StatusDeprecated; absent/null/false must leave it active.
Comment thread internal/npm/npm_test.go Outdated
Comment on lines +239 to +241
if got := string(v.Metadata["deprecated"].(deprecatedField)); got != wantDep[num] {
t.Errorf("%s deprecated metadata = %q, want %q", num, got, wantDep[num])
}

@andrew andrew left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this — the fix and test coverage look good.

Two things before merge:

  1. This now conflicts with main after #47 landed (both PRs insert tests at the same spot in npm_test.go and touch adjacent lines in the versionInfo struct). Could you rebase?
  2. Optional: at the Metadata assignment, consider "deprecated": string(v.Deprecated) so the map keeps holding a plain string as before rather than the named deprecatedField type. Nothing currently type-asserts it, so not blocking.

The npm packument spec defines a version's `deprecated` field as a string
carrying the deprecation message, but some packuments emit a boolean
instead (false meaning "not deprecated", true meaning deprecated with no
message). The struct typed it as `string`, so unmarshalling failed with
"cannot unmarshal bool into Go struct field versionInfo.versions.deprecated
of type string" and FetchVersions returned an error for the whole package
(e.g. react, which has 5 versions with `deprecated: false`).

Introduce a `deprecatedField` string type with a custom UnmarshalJSON that
accepts a string, boolean, or null and normalizes them to the string form
the rest of the code relies on. Strings are kept verbatim; `true` maps to
"true" (deprecated); `false` and null map to "" (not deprecated). This
preserves the existing non-empty check that drives StatusDeprecated, so
boolean-false versions stay active while string/true versions stay
deprecated.

Adds TestFetchVersions_DeprecatedShapes covering absent, string-message,
boolean-false, and boolean-true forms.
@acidghost
acidghost force-pushed the fix/npm-deprecated-boolean branch from 3407fc1 to 2bcf06a Compare July 26, 2026 18:31
@acidghost
acidghost requested a review from andrew July 26, 2026 18:31

@andrew andrew left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the rebase and the metadata cast — LGTM.

@andrew
andrew merged commit 0b4dcab into git-pkgs:main Jul 27, 2026
2 checks passed
@acidghost
acidghost deleted the fix/npm-deprecated-boolean branch July 27, 2026 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants