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

Webpack bundle dependencies #2511

Merged

Conversation

absidue
Copy link
Member

@absidue absidue commented Aug 26, 2022


Webpack bundle dependencies

Pull Request Type

  • Feature Implementation

Description
This pull request makes various changes to reduce FreeTube's package size. The first major one is allowing webpack to bundle the dependencies, this means that only the code that actually used is included in the final build (webpack failed to bundle both ytsr and ytpl, so for the moment they will still be included as is in the final package).

Bundling your code and dependencies is recommended by the Electron performance guidelines, electron-builder however defaults to packaging all non dev dependencies in the node_modules directory into your package, which blows up the package size unnecessarily.

As the default behaviour of electron-builder is now undesired, I've blacklisted the entire node_modules folder and only whitelisted ytsr, ytpl as well as their shared dependency miniget.

I also added the configuration option to tell the vue compile to condense whitespace, this change only changed the bundle size by a few 10s of kb but as it's such an easy change it doesn't hurt. This got rid of whitespace that we actually need, so I got rid of that option.

The final change was to import the icons we need from the fontawesome solid icons icon pack, instead of importing the whole pack. This allows webpack to only bundle the icons we need instead of bundling all of them. This reduced the size of renderer.js file by another 0.6MB (now 1.93MB 1.96MB).

All in all the changes resulted in the app.asar file now only being 6.51MB 6.53MB big (down from 36.6MB). We can probably make it even smaller but that's something for future me to tackle.

Some ideas that I've got are minimising the CSS (would require us to add a load more development dependencies though, thanks CssMinimizerWebpackPlugin), miminising the static JSON files and only packaging MacOS icons in MacOS builds, the same for the Windows ones.

Screenshots (if appropriate)
Before:
before

After:
see new screenshot in comment below
after

Testing (for code that is not small enough to be easily understandable)
I tested both development yarn dev and production builds yarn build. I also backed up and nuked my FreeTube user data to make sure no cached files were being used.

Desktop (please complete the following information):

  • OS: Windows
  • OS Version: 10
  • FreeTube version: 0.17.1

@absidue absidue added the PR: waiting for review For PRs that are complete, tested, and ready for review label Aug 26, 2022
@PrestonN PrestonN enabled auto-merge (squash) August 26, 2022 13:29
Copy link
Collaborator

@PikachuEXE PikachuEXE left a comment

Choose a reason for hiding this comment

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

Code review passed
Time to make another custom build...

Copy link
Member

Choose a reason for hiding this comment

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

Lgtm

@PikachuEXE
Copy link
Collaborator

PikachuEXE commented Aug 31, 2022

Style suggestion on settings UI

image
image

Code changes:
image
image

Edit: Wrong PR
Time to devolve back to Pichu

@absidue
Copy link
Member Author

absidue commented Aug 31, 2022

okay so i'm going to have to get rid of the whitespace condense vue option, as it seems to not condense whitespace in the same way that web browsers do so we end up with the issue that it removes whitespace that we actually need:
vue-bug

this is what it's supposed to look like:
what-it-should-look-like

@absidue
Copy link
Member Author

absidue commented Aug 31, 2022

Unfortunately the sizes are no longer accurate, the app.asar file is now 6.53MB:
new-size

renderer.js is now at 1.96MB

If I can find a way to use that option without it breaking stuff, then I'll add it back again in the future. For the moment I'll take the small size increase to keep stuff working.

Copy link
Member

@ChunkyProgrammer ChunkyProgrammer left a comment

Choose a reason for hiding this comment

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

build and dev worked great for me

Copy link
Collaborator

@PikachuEXE PikachuEXE left a comment

Choose a reason for hiding this comment

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

NO issue spotted after using for 1 week

@PrestonN PrestonN merged commit eaa15ea into FreeTubeApp:development Sep 6, 2022
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Sep 6, 2022
@absidue absidue deleted the webpack-bundle-dependencies branch September 6, 2022 07:53
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.

5 participants