JSON error on oro view react

[zkat]

For some bizarre reason, we get this error on oro view react:

Error: oro_client::bad_json (link)

  × Received some unexpected JSON. Unable to parse.
  ╰─▶ invalid type: boolean `false`, expected a sequence at line 1 column 344547
   ╭─[https://registry.npmjs.org/react:1:1]
 1 │ 098138909397134"},"_hasShrinkwrap":false},"0.0.0-f22621f88":{"name":"react","des
   ·                                         ▲
   ·                                         ╰── here
   ╰────

Which doesn't really make sense, since _hasShrinkwrap is defined as:

    #[serde(rename = "_hasShrinkwrap", skip_serializing_if = "Option::is_none")]
    pub has_shrinkwrap: Option<bool>,

And I looked at the React package and all of its _hasShrinkwrap fields are false, so it's not like we have some floating [] in the data.

[Gaelan]

I have a theory: the version in question (16.7.0) isn't the first one with _hasShrinkwrap, but it is the first one with a bunch of other miscellaneous keys:

      "_from": "react@0.0.0-4a1072194",
      "_id": "react@16.7.0",
      "_inBundle": false,
      "_integrity": "sha512-ZUj2lkUDLjwJaGu4WD0dYSvsfIyhQt2l/AJDlg4ij+rCDU3fSFKgHWanNovViUoaWHAxgrpft3KGFfvWPZH5LA==",
      "_location": "/react",
      "_phantomChildren": {},
      "_requested": {
        "type": "version",
        "registry": true,
        "raw": "react@0.0.0-4a1072194",
        "name": "react",
        "escapedName": "react",
        "rawSpec": "0.0.0-4a1072194",
        "saveSpec": null,
        "fetchSpec": "0.0.0-4a1072194"
      },
      "_requiredBy": [
        "#USER",
        "/"
      ],
      "_resolved": "https://registry.npmjs.org/react/-/react-0.0.0-4a1072194.tgz",
      "_shasum": "eca0e35d0d40fd15770de2e46073aeb654723443",
      "_spec": "react@0.0.0-4a1072194",
      "_where": "/Users/acdlite/Code/react/build/node_modules",

I suspect - with only circumstantial evidence, mind you - that serde_json is trying to parse _hasShrinkwrap as part of the _rest in Manifest, now that there's other stuff there. Still weird, though: _rest is defined as HashMap<String, serde_json::Value>, so there's no reason _hasShrinkwrap: false wouldn't be valid there.

[poliorcetics]

Commenting _rest definition does nothing for the error, I dont't think it's that

[cthulhua]

Hello, you successfully nerd-sniped me from my mastodon feed.

There are multiple things going on here:

1. The naughty field is Manifest's bundled_dependencies field. Per npm docs this can be a boolean, with true indicating that all dependencies are bundled and false indicating that none are. I have added a (failing) test demonstrating the failure to deserialize false bundle_dependencies in boolean_bundled_dependencies. I also confirmed by replacing the problematic manifest in question's false bundled_dependencies with [] and observing success. I'm not sure how this is preferred to be handled - Either<boolean, Vec<String>> and a boolean_or_vec_string fn to deserialize is one option, but I don't believe I saw Either in use in this crate. An explicit BundledDependencies enum with bool and Vec<String> variants is another option.
2. The diagnostic leads one to believe that the _hasShrinkwrap property on the package is where the problem comes from, however, this is not the case, it's the Manifest in VersionMetadata. I'm unsure as to why a misleading diagnostic is emitted by serde.

I'm perfectly happy to fix this particular issue and submit a PR (well, updating the bundled_dependency type for this variant, idk about the serde thing), just lmk how you would prefer this resolved.

[larsgw]

The diagnostic leads one to believe that the _hasShrinkwrap property on the package is where the problem comes from, however, this is not the case, it's the Manifest in VersionMetadata. I'm unsure as to why a misleading diagnostic is emitted by serde.

From the limited tests I did, the cursor/position of the error is usually the end of the value (that's what I saw in a smaller example, at least). In this case the cursor is on the }, not the e. I also noticed that serde_path_to_error reports versions.16.7.0 as opposed to versions.16.7.0._hasShrinkwrap. All in all it seems to be pointing to the whole version object itself, in a way that is consistent but very confusing. I don't know why it can't point to bundleDependencies though, maybe because of the flatten?

[zkat]

@cthulhua a dedicated BundledDependencies enum is my preference

[zkat]

This is now fixed and will be included in the next release (0.3.33 and up)