[NpmUnpackedSize] Unpacked Size Badge #9954

erayerdin · 2024-02-07T12:05:28Z

github-actions · 2024-02-07T12:06:56Z

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

Generated by 🚫 dangerJS against 25c5207

erayerdin · 2024-02-07T12:49:52Z

Some tests for NPM apart from what I've added fail oddly. I think it's due to requiring a token, maybe?

services/npm/npm-unpacked-size.service.js

github-actions · 2024-02-11T14:29:27Z

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

chris48s · 2024-02-11T14:38:54Z

services/npm/npm-unpacked-size.service.js

+      message: unpackedSize ? prettyBytes(unpackedSize) : 'unknown',
+      color: unpackedSize ? 'blue' : 'lightgray',


Can you provide any detail on when this value would or would not be populated in the response?

Express 4.18.2 Registry Endpoint

Express 4.15.0 Registry Endpoint

4.15.0 does not have dist.unpackedSize while 4.18.2 or latest has it.

I think the field is populated when someone publishes a package on NPM, so some older versions of the same package (especially when the package is old) does not have dist.unpackedSize.

chris48s · 2024-02-11T14:39:41Z

OK. the tests that are failing are the badges that get stats from npm-stat.com Looks like that service may have gone away. https://npm-stat.com/

This is unrelated to this PR, so lets ignore that for now. Lets move that issue to #9964 . I've changed the PR title so we only run tests for the service added in this PR.

chris48s · 2024-02-11T14:40:23Z

services/npm/npm-unpacked-size.service.js

