Reduce desktop app size #7744

kidroca · 2022-02-14T17:51:29Z

Details

electron-builder uses package.json to determine what node_modules needs to be packed inside the app (dependencies)
Our root package contains many dependencies irrelevant to desktop (e.g. ios/android stuff) an existing pattern dealing with this is to create a 2nd package.json only for Electron (native) dependencies: https://www.electron.build/tutorials/two-package-structure. So this is what we decided to do here.

The webpack configuration is updated so that we don't use a dist folder external to desktop
We're also using webpack to bundle main.js (config inspired by https://github.com/electron-react-boilerplate/electron-react-boilerplate).
It helps us by:

injecting .env to webpack and main.js
we don't have to keep the files list upto date each time we import something from ../src

Added some notes in desktop/README

Excluded package-lock changes to have less PR changes

⚠️Todo: run npm install once this get reviewed

Fixed Issues

$ #6927

Tests

Run npm install before you start

Verify running locally npm run desktop still works (with/without .env file)
Verify the build scripts work to build local version. Running build-desktop and build-desktop-staging should not publish unless the --publish flag is used
Verify the build scripts would work in GH actions by trying them with the --publish flag.
```
npm run build-desktop-staging -- --publish always
```

Since the webpack configs for web were modified we should check those as well

Verify npm run web still works
Verify npm run build builds the production web bundle
Verify npm run build-staging builds the staging web bundle

iOS and Android should be unaffected

QA Steps

For each env do the following:

Download the Desktop app .dmg and verify the size is close to 90 Mb instead of 334Mb
Install the app and verify that the installed size is close to 215Mb instead of 856Mb
staging app: https://staging.new.expensify.com/NewExpensify.dmg
prod app: https://new.expensify.com/NewExpensify.dmg

Sample.check.desktop.app.sizes.mp4

Tested On

Screenshots

Web

Mobile Web

Desktop

DEV

STAGING

PROD

iOS

Android

kidroca · 2022-02-14T18:11:32Z

Excluded package-lock changes to have less PR changes

Todo: run npm install once this get reviewed

kidroca

Added notes on some of the changes

config/webpack/webpack.common.js

config/webpack/productionConfig.js

kidroca · 2022-02-14T18:26:52Z

config/electron.config.js

+        target: [
+            {target: 'dmg', arch: ['x64', 'arm64', 'universal']},
+        ],


