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

chore: change icons to reduce bundle #537

Merged
merged 2 commits into from
May 4, 2024
Merged

Conversation

SpecialAro
Copy link
Member

@SpecialAro SpecialAro commented May 4, 2024

Pre-flight Checklist

Please ensure you've completed all of the following.

Description of Change

After some discussion with @vraravam we came to the conclusion that SVG increase local bundle size for users, which is unnecessary given users can fetch the static assets generated by jsdelivr.

Indeed, before this PR total package bundle of all recipes was 7.34 MB and now it is 377 KB, which is ~19.5 times smaller.

To test this PR do the following:

  • Checkout the PR in the main repo: chore: change icons to reduce bundle ferdium-app#1740
  • Go to the recipes submodule and add my ferdium-recipes fork as a remote
  • Checkout this branch PR
  • Run pnpm i and pnpm package
  • Go back to the main repo folder
  • Compile the app and run it for the first time (add 2 or 3 recipes)
  • Go to your system folder (in case of Windows %AppData%/Ferdium/recipes) and manually bring the version down of each recipe package.json
  • Reload Ferdium and wait for the prompt to reload services.

What to expect:

  • Icons won't change (given we are using the same icons)
  • The SVG file in the %AppData%/Ferdium/recipes/{recipeId} folder should be automatically deleted.

This PR needs to be merged before the one on the main repo (ferdium/ferdium-app#1740).

@SpecialAro SpecialAro requested review from vraravam and a team May 4, 2024 02:59
@SpecialAro SpecialAro self-assigned this May 4, 2024
Copy link
Contributor

@vraravam vraravam left a comment

Choose a reason for hiding this comment

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

LGTM!
Nice work @SpecialAro

@vraravam vraravam merged commit f02fda1 into ferdium:main May 4, 2024
2 checks passed
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