+
+  static route = {
+    base: 'npm/unpacked-size',
+    pattern: ':packageName/:version*',


The badge should accept a scoped or unscoped package name. Can you

Fix the route + code (route should be scope(@[^/]+)?/:packageName/:version* )

Add service tests for scoped package with and without version

Completed. Resolve if okay.

chris48s · 2024-02-11T14:40:39Z

services/npm/npm-unpacked-size.tester.js

+
+t.create('Latest unpacked size')
+  .get('/express.json')
+  .expectBadge({ label: 'unpacked size', message: /.* kB/ })


Can we use the isFileSize test validator for the valid tests instead of writing a custom regex please

Wasn't aware it existed, I've checked other npm services and bundlephobia service for my code. Will do it.

Completed. Resolve if okay.

chris48s · 2024-02-11T14:41:27Z

services/npm/npm-unpacked-size.service.js

+
+  async fetch({ packageName, version }) {
+    return this._requestJson({
+      schema: Joi.any(),


Instead of using Joi.any() here, can you write a schema that ensures the data we need exists in the response and is of the right type. There's some docs on input validation in https://github.com/badges/shields/blob/master/doc/input-validation.md . Not tested, but I guess either

Joi.object({ dist: Joi.object({ unpackedSize: nonNegativeInteger.required(), }).required() }).required()

or

Joi.object({ dist: Joi.object({ unpackedSize: nonNegativeInteger }).required() }).required()

will be what we want here, depending on how we want to handle the case where unpackedSize does not exist. One option would be to accept a missing value and handle it in the code (like you've done). the other option would be to reject the response at the validation stage. I guess that depends on the answer to #9954 (comment)

Could not complete, refer to the review: #9954 (comment)

chris48s · 2024-02-11T15:03:39Z

Thanks for working on this btw. There's a few issues to iron out, but it is a good start 👍

erayerdin · 2024-02-12T20:30:20Z

services/npm/npm-unpacked-size.tester.js

+
+t.create('Nonexistent unpacked size with version')
+  .get('/express/4.16.0.json')
+  .expectBadge({ label: 'unpacked size', message: 'unknown' })


After adding schema to validate NPM registry response body, this test fails saying this:

5) NpmUnpackedSize [live] Nonexistent unpacked size with version [ GET /express/4.16.0.json ]: label mismatch + expected - actual -size +unpacked size

When I manually requested the local dev endpoint, I've seen the badge as such:

(could not paste the PNG of it as it's not available in the dev server)

I expected the label to be unpacked size instead of only size. It seems like, when the schema validation fails, it renders the label as size, which, I think, is a built-in mechanism.

I think when dist.unpackedSize is not defined, the label should still be unpacked size to imply that this is not any kind of other size such as minified or min+gzipped.

@chris48s, do you have any idea as to how to override the default badge label and message when dist.unpackedSize does not exist? I've tinkered a little bit with the codebase but I could not see any way (maybe I've missed it or haven't realized).

Actually, I've checked it through BaseJsonService._requestJson method, which calls BaseService._validate. BaseService._validate has positional options parameter, in which, prettyErrorMessage has been set to "invalid response data".

All the way up the stack (from NpmUnpackedSize service), I cannot set prettyErrorMessage because it is internal to BaseJsonService and BaseService. Given that BaseJsonService._requestJson and BaseService._validate are internals (prepended by underscore), I naturally hesitate to change them.

Thus, there are two possible paths here:

Modify BaseService._validate and BaseJsonService._requestJson methods so that I can reach and modify the message (and thus, the label) when dist.unpackedSize does not exist. Given that these methods are defined as internals and tons of services rely on their signature, this might/would cause enormous amount of failures on some tests in the best case, or unexpected behavior on runtime at worst.

Accept the sweet fate and let the badge stay as size: invalid response data. I thought it wouldn't be a viable solution as (given that JS is used in web) it is important to differ unpacked size from minified or min+gzipped sizes but there's no sane room here. So, I can only modify the test to satisfy size: invalid response data badge.

@chris48s What's your idea?

Ah right - sorry. In my original post, the suggestions should have been

optionalNonNegativeInteger and nonNegativeInteger
not
nonNegativeInteger and nonNegativeInteger.required()

🤦

if we switch to using optionalNonNegativeInteger for the unpackedSize key, then you should be able to carry on doing what you were doing and render "unknown" if the unpackedSize key is missing.

The validation is still useful, because it ensures the response has a dist object and that unpackedSize is of the right type if it exists.

If you want to set the label as "unpacked size" even when an exception is thrown, setting

static defaultBadgeData = { label: 'unpacked size' }

will do the job.

Does that unstick you?

Thanks, did it. We definitely need documentation about these premade validators, missed it somehow. That's maybe the topic of another PR.

With that in mind, seems like this is completed.

The validators are included in the internal api docs here: https://contributing.shields.io/module-services_validators.html

…eger

erayerdin · 2024-02-14T07:19:44Z

services/npm/npm-unpacked-size.service.js

+            example: '4.18.2',
+          }),
+          queryParam({
+            name: 'registry_uri',


Also added a registry_uri query param as they were existent on the other NPM-based services.

chris48s · 2024-02-14T19:23:25Z

services/npm/npm-unpacked-size.service.js

+
+  async handle(namedParams, queryParams) {
+    const { scope, packageName, tag, registryUrl } =
+      this.constructor.unpackParams(namedParams, queryParams)


I think you had this right first time. We now seem to be mixing the concepts of version and dist tag.
For some badges, we use a tag param, but that's not the same as a version. So for example

latest version of npm package: https://img.shields.io/npm/v/npm
latest version of npm package with dist-tag "next-8": https://img.shields.io/npm/v/npm/next-8

but its not the same as a version. What you're actually implementing here is a version param. i.e: the unpacked size of a specific version of this package. The thing we're accepting here isn't a dist tag.

I'd say just list the actual params in the signature of handle() rather than use the wrong helper.

Welp, I confused that. Sorry. Alright, let me get to reimplement version back.

Completed. Reverted back to using version instead of tags.

github-actions · 2024-02-19T09:28:37Z

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

chris48s

cool - latest changes LGTM 👍

erayerdin added 7 commits February 6, 2024 01:29

init npm-unpacked-size service

fca445a

add sample badge

d1b5ada

fetch unpacked size from latest version

b5da854

format unpacked size

e8aff1e

parametrize unpacked size by package name

9bb8d19

add optional version parameter

f87c9d2

fix typo on test

1457c8f

erayerdin added 6 commits February 7, 2024 15:14

rename test to tester

926320d

test against json endpoint instead of svg

53b6dd4

test and impl version with undefined unpacked size

1439bcf

test version with defined unpacked size

49fa937

change color to lightgray when unpackedSize is undefined

8d4712a

add openapi docs

75f9ccb

erayerdin marked this pull request as ready for review February 7, 2024 12:48

erayerdin commented Feb 8, 2024

View reviewed changes

services/npm/npm-unpacked-size.service.js Outdated Show resolved Hide resolved

extend NpmBase instead of BaseJsonService

908d2c3

chris48s changed the title ~~[NPM] Unpacked Size Badge~~ [NpmUnpackedSize] Unpacked Size Badge Feb 11, 2024

Merge branch 'master' into feat/npm-unpacked-size-badge

8f52509

chris48s reviewed Feb 11, 2024

View reviewed changes

chris48s added the service-badge New or updated service badge label Feb 11, 2024

erayerdin added 3 commits February 12, 2024 22:37

Merge branch 'master' into feat/npm-unpacked-size-badge

7c3a727

use isFileSize validator

bb14a63

add schema to validate npm registry response body

8f0d4c0

erayerdin commented Feb 12, 2024

View reviewed changes

erayerdin added 4 commits February 13, 2024 15:52

add tests for scoped packages

35e3799

impl scoped package route

e8d5684

change the type of schema.dist.unpackedSize to optionalNonNegativeInt…

b402fe2

…eger

add registry_uri query param

1449d4c

erayerdin mentioned this pull request Feb 13, 2024

Up/Down Badge works when site is up - not when down #9844

Closed

erayerdin added 2 commits February 14, 2024 02:14

add default label to badge

8f18b78

unpack tag instead of version

488eb2a

erayerdin commented Feb 14, 2024

View reviewed changes

chris48s reviewed Feb 14, 2024

View reviewed changes

revert back to versions instead of tags

25c5207

chris48s approved these changes Feb 19, 2024

View reviewed changes

chris48s added this pull request to the merge queue Feb 19, 2024

Merged via the queue into badges:master with commit 503764e Feb 19, 2024
22 checks passed

erayerdin deleted the feat/npm-unpacked-size-badge branch February 19, 2024 10:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[NpmUnpackedSize] Unpacked Size Badge #9954

[NpmUnpackedSize] Unpacked Size Badge #9954

erayerdin commented Feb 7, 2024

github-actions bot commented Feb 7, 2024 •

edited

Loading

erayerdin commented Feb 7, 2024

github-actions bot commented Feb 11, 2024

chris48s Feb 11, 2024

erayerdin Feb 12, 2024

chris48s commented Feb 11, 2024 •

edited

Loading

chris48s Feb 11, 2024

erayerdin Feb 13, 2024

chris48s Feb 11, 2024

erayerdin Feb 12, 2024

erayerdin Feb 13, 2024

chris48s Feb 11, 2024

erayerdin Feb 13, 2024

chris48s commented Feb 11, 2024

erayerdin Feb 12, 2024 •

edited

Loading

erayerdin Feb 13, 2024 •

edited

Loading

chris48s Feb 13, 2024

erayerdin Feb 13, 2024

chris48s Feb 14, 2024

erayerdin Feb 14, 2024

chris48s Feb 14, 2024

erayerdin Feb 14, 2024

erayerdin Feb 16, 2024

github-actions bot commented Feb 19, 2024

chris48s left a comment

		message: unpackedSize ? prettyBytes(unpackedSize) : 'unknown',
		color: unpackedSize ? 'blue' : 'lightgray',

[NpmUnpackedSize] Unpacked Size Badge #9954

[NpmUnpackedSize] Unpacked Size Badge #9954

Conversation

erayerdin commented Feb 7, 2024

github-actions bot commented Feb 7, 2024 • edited Loading

erayerdin commented Feb 7, 2024

github-actions bot commented Feb 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chris48s commented Feb 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chris48s commented Feb 11, 2024

erayerdin Feb 12, 2024 • edited Loading

Choose a reason for hiding this comment

erayerdin Feb 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Feb 19, 2024

chris48s left a comment

Choose a reason for hiding this comment

github-actions bot commented Feb 7, 2024 •

edited

Loading

chris48s commented Feb 11, 2024 •

edited

Loading

erayerdin Feb 12, 2024 •

edited

Loading

erayerdin Feb 13, 2024 •

edited

Loading