Refactor named logo handling into badge-maker

[jNullj]

• moved /lib helper files into badge-maker as they seem more relevant there now.
• handle named logo size and base64 conversion in badge-maker

TODO

• update dependencies (make named logo optional)
• update tests to fit the refactor
• expose named logo/named logo color params in badge maker
• Should i present error on badge if badge-maker has named logo but not the dependency or only backend error like now?
• Should we test both with and without the dependency when running tests?

[github-actions]

	Warnings
⚠️	This PR modified package.json, but not package-lock.json - Perhaps you need to run npm install?

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

Generated by 🚫 dangerJS against ae466e9

[jNullj]

I would like some help regarding my next step.
I did not upload all new commits i made yet but it boils down to this:

1. I moved simple-icons utils from /libs into /badge-maker/simple-icons-utils, the logic being that we will now only use simple-icons directly in shields only for tests, making it dev only dependency.
2. badge-maker is CJS
3. the utils is ESM
   So should i change the utils to CJS, are we keeping badge-maker as CJS for backwards compatibility, should i mix ESM and CJS (i hope not, as its harder to add esm to cjs) or should I update badge-maker to ESM? which might cause other compatibility issues for users of the package...

@chris48s

[chris48s]

When we originally migrated the main codebase from CommonJS to ESM, ESModules was still quite new. There were a lot of projects still using CommonJS. At that point in time, it wasn't really reasonable for us to move badge-maker to ESM just yet. Today, things have moved on. That is not really true and it is entirely reasonable to release an ESM version of badge-maker. It will require a major version bump because it is a breaking change, but that is fine. We can use the opportunity to drop compatibility with a bunch of unsupported node versions at the same time which is another breaking change we need to do in the next major.

So in principle it is fine to migrate badge-maker to ESM.

That said..

There is already a lot happening in this PR, and I don't want to scope-creep it to include "migrate badge-maker to ESM". It is just going to make the whole thing too much of a giant PITA to review.

So I think we should either:

1. Start a new branch/PR, the scope of which is only "migrate badge-maker to ESM" (just scoped to the package's existing functionality). Get that done/reviewed. Then we circle back to this PR and rebase it on the ESM version of badge-maker.
2. Switch the code you need back to CommonJS for now, and then migrate the whole lot back to ESM as a follow-up.

Both add some complexity, but I think option 1 is probably the approach I would go for. Option 2 feels like unnecessary churn.

[socket-security]

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License

View full report

[LitoMore]

Please let me know if you need help. I can help make the API standard/convention for optional packages.

For example, we can expose an async resolver function for loading external icons:

import {makeBadge} from 'badge-maker';

const badge = await makeBadge({
	message: 'external',
	iconSetResolver: async () => {
		const simpleIcons = await import('simple-icons');
		const customIcons = await import('custom-icons');
		return {...simpleIcons, ...customIcons};
	}
});

What do you think?

[jNullj]

I wasn't able to get back to this for some time, I hoped to be able to work on this during this weekend.

If you wish to, you could fork my work or start another PR if you find that better for you'r approach. Just tag me so I can notice your WIP so we don't end up with a conflict/working on same thing twice.

Please note that there is no agreement yet about support for other icons libs other then simpleicons, so while it could be fine to code with that in mind, I prefer if you did not include that in the code yet.