update simple-icons to v5 with by-name lookup backwards compatibility

[calebcartwright]

Implements the second option described in #6580 to update to v5 of simple-icons while maintaining backwards compatibility with how we've been handling named logos that contain spaces in the icon name/title. Note this means that icons can now be referenced via their slug or via their title, and title's containing spaces can be referenced either by escaping the spaces or replacing them with hyphens.

https://shields-staging-pr-6591.herokuapp.com/badge/foo-bar-blue?logo=linux-foundation

https://shields-staging-pr-6591.herokuapp.com/badge/foo-bar-blue?logo=linux%20foundation

https://shields-staging-pr-6591.herokuapp.com/badge/foo-bar-blue?logo=linuxfoundation

(know the icon loading isn't under the core directory but adding the label since it feels like a "core" item)

[shields-ci]

	Messages
📖	Thanks for contributing to our documentation. We ❤️ our documentarians!
📖	✨ Thanks for your contribution to Shields, @calebcartwright!

Generated by 🚫 dangerJS against 0e426d8

[chris48s]

📚 Background reading:

• Migrate to simple-icons slugs #4273
• Update simple-icons names #3641

I think if we're going to (finally) enable the proper SI slugs to be used, that should be the only documented option (but the others retained for backwards compatibility). I think our original plan was to literally hard-code old --> new maps and stop generating the old style slugs but maybe we're too far gone for that and the simplest thing to do is just generate more and more of them forever. . Do you have any thoughts on the tradeoffs?

[calebcartwright]

Background reading:

Thanks for this. Vaguely remember reading those back in the day but the dots hadn't fully connected for me until now.

I think if we're going to (finally) enable the proper SI slugs to be used, that should be the only documented option (but the others retained for backwards compatibility)

SI have forced our hand on this with the v5 update. We'd have to really bend over backwards to not support the proper slugs, and I can't imagine why we'd do extra work to avoid that. If we intend to continue supporting icon-by-name as well (regardless of fully or partially via hard coded table), then I guess I'd be fine not documenting/publicizing it.

I think our original plan was to literally hard-code old --> new maps and stop generating the old style slugs but maybe we're too far gone for that and the simplest thing to do is just generate more and more of them forever. . Do you have any thoughts on the tradeoffs?

At this point I'd be opposed to attempting to maintain a hard coded map of old slugs to the current set.

There'd be far too many, and then if/when the slugs are updated in SI we'd have to go update our maps. We'd also have to deal with questions/requests to maintain others (technically if we were to maintain such a mapping we could include entries for all these updated to prevent logos from disappearing on existing badges when we deploy a major version update. I know we aren't obligated to do so, just noting I'm not keen on having to explain-to/debate-with users why we're mapping some but not all.

I'd also worry about the impact on the user experience. At this point we certainly have users that have become accustomed to current by-name reference using the - or %20 delimiters, and I think it'd be really confusing as a user if I've always used that pattern, e.g. ?logo=visual-studio-code, and that pattern works for a bunch of other logos (assuming hard coded mapping), but when I try to apply that same pattern to some new Foo Bar logo it doesn't work, since in this hypothetical we wouldn't have mappings for new logos in the old table, and wouldn't directly support the name reference. Just seems it'd be really confusing/magical why the pattern would work for some/most but not all logos.

At the end of the day, I think it's inevitable that we'll start supporting the by-slug references directly as part of the v5 update, so IMO the question is really whether we want to also continue supporting by-name (and if not then if/how on the BC front).

Also, I'm not quite sure what the concerns or issues are with also supporting the by-name pattern given the v5 context. Feels like the reported issues have historically been driven by challenges associating to slug (no longer an issue IMO with the change of the exports in v5), users attempting and failing to use the actual slug in cases where our coercion failed to match the actual slug, couple quirky names (apostrophes/backticks), and our own locally maintained logos. Anything you can share/remind me of relative to why supporting both would be problematic?

[developStorm]

I was working on a project that shows all the Simple Icons on corrensponding Shields IO badge and experenced a hard time to find the correlation between slug/title of Simple Icons with logo option of Shields IO until I saw this PR (looks like I just happened to stuck in a major version update).

I've successfully integrated the test build of Shields above into my project, and it seems to be working well. Hope this could be deployed soon!

[chris48s]

At this point I'd be opposed to attempting to maintain a hard coded map of old slugs to the current set.

All good points 👍 Being ~2 years further down the path we are already on is significant.

Anything you can share/remind me of relative to why supporting both would be problematic?

I think the only downside with generating two sets of our own additional identifiers forever as well as the SI-provided slugs would be if there is any way we might generate a conflict or collision between an ID we've generated and one of the proper SI slugs. It looks super-unlikely that would happen but its hard to commit to saying it will never happen with any future release of SI.

For belt-and-braces we could throw an exception if we generate title or legacyTitle and it is in Object.keys(originalSimpleIcons).filter(slug => slug !== key). If it never happens, great. If it does, the tests fail when we upgrade to the release that causes it and we defer working out what to do about it to us-from-the-future.

[calebcartwright]

It looks super-unlikely that would happen but its hard to commit to saying it will never happen with any future release of SI.

Interesting scenario. The only extra slugs we'd be generating with this proposal is the lowercased version of the title, and the lowercased version of the title with the spaces replaced with hyphens. As of v5 SI no longer permit hyphens in the slugs (and never have had whitespace AFAIK), so the only path I could see for this to occur would be something a case where the title of one icon randomly matches the slug of another in the exports, e.g. something like this

{
  dog: { title: 'Cat', slug: 'dog', ... },
  cat: { title: 'Duck', slug: 'cat', ... },
  ...
}

Guess it's not 100% impossible even if it does feel like a stretch, but yes we could certainly include a check for this scenario

[ericcornelissen]

Hi all 👋 One of the core maintainers of Simple Icons here. I'm sorry for all the trouble we're causing 😅

I wanted to chime in to clarify some things regarding the slug value. The slug value is specifically intended to avoid problems due to brands with the same name or auto-generated slug (for example we have two icons both for a brand called "Hive") So, the slug is either derived from the title deterministically, indeed without whitespace and now also without hyphens, or it is custom, unique, and hardcoded in our repository in which case it will always contain an _.

So, in short, the above scenario would never occur. Supporting our slug value is a must if you want your users to be able to use all our icons; but using the title or hyphenated title should also always work except for the rare case where multiple brands share a name.

[calebcartwright]

No worries @ericcornelissen and thanks for the additional context! Long term I think the changes in v5 will actually be better for us and address some long standing problems. Just a matter of whether/how much we want to do to prevent logos disappearing on badges for some of our existing users 😄

[DenverCoder1]

👍 Version can be bumped to 5.1.0 as well now

[calebcartwright]

@chris48s

Have kept the by-name support in there, but updated docs and the site to remove those mentions and point everyone to the slugs. Let me know if you'd still like add some handling for existing functions. The case @ericcornelissen shared with Hive is a great one, as it demonstrates why we wouldn't want throw an exception on encountering a title that maps to an existing key. At the moment, such cases will mean that we process that particular icon twice on loading, but the end result would still be the same correct icon mapping.

That's obviously doing some unnecessary work and I'm tempted to add the key for presence in our map to avoid it, though held off for the moment since this is a one time/loadup not on a hotpath and because I feel like such an addition would need some additional inline commentary to provide context, which would have us trending towards having more lines of comments than javascript 😆

(also went ahead and updated to v5.1)

[chris48s]

Shall we change the order here.
i.e: if we're going to say the SI slug is the canonical ID and the others are fallbacks, then in the case of a collision like "hive" then if we set the SI slug last then that value is guaranteed to "win" the clash here.

[calebcartwright]

Good shout. Actually needed to incorporate the key check after all as originally proposed (just skipping instead of throwing exception). The order of the assignment within a given iteration doesn't matter so much, but more about needing the conditional checks otherwise the value would be determined by the last iteration that contains a colliding key. 32554a2

[chris48s]

👍 Lets ship it 🚢