We can drop the universal target if we're not going to use it.
The idea is to give x64 to "old" mac users and arm64 for M1s
Universal works for both but is 2x the size (This was briefly touched on the issue as well: #6927 (comment))

BTW I have no problem running the x64 version on M1 - you can't really tell the difference

Sample build output for these targets:

config/electron.config.js

roryabraham · 2022-02-14T19:50:23Z

I would like to review this once it's ready. A couple other suggestions that might be worth investigating to reduce bundle size:

kidroca · 2022-02-15T13:05:55Z

A couple other suggestions that might be worth investigating to reduce bundle size:

App bundling enhancements would only reduce the app.js script size - 5.9Mb at the moment

Even entirely removing app.js would only make App ~5Mb smaller, while the dmg would shrink by upto 1.5Mb

We would gain more by excluding source maps - 16.7Mb for app and 2.3Mb for pdf worker, though it would again make the installer just 5Mb smaller
(Source maps could be useful to someone debugging the prod app though)

This PR focuses on a lot bigger scale (not directly related to webpack bundling, but how electron-builder works)

getting installer from 326 down to 90Mb
installed size from 812 down to 210Mb

Babel enhancements are still worth for making the web app load faster (over internet), but don't bring much for the Desktop app

Default is `dist` and is used for temporary content while building We also use `dist` to bundle the js script there We should not use the same folder as in the end temp content (200MB) gets bundled in to the desktop app

Provide arguments through Webpack `env` CLI API 1. Have the base configuration setup for PROD 2. Have the dev configuration override it 3. Desktop use creates 2 configurations in parallel

This script is now getting bundled into `main.js` before `package.json` is modified by electron-builder But the build process sets the `process.env.ELECTRON_ENV` so it's not a problem to just remove these lines Note: updated return docs - the returned values are the lowercase names and not ['PROD', 'STG', 'DEV']

kidroca

@roryabraham
I've unified the 2 electron Builder configs and had to make changes to usages of ELECTRON_ENV. Particularly to ELECTRON_ENVIRONMENT.js I think it serves the same purpose this way.

Added a few more notes as code review comments

config/webpack/webpack.common.js

config/webpack/webpack.desktop.js

desktop/build.sh

desktop/ELECTRON_ENVIRONMENT.js

tgolen

Looks pretty good to me. Just noticed a few things that could be improved in the readme.

desktop/README.md

Co-authored-by: Tim Golen <[email protected]>

kidroca · 2022-02-24T20:14:33Z

✔️ Ready for Review

tgolen

Thanks! Also, nice catch on renaming CONST/index.js.

tgolen · 2022-02-24T21:30:52Z

conflicts :(

roryabraham

LGTM and thanks for making all the revisions.

kidroca · 2022-02-25T10:59:45Z

✔️ Conflict resolved
It was only due to CONST/index.js becoming CONST.js again and there was no conflict in merging the actual content. Even the approving reviews are still valid

OSBotify · 2022-02-25T16:54:00Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

OSBotify · 2022-03-01T19:54:17Z

🚀 Deployed to staging by @roryabraham in version: 1.1.41-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	failure ❌

roryabraham · 2022-03-01T20:01:06Z

@kidroca I think this might have broken storybook builds: https://github.com/Expensify/App/runs/5380104817?check_suite_focus=true

I am able to reproduce the error on main by running npm run storybook-build

kidroca · 2022-03-01T21:50:19Z

That's something I forgot to test, I'll try it and get back to you

kidroca · 2022-03-01T21:53:47Z

Yes definitely broke storybook with the changes to webpack.common.js but can make it work again

OSBotify · 2022-03-02T10:47:35Z

🚀 Deployed to staging by @roryabraham in version: 1.1.41-0 🚀

platform	result
🤖 android 🤖	failure ❌
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	failure ❌

OSBotify · 2022-03-11T00:04:08Z

🚀 Deployed to production by @francoisl in version: 1.1.41-6 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

kidroca force-pushed the kidroca/feature/new-desktop-config branch 2 times, most recently from 3869c29 to 7a9c564 Compare February 14, 2022 18:09

kidroca force-pushed the kidroca/feature/new-desktop-config branch from 7a9c564 to ff7da1b Compare February 14, 2022 18:23

kidroca commented Feb 14, 2022

View reviewed changes

roryabraham self-requested a review February 14, 2022 19:49

kidroca added 13 commits February 15, 2022 20:06

Make separate desktop package.json

02fd399

Set desktop build output directory

03d6cb7

Default is `dist` and is used for temporary content while building We also use `dist` to bundle the js script there We should not use the same folder as in the end temp content (200MB) gets bundled in to the desktop app

Build for multiple desktop architectures

b9e02c0

Ignore build artifacts

1550017

Change file structure - files from parent folders cannot be included

f3b03ac

New webpack.web.js configuration to substitute prod/staging

77361f3

New webpack.desktop.js configuration

844309e

Simplify webpack configs

645f82e

Provide arguments through Webpack `env` CLI API 1. Have the base configuration setup for PROD 2. Have the dev configuration override it 3. Desktop use creates 2 configurations in parallel

Add documentation regarding packaging

0500f4c

Fix main USE_WEB_PROXY check

9625529

Get rid of the nested package folder

922b661

Remove unneeded stuff from webpack.desktop

67cb30b

One electronBuilder config

1e653bd

kidroca force-pushed the kidroca/feature/new-desktop-config branch from ff7da1b to 121a41a Compare February 15, 2022 20:50

kidroca added 3 commits February 15, 2022 22:53

Extract desktop/build.sh script

4e9d972

use CustomVersionPlugin only on web

8a09b0e

kidroca force-pushed the kidroca/feature/new-desktop-config branch from 121a41a to b8cf68e Compare February 15, 2022 20:54

kidroca commented Feb 15, 2022

View reviewed changes

config/webpack/webpack.common.js Show resolved Hide resolved

config/webpack/webpack.desktop.js Outdated Show resolved Hide resolved

desktop/build.sh Outdated Show resolved Hide resolved

kidroca marked this pull request as ready for review February 15, 2022 23:07

kidroca requested a review from a team as a code owner February 15, 2022 23:07

MelvinBot removed the request for review from a team February 15, 2022 23:07

Fix scripts relative path

781c8d5

roryabraham requested changes Feb 23, 2022

View reviewed changes

desktop/ELECTRON_ENVIRONMENT.js Show resolved Hide resolved

tgolen requested changes Feb 23, 2022

View reviewed changes

desktop/README.md Outdated Show resolved Hide resolved

desktop/README.md Outdated Show resolved Hide resolved

desktop/README.md Outdated Show resolved Hide resolved

kidroca and others added 3 commits February 24, 2022 20:34

Fix documentation typo

f48301f

Co-authored-by: Tim Golen <[email protected]>

Delete ENVIRONMENT.js and move definitions back inside CONST/index.js

bf345de

Expand on desktop packaging documentation

e1b0861

kidroca requested review from tgolen and roryabraham February 24, 2022 20:14

tgolen approved these changes Feb 24, 2022

View reviewed changes

roryabraham approved these changes Feb 25, 2022

View reviewed changes

Merge branch 'main' into kidroca/feature/new-desktop-config

e6df824

roryabraham merged commit 3e674c4 into Expensify:main Feb 25, 2022

kidroca deleted the kidroca/feature/new-desktop-config branch February 28, 2022 09:59

OSBotify mentioned this pull request Mar 1, 2022

Deploy Checklist: New Expensify 2022-03-01 #7954

Closed

68 tasks

mvtglobally mentioned this pull request Mar 3, 2022

Desktop build is not updated #7987

Closed

kidroca mentioned this pull request Mar 3, 2022

Fix storybook config #7988

Closed

35 tasks

roryabraham mentioned this pull request Mar 4, 2022

[CP Stg] Add title and artifactName back in ElectronBuilder config #7996

Merged

50 tasks

kavimuru mentioned this pull request Mar 5, 2022

Desktop app size is 161 Mb instead 90 Mb #8014

Closed

Stutikuls mentioned this pull request Mar 16, 2022

Implement infinite sessions #7

Merged

roryabraham mentioned this pull request Jul 8, 2022

[HOLD for payment 2022-08-11] [$250] Electron log file not working #9794

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reduce desktop app size #7744

Reduce desktop app size #7744

kidroca commented Feb 14, 2022 •

edited

Loading

kidroca commented Feb 14, 2022 •

edited

Loading

kidroca left a comment

kidroca Feb 14, 2022 •

edited

Loading

roryabraham commented Feb 14, 2022

kidroca commented Feb 15, 2022

kidroca left a comment

tgolen left a comment

kidroca commented Feb 24, 2022

tgolen left a comment

tgolen commented Feb 24, 2022

roryabraham left a comment

kidroca commented Feb 25, 2022

OSBotify commented Feb 25, 2022

OSBotify commented Mar 1, 2022

roryabraham commented Mar 1, 2022

kidroca commented Mar 1, 2022

kidroca commented Mar 1, 2022

OSBotify commented Mar 2, 2022

OSBotify commented Mar 11, 2022

Reduce desktop app size #7744

Reduce desktop app size #7744

Conversation

kidroca commented Feb 14, 2022 • edited Loading

Details

Fixed Issues

Tests

QA Steps

Tested On

Screenshots

Web

Mobile Web

Desktop

DEV

STAGING

PROD

iOS

Android

kidroca commented Feb 14, 2022 • edited Loading

kidroca left a comment

Choose a reason for hiding this comment

kidroca Feb 14, 2022 • edited Loading

Choose a reason for hiding this comment

Sample build output for these targets:

roryabraham commented Feb 14, 2022

kidroca commented Feb 15, 2022

kidroca left a comment

Choose a reason for hiding this comment

tgolen left a comment

Choose a reason for hiding this comment

kidroca commented Feb 24, 2022

tgolen left a comment

Choose a reason for hiding this comment

tgolen commented Feb 24, 2022

roryabraham left a comment

Choose a reason for hiding this comment

kidroca commented Feb 25, 2022

OSBotify commented Feb 25, 2022

OSBotify commented Mar 1, 2022

roryabraham commented Mar 1, 2022

kidroca commented Mar 1, 2022

kidroca commented Mar 1, 2022

OSBotify commented Mar 2, 2022

OSBotify commented Mar 11, 2022

kidroca commented Feb 14, 2022 •

edited

Loading

kidroca commented Feb 14, 2022 •

edited

Loading

kidroca Feb 14, 2022 •

edited

Loading