Expose the concept of logos, add SimpleIcons as a peer dependency

[chris48s]

:clipboard: Description

Expose logo and logo-specific params (e.g: logoColor, logoWidth). There are 3 cases for logos:

• Named SimpleIcons logo
• Custom base64-encoded logo
• Named shields custom logo https://github.com/badges/shields/tree/master/logo

At a library level it probably makes sense to reduce that to two cases - SimpleIcons and base64. We can keep our handful of custom logos outside of badge-maker and just treat them as something we handle in server somewhere and pass to the renderer as custom base64.

This will allow us to remove final relative import to the package requiring anything other than makeBadge() https://github.com/badges/shields/blob/8621fe42d76b809e9ba4ae844920f8ed0373817a/lib/logos.js#L4

#4524 never happened, but there are some notes on testing code with peer dependencies in https://github.com/badges/shields/pull/4524#discussion_r365603567

This work should also remove all/most of the remaining code from https://github.com/badges/shields/tree/master/lib (and if there is anything left, we should work out where to move it to)

[paulmelnikow]

At a library level it probably makes sense to reduce that to two cases - SimpleIcons and base64. We can keep our handful of custom logos outside of badge-maker and just treat them as something we handle in server somewhere and pass to the renderer as custom base64.

That's an interesting thought! Though it would have a downside of the logos not being available to package users (i.e. the package and the service supporting different sets of logos).

[paulmelnikow]

Though I'd pick this up and see if we can keep closing the gap toward dogfooding our own package.

I think there's one question we should resolve first, which is whether to handle the same three cases (simpleicons, custom base64, and shields logos) in the package, or handle simpleicons and base64 in the package and keep the responsibility for the shields logos with the server.

I'd lean toward supporting all three in the package, so that the service and the package support the same set of logos. If users are accustomed to using certain logos on the server, having the package support a smaller set seems like it would be confusing.

[chris48s]

Some semi-random thoughts on this topic that don't necessarily all agree with each other..

• If you think of the package as being something that helps you build your own shields.io then yes its odd if the package and service behave differently, but I tend to assume that library users want to do something outside the scope of what using shields.io directly or self-hosting an instance can do for them rather than exactly replicate it.

• If you think of a package user as someone who is just building their own application, its maybe more confusing if you add SimpleIcons to your dependency tree, but some of the named logos from SimpleIcons can't be accessed and get overridden by a weird custom one you weren't expecting, which is what happens on shields.io.

• For the same reason we would make SimpleIcons a peer dependency rather than a required dependency, I probably wouldn't want to ship a bunch of logos to users by default (although I do accept there is a big scale difference in the size of the collections).

• All 11 of the 11 custom icons we have do also exist in SimpleIcons - they're overrides not extra icons

