JoshuaKGoldberg / create-typescript-app

Quickstart-friendly TypeScript template with comprehensive, configurable, opinionated tooling. 💝
MIT License
918 stars 69 forks source link

🐛 Bug: Allcontributors table images should have loading=lazy #1312

Open JoshuaKGoldberg opened 7 months ago

JoshuaKGoldberg commented 7 months ago

Bug Report Checklist

Expected

Images in the allcontributors table should have loading="lazy" so that they don't slow down initial page load or render.

Actual

I tried loading https://github.com/JoshuaKGoldberg/create-typescript-app in a throttled network connection and noticed a lot of images loading right away. They even seemed to be blocking first page render Yuck.

Additional Info

This might involve fixing on the allcontributors side?

I also don't know if this is possible in GitHub's markdown renderer... but it really should be. If it isn't, I should find a place to request it / upvote an existing request.

johnnyreilly commented 6 months ago

I very much like this idea (having written about how to do this in Docusaurus and helped make it the default behaviour for Docusaurus I declare an interest 😄 )

Looking at the README.md, it should just work; https://raw.githubusercontent.com/JoshuaKGoldberg/create-typescript-app/main/README.md

However, the proof is in the pudding. I'm going to tweak the README on a branch and see if it works.

johnnyreilly commented 6 months ago

So in tragic news, I don't think this is doable unless the GitHub Markdown rendered changes. Consider:

screenshot of devtools

I tweaked the README.md on the test-loading-lazy branch: https://github.com/JoshuaKGoldberg/create-typescript-app/blob/test-loading-lazy/README.md and hacked in loading="lazy" attributes. Then I went to: https://github.com/JoshuaKGoldberg/create-typescript-app/tree/test-loading-lazy

And as the screenshot of devtools shows, the loading="lazy" were stripped by the GitHub Markdown renderer. So unless that changes, there's not a way forward. How to find out....

johnnyreilly commented 6 months ago

BTW feel free to delete the branch - don't want to clutter. More just wanted to demostrate my findings!

johnnyreilly commented 6 months ago

The library GitHub uses for Markdown rendering seems to be:

https://github.com/gjtorikian/commonmarker

(If I understand https://github.com/github/markup right)

Potential issue:

Please note that only the first step is covered by this gem - the rest happens on http://GitHub.com. In particular, markup itself does no sanitization of the resulting HTML, as it expects that to be covered by whatever pipeline is consuming the HTML

johnnyreilly commented 6 months ago

So commonmarker wraps the rust library https://github.com/kivikakk/comrak

I've had a little dig through that and don't detect any special processing around images. This makes it seem likely that (as per comment above) GitHub itself does the loading attribute stripping. As such, unless we can influence GitHub in some way, I don't think we can do anything about this issue.

johnnyreilly commented 6 months ago

I've raised a request with GitHub support to discuss whether the loading="lazy" attribute could be allowlisted on GitHub.com.

https://support.github.com/ticket/personal/0/2692240

johnnyreilly commented 6 months ago

It's a requested feature, this issue cannot advance unless the following is implemented:

https://github.com/orgs/community/discussions/42558

See also: https://gist.github.com/kivikakk/622b5dcf395e26c49e2334f0eb19e6f9

JoshuaKGoldberg commented 6 months ago

Thanks for the investigation, great finds!