Skip to content

Conversation

cprecioso
Copy link
Member

@cprecioso cprecioso commented Oct 1, 2025

Before

3.8M	./app/src/client/static/open-saas-banner-dark.png
2.2M	./app/src/client/static/open-saas-banner-light.png

After

118K	./app/src/client/static/open-saas-banner-dark.svg
118K	./app/src/client/static/open-saas-banner-light.svg

@cprecioso cprecioso self-assigned this Oct 1, 2025
@cprecioso cprecioso requested a review from vincanger October 1, 2025 08:52
@cprecioso cprecioso changed the base branch from cprecioso/optimize-assets to main October 1, 2025 08:53
Copy link
Contributor

@FranjoMindek FranjoMindek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great initiative.

Just check how this works with prettier before the merge:
#518 (comment)

import { Link as WaspRouterLink, routes } from "wasp/client/router";
import openSaasBannerDark from "../../client/static/open-saas-banner-dark.png";
import openSaasBannerLight from "../../client/static/open-saas-banner-light.png";
import openSaasBannerDark from "../../client/static/open-saas-banner-dark.svg";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC loading SVG files through <img> element is not recommended.
But I think it's fine here because we literally use it as an image.

Usual reasons are:

  • bad for performance of small SVG icons
  • lose the option of styling the SVG through classes (stroke, fill...)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, this doesn't apply here as we're literally just using an image.

@cprecioso cprecioso merged commit fa15f85 into main Oct 1, 2025
1 check passed
@cprecioso cprecioso deleted the cprecioso/use-svg branch October 1, 2025 09:06
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.

2 participants