• One other thing to consider at the library level is if you use SI as a dependency, it would be really nice if the actual SI slugs work. Obviously we painted ourselves into a corner by making up our own and we're just stuck with that now, but if you're using SI as an NPM lib and badge-maker as an NPM lib, we should probably think about whether we can make them interop in the obvious way. (I've done no investigation on how easy/hard that would be)

• Another completely different approach we could take would be to allow base64 and not directly support any named logos or SimpleIcons - just be totally library-agnostic: Use SimpleIcons, FontAwesome, OctoIcons, Noun Project.. whatever you want, but you're responsible for encoding it (or maybe provide a helper function?). This is more like how core currently interacts with badge-maker anyway. Also, back in v2.x of the lib (the BadgeFactory era), we never documented it (or really intended it tbh :grimacing: ) but because we didn't fully validate the input object, you could pass extra undocumented params through to the internal rendering function, so you actually could use it like that and I've seen code out there that (ab)used that property e.g:

  • https://github.com/tejashah88/devpost-shields/blob/master/shield-delivery/src/gen-badge.js
  • https://github.com/elias551/postman-tests/blob/master/scripts/createBadge.js

  One of the advantages of this approach would be that if SI becomes a peer dependency, our test matrix becomes more complex because we need to run the package tests with SI installed and without SI installed to verify it works properly in both situations (and in the local dev context we'll never run without SI installed so we'll always forget that case). If we just support base64 in the library, we could keep that simpler and it doesn't actually meaningfully restrict the developer of a custom application from using whatever logos/images they want from any library. A library user has way more scope to plug-and-play their own solutions than a SaaS user does.

[bilalq]

Just chiming in here:

We have a use-case where we're basically hope to host badge-maker in a Lambda and invoke it directly in our CI/CD pipeline flows. We don't want the hassle of having to run and operate the whole shields service, so a serverless setup here is very attractive. I know the team here has previously discussed why serverless isn't optimal from a cost perspective for the hosting of https://shields.io, but for self-hosted use-cases of a smaller company like ours that would only generate a couple hundred requests for badge creation a day, it makes a lot of sense.

To that end, it was kind of surprising when I went to go spin this up and found that badge-maker doesn't have parity with shields.io's featureset. I'd love to see logos (particularly custom logo support) included in badge-maker.

[calebcartwright]

@bilalq - could you provide some more context around your use case? are you operating within a restricted networking environment?

Admittedly not much detail in your message but making https requests to some api/endpoint which internally utilizes our npm package doesn't sound like a terribly efficient way of getting badges and I suspect there might be some better options for you. The topic of badges within pipelines has been discussed at length in various other issues (not particularly relevant to this issue), and there's a number of viable strategies members of the community have employed, including our Endpoint badge and our Dynamic badge which consumes json/xml artifacts generated during the pipeline process and published somewhere.

Finally, while this is admittedly inherently subjective, I don't personally agree with the characterization of running the Shields server as a "hassle", speaking both as a member of the ops team for Shields.io and as someone that runs a self-hosted Shields server at my day job. We distribute the server as a docker image that can quickly and easily be deployed in a number of environments, both on lower level compute environments as well as higher level cloud services that support running containers

[bilalq]

I wasn't aware of the endpoint badge, that's really neat!

However...

https requests to some api/endpoint which internally utilizes our npm package doesn't sound like a terribly efficient way of getting badges

We're not making HTTPS requests manually, but rather, leveraging some of the many integrations that exist with AWS Lambda. As an example, AWS CodePipelines can have an approval step defined as just invoking a lambda and using treating the output of that Lambda as a build artifact that gets saved to S3. We can expose these badges by simply having a CloudFront distribution pointing to S3 as an origin. We use the CloudFront URL anywhere we want to display the badge and the content will auto-update anytime the origin updates as a result of a pipeline step run.

There are security/confidentiality benefits to self-hosting shields, and being able to more easily host it in a lightweight serverless capacity using Functions-as-a-Service just makes it easier and cheaper.

Unrelated to all this: I wanted to utilize badge-maker for a complex scenario of pipeline visualization. A pipeline can be thought of as a series of stages that run in serial with each stage containing several steps that can be a mixture of parallel and serial. If we handwave away presentation of the parallel flows, then a pipeline can be visualized as 2D grid. Each stage can be a column and the rows within the stage can have badges. Generating a single SVG of that 2D grid would enable easy embedding of the full pipeline state in any view. I planned to use badge-maker as a baseline to generate that bigger SVG, so having the functionality in shields would be useful.

Finally, while this is admittedly inherently subjective, I don't personally agree with the characterization of running the Shields server as a "hassle", speaking both as a member of the ops team for Shields.io and as someone that runs a self-hosted Shields server at my day job. We distribute the server as a docker image that can quickly and easily be deployed in a number of environments, both on lower level compute environments as well as higher level cloud services that support running containers

I certainly didn't mean this as a dig at shields. My sincere apologies if it came across like that in anyway. You and the rest of the team working on it have built an amazing product and done a great job at documenting and making it easy to run for others.

To clarify what I meant: I consider any service with more complexity than can be put in a Functions-as-a-service runtime a hassle to run (in comparison). Services in Lambda can easily have multi-AZ and multi-region redundancy and auto-scaling based on load. Even with all that redundancy and operational simplification, the costs to run will be less than 1 cent per month. You can definitely achieve a setup with equivalent or possibly better reliability running Kubernetes or whatever, but it'll take a lot more effort and cost way more.

[calebcartwright]

Thanks @bilalq. As the above history of this thread highlights, there's a consensus on the general theme of supporting logos through the exported interface of the package, it's just a matter of discussing and identifying our target solution, and then implementing. This was teed up because we know it's important for various use cases, both internal and external, so thanks for sharing that this would be useful to you as an external consumer of the package.

However, I feel like we're veering pretty far into orthogonal subjects and I don't want this thread to be taken any more off topic. You'll find that both the maintainer team and the community at large are happy to share opinions, provide guidance, participate in dialog, etc. but if you'd like any recommendation from us and/or a response on some of the more philosophical cloud concepts, self-hosting strategies, etc. I'd ask that you open a new Discussion item so that we can keep the discussion here focused on the specific topic about the package API surface for logos.

[bilalq]

Yeah, sorry for going off on a tangent. Upon digging into the codebase a bit deeper, I see that all my needs can actually be achieved today by using the "non-public" makeBadge function imported from badge-maker/lib/make-badge'. That said, it'd be neat make that part of the "public" interface.

As a library consumer, I'd be perfectly happy with the base64 only approach suggested by @chris48s. That would let the consumer use whatever icon sets they want without unnecessary bloat. Even if I try to put myself in the shoes of someone running a shields server, the library would ultimately be resolving those to either base64 data or a raw SVG element string anyway, right? Support for logoColor would be cool to have in here, but I can see how it's tricky in the case of non-monochrome svgs.

This is my understanding of the issue:

In Scope:

• Support logo and logoWidth as passthrough properties in the main exported makeBadge function (i.e., don't exclude them in the _clean helper function.
• Update typings and tests

Uncertain:

• Is logoPosition actually used or can it be removed from badge-maker? None of the renderers seem to use it. I do see some references to it in badge-urls and base-service. Not sure if those are related though.
• It may be nice if logo could actually just be a raw SVG string instead of needing to do a base64 encode. I don't really feel strongly either way though.
• Adding a logoColor property that uses the same logic as shields/lib/logos.js could work, but it might make non-monochrome look weird. And there's a good argument to be made that logo transformation is not a core badge rendering concern.

Out of Scope:

• Updating shields to use this newer interface is something that can be left for #4950.

Assuming you're okay with this approach and can clarify the scope, I'd be happy to send a PR to address this.

[Abhi347]

Do we have any update on this? I was looking for adding a custom logo for the badges and decided to use the non-public method too. It would be nice to expose these logo params in the types, at least not failing the validation.
If we agreed on this, I can create a PR to resolve this part at least.