Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix featured sponsors SVG (404 error) #192

Merged
merged 3 commits into from
Apr 16, 2024

Conversation

stasadev
Copy link
Member

@stasadev stasadev commented Apr 16, 2024

The Issue

The sponsors image is broken https://github.com/ddev/ddev#wonderful-sponsors

https://ddev.com/resources/featured-sponsors.svg

https://ddev.com/resources/featured-sponsors-darkmode.svg

https://github.com/ddev/ddev.com/actions/runs/8649477119/job/23715791078#step:6:227

15:41:55   └─ /resources/featured-sponsors-darkmode.svg15:41:55 [WARN] [router] No API Route handler exists for the method "GET" for the route "/resources/featured-sponsors-darkmode.svg".
Found handlers: "get"

 (+0ms)
15:41:55 λ src/pages/resources/featured-sponsors.svg.js
15:41:55   └─ /resources/featured-sponsors.svg15:41:55 [WARN] [router] No API Route handler exists for the method "GET" for the route "/resources/featured-sponsors.svg".
Found handlers: "get"

How This PR Solves The Issue

There were two issues after:

  1. changes in Astro routes, I fixed it with this instruction.

  2. using node-base64-image - I don't know much about JS so I just changed the library back to base64-img.

Manual Testing Instructions

Check for /resources/featured-sponsors.svg and /resources/featured-sponsors-darkmode.svg

https://20240416-fix-featured-sponso.ddev-com-front-end.pages.dev/resources/featured-sponsors.svg

https://20240416-fix-featured-sponso.ddev-com-front-end.pages.dev/resources/featured-sponsors-darkmode.svg

Automated Testing Overview

Related Issue Link(s)

Release/Deployment Notes

@stasadev stasadev requested a review from bmartinez287 April 16, 2024 17:17
@bmartinez287
Copy link
Collaborator

Oooooh, I like how well-documented your issue is. I'm going to look into improving my own haha.
I will take a look at the changes, I must have missed this issue during the upgrade process.
I have some concerns with adding base64-img back since that package has some security vulnerabilities as its dependencies that will get flagged by GitHub and npm at build time. I will double-check to see if node-base64-image can handle it since it's supposed to do the same.

@stasadev
Copy link
Member Author

Thanks, encode() function is a promise, and it should be called in a different way.

I compared the output for them:

base64Img.base64Sync(image.path)

data:image/svg+xml;base64,BASE64OUTPUTHERE...

await encode(image.path, {string: true, local: true})

BASE64OUTPUTHERE...

There is a difference in data:image/svg+xml;base64, and the main problem for me - I don't know how to call it correctly 🙂

@bmartinez287
Copy link
Collaborator

bmartinez287 commented Apr 16, 2024

This might take a little while. I tried a couple of approaches but I haven't reached the same results yet.
If you guys feel like it would be better to add it and refactor it later I'm good with that otherwise I will try to find an alternative this week.
An alternative to the base64-img node package that has all sorts of security issues but works.

@stasadev
Copy link
Member Author

I agree with any decision. The image has been down for almost two weeks, and I just noticed it now. If it can wait longer, I don't mind.

@rfay
Copy link
Member

rfay commented Apr 16, 2024

The bottom line is, thanks to both of you for your work on this.

Copy link

cloudflare-workers-and-pages bot commented Apr 16, 2024

Deploying ddev-com-front-end with  Cloudflare Pages  Cloudflare Pages

Latest commit: f3f5d40
Status: ✅  Deploy successful!
Preview URL: https://a91f6ee5.ddev-com-front-end.pages.dev
Branch Preview URL: https://20240416-fix-featured-sponso.ddev-com-front-end.pages.dev

View logs

@stasadev
Copy link
Member Author

Should be better now, without any library 🙂

Not sure why the test fails:

Warning: No existing directories found containing cache-dependency-path="./yarn.lock"
Error: Some specified paths were not resolved, unable to cache dependencies.

@stasadev stasadev merged commit 7a84958 into main Apr 16, 2024
1 of 2 checks passed
@stasadev stasadev deleted the 20240416_fix_featured_sponsors_svg branch April 16, 2024 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants