-
-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
Desktop: Fixes #11405: Reduce application size by removing unnecessary Rust files #11412
Conversation
Thanks Pedro, so what is causing this 30 MB increase in size? |
I'm going to build the two versions and see why the difference is appearing. |
When comparing the two AppImages unpacked, the biggest change has been on Note that these sizes are before packaging so that they are so high. Comparing Details
I'm not sure how easy it will be to understand, but when I was investigating I saved some logs to compare again: |
@@ -61,7 +61,7 @@ export default class InteropService_Importer_OneNote extends InteropService_Impo | |||
const outputDirectory2 = join(tempOutputDirectory, baseFolder); | |||
|
|||
const notebookFiles = zip.getEntries().filter(e => e.name !== '.onetoc2' && e.name !== 'OneNote_RecycleBin.onetoc2'); | |||
const { oneNoteConverter } = shim.requireDynamic('../../../onenote-converter/pkg/onenote_converter'); | |||
const { oneNoteConverter } = shim.requireDynamic('../../node_modules/@joplin/onenote-converter/pkg/onenote_converter'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be updated
Fixes #11405
Summary
After the inclusion of the
onenote-converter
package to the project, the release build had a 100MB+ increase in size.This PR addresses that by removing the
target
folder used by Rust to store build and was being imported to the application bundle.After this PR:
Further work:
The biggest contributors to the size are:
It seems like the packages are duplicated, for example
@joplin/renderer
requiresmermaid
, but@joplin/lib
by requiring@joplin/renderer
also hasmermaid
inside of it.Not sure how much work, but I think we might be able to cut down some MBs with a way to remove duplication when they happen. (maybe this already happen on the executable and it is only visible when the
asar
is unpacked??)Testing
git clean -dfx
on project rootyarn
cd packages/onenote-converter && IS_CONTINUOUS_INTEGRATION=1 yarn build
cd ../app-desktop
yarn dist
ls -lah dist/
./dist/Joplin-3.2.3.AppImage
and make sure it is working