[npm] - Last update badge added

[MohanKumarAmbati]

Hi Team,

Extending NpmBase points registry URL to https://registry.npmjs.org/express/latest - latest, which do not include last updated time. Hence, extended BaseJsonService to fetch the json.

closes - #10023

[github-actions]

	Messages
📖	✨ Thanks for your contribution to Shields, @MohanKumarAmbati!

Generated by 🚫 dangerJS against 93951a3

[jNullj]

also this.

[jNullj]

Finished a review round, i have some inputs.

I think we could benefit from using buildRoute but i could not find an elegant way to use it while adding version queryparam, let me know if you have an idea.
But i think that we could also just define the route in the service.

I noticed scoped packages failed, notice my change requests that fix that, maybe we should add a test for scoped package as well.

[MohanKumarAmbati]

I think we could benefit from using buildRoute but i could not find an elegant way to use it while adding version queryparam, let me know if you have an idea. But i think that we could also just define the route in the service.

Just thinking whether we can do something like taking queryparam as input, if queryParamSchema: queryParams || queryParamSchema, we'll give default queryparam.

[MohanKumarAmbati]

I think we could benefit from using buildRoute but i could not find an elegant way to use it while adding version queryparam, let me know if you have an idea.

Other way - 96ced45, introduced new additionalQueryParamSchema, if it is not null then it will be appended to the existing queryParamSchema. I think it would be better way, other packages if any can utilize as well.

[chris48s]

Just quickly unwinding this.. what problem does the ?version= param solve here?

AFAIK a npm version is immutable. Once you've pushed a release with a given version number you can't push that version again. So the "last updated" timestamp for a specific version is static. It is a value that isn't going to change (unlike "last updated" for the package itself or a tag, which will change over time as versions are published).

[MohanKumarAmbati]

Just quickly unwinding this.. what problem does the ?version= param solve here?

This param helps to fetch last updated for a given version in the package.

Also yeah it won't change 🙂. As the data is available, I thought of giving it like an option to fetch last updated for a particular version. Thought it would be good to have one.

[MohanKumarAmbati]

Just quickly unwinding this.. what problem does the ?version= param solve here?

@chris48s,

just tried to include versions somehow, gave it a second thought and looks like not much useful. Hence, removed it.

@jNullj, can you please review changes.

[mbtools]

"Last update" by version is not very useful, indeed. However, it would be good to get the last update for a given dist-tag esp. for projects that keep several active release branches.

See Verdaccio, for example.

[jNullj]

"Last update" by version is not very useful, indeed. However, it would be good to get the last update for a given dist-tag esp. for projects that keep several active release branches.

See Verdaccio, for example.

Makes sense, @MohanKumarAmbati can you add tags to your fetch and enable tags in path? (also openapi and tests)

[MohanKumarAmbati]

Thanks for the input @mbtools, but looks like they're not exposing last updated time in the API verdaccio - next-8. please let me know if there is an API that we can make use of.

[mbtools]

After you fetch the manifest (https://registry.npmjs.org/verdaccio), lookup the dist-tag and get the associated version:

Then use that version to get the time:

For next-8, the time would be 2024-10-08T06:34:29.911Z.

[MohanKumarAmbati]

Thanks for the input @mbtools, made changes accordingly - d7cf23c.

[MohanKumarAmbati]

Hi @chris48s, can you please add hacktoberfest-accepted label to this PR. As we're end of the month, post which my contributions won't be considered under that and adding this label might help. I'll work on my changes till all the review comments are done and PR is good.

[MohanKumarAmbati]

Thank you @chris48s

[jNullj]

Two more small things, but overall seems alright.

[jNullj]

tag in packageData['dist-tags'] I think we could improve a bit if we test the array once and use the result twice. I would expect this to be a long array for some packages.

[jNullj]

LGTM, Thank you @MohanKumarAmbati for the contribution.

[chris48s]

Thanks for your work on this @MohanKumarAmbati , and for taking the review @jNullj

Just having a quick look over this I have a couple of additional comments.

[mbtools]

nice work. thanks @MohanKumarAmbati 🙏

[github-actions]

🚀 Updated review app: https://pr-10641-badges-shields.fly.dev

[chris48s]

Thanks. Good work all. I'm going to merge this.

One thing I have realised about this badge is that it will work fine for most packages, but the https://registry.npmjs.com/<packagename> response can get really big for some packages. For example, https://registry.npmjs.com/npm is 22Mb of JSON

Shields has a hard limit on the size of response we will accept from an API. We cap it at 10Mb so for some subset of packages this badge will not work.

I think that's just something we live with. In reality the number of packages that actually generate such a huge response (>10Mb) is quite small.

[mbtools]

The typescript package is another one (13 MB). You could get the abbreviated manifest first (https://github.com/npm/registry/blob/main/docs/responses/package-metadata.md). That has last-modified time already and the tags. Then if a tag was selected, get the individual version json instead of the complete manifest.

[jNullj]

That's a huge difference.

$ curl -H "Accept: application/vnd.npm.install-v1+json" https://registry.npmjs.org/npm > test-abbreviated.json
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 2194k  100 2194k    0     0  25.7M      0 --:--:-- --:--:-- --:--:-- 26.4M

$ curl https://registry.npmjs.org/npm > test.json
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 21.1M  100 21.1M    0     0  48.4M      0 --:--:-- --:--:-- --:--:-- 48.4M
$ ls -la --block-size=MB test*
-rw-r--r-- 1 user 197609 23MB Nov  2 23:34 test.json
-rw-r--r-- 1 user 197609  3MB Nov  2 23:34 test-abbreviated.json

From the test-abbreviated we can get next-7: "7.24.2"
then in theory you could call https://registry.npmjs.com/npm/7.24.2 or https://registry.npmjs.com/npm/next-7 (maybe call next-7 without the first step as well)
But the response from /package/version does not seem to include the any timestamp... so im not sure we could use it here.

The only thing i think is useful with abbreviated response is when we don't use a tag.

Altho, i think i just found a trick ;)
Notice how this part of the abbreviated response file name is picked? looks like a timestamp

"_npmOperationalInternal":{
  "tmp":"tmp/npm_7.24.2_1633367792820_0.6414313657198436",
  "host":"s3://npm-registry-packages"
}

163336779282 -> Monday, 4 October 2021 17:16:32.820
And full package metadata response returns
7.24.2: "2021-10-04T17:16:33.168Z"

Not sure if we can trust this, but it sure looks like a cool party trick

[chris48s]

Making 2 requests in sequence has its own performance impact. I think a reasonable solution would be:

• If the badge URL is /npm/last-update/{packageName} request the abbreviated version
• If the badge URL is /npm/last-update/{packageName}/{tag} request the full version

That would be a huge improvement for the most common case.

I think we should stick to relying on the documented API behaviour. Relying on undocumented assumptions about API internals generally ends up biting us in the long run.