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

Minify and concatenate JS/CSS/HTML #820

Closed
jviotti opened this issue Nov 2, 2016 · 13 comments
Closed

Minify and concatenate JS/CSS/HTML #820

jviotti opened this issue Nov 2, 2016 · 13 comments

Comments

@jviotti
Copy link
Contributor

jviotti commented Nov 2, 2016

  • Revise that every development-related file is excluded from the final package.
  • Concatenate and minify all JavaScript.
  • Minify CSS.
  • Minify HTML.

From #632

@lurch
Copy link
Contributor

lurch commented Nov 2, 2016

I suppose the HTML could be minified too?

@jviotti
Copy link
Contributor Author

jviotti commented Nov 2, 2016

Correct. I'll add it the issue description.

On Wed, Nov 02, 2016 at 11:05:43AM -0700, Andrew Scheller wrote:

I suppose the HTML could be minified too?

You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#820 (comment)

Juan Cruz Viotti
Software Engineer

@alexandrosm
Copy link
Contributor

what kind of improvement are we talking about?

Alexandros Marinos

Founder & CEO, Resin.io

+1 206-637-5498

@alexandrosm

On Wed, Nov 2, 2016 at 11:14 AM, Juan Cruz Viotti notifications@github.com
wrote:

Correct. I'll add it the issue description.

On Wed, Nov 02, 2016 at 11:05:43AM -0700, Andrew Scheller wrote:

I suppose the HTML could be minified too?

You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#820 (comment)

Juan Cruz Viotti
Software Engineer


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#820 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABLUCK7lpghOcwGs84b3a3gGc_cXl9Vwks5q6NMXgaJpZM4Knj63
.

@lurch
Copy link
Contributor

lurch commented Nov 3, 2016

I doubt the minification will save much, relative to the size of Etcher; but removing the dev-files from the final packages should save a good chunk.
</guess>

@alexandrosm
Copy link
Contributor

yeah, the minification sounds like pain for little gain. but a test should
tell us :)

Alexandros Marinos

Founder & CEO, Resin.io

+1 206-637-5498

@alexandrosm

On Wed, Nov 2, 2016 at 5:29 PM, Andrew Scheller notifications@github.com
wrote:

I doubt the minification will save much, relative to the size of Etcher;
but removing the dev-files from the final packages should save a good chunk.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#820 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABLUCG8drB96s6Qr_Nub7rKAuG9kcIceks5q6SrcgaJpZM4Knj63
.

@lurch
Copy link
Contributor

lurch commented Nov 3, 2016

So, as well as the /usr/share/etcher-electron/resources/app.asar (installed by the etcher-electron .deb) containing the entire scripts (2.6MB) directory (as mentioned in #632 (comment) ), it also includes other 'useless' stuff like node_modules/electron-winstaller-fixed (9.8MB), two copies of liblzma.dll (300KB), 7 .exe files (1.1MB), 5 .a files (1.3MB), 80 .o files (535KB), 3 .bz2 files (1.1MB).
Lots of scope for improvement ;-)

Also, I've never worked on any NodeJS projects before, but node_modules contains 666 directories, which seems a little excessive? shrug (and find node_modules/ -type d -iname "node_modules" -print|wc -l reports that 139 of those have their own node_modules directory?!)

@jviotti
Copy link
Contributor Author

jviotti commented Nov 3, 2016

So, as well as the /usr/share/etcher-electron/resources/app.asar
(installed by the etcher-electron .deb) containing the entire
scripts (2.6MB) directory (as mentioned in
#632 (comment)
), it also includes other 'useless' stuff like
node_modules/electron-winstaller-fixed (9.8MB), two copies of
liblzma.dll (300KB), 7 .exe files (1.1MB), 5 .a files (1.3MB), 80 .o
files (535KB), 3 .bz2 files (1.1MB).
Lots of scope for improvement ;-)

Definitely. I've been giving this a try and I feel the --ignore option
from electron-packager is starting to fall short. If I ignore
"scripts", then it will ignore everything that contains that string,
plus blacklisting stuff is starting to get unwieldy.

The task that electron-packager performs is very simple though: it
just downloads a pre-built Electron package, creates an asar with the
code, puts every file according to Electron's convention, and renames
the executable name to "Etcher", plus some other trivialities like that.
If we take care of this during the build phase by ourselves, it means we
can take a whitelisting approach instead.

Also, I've never worked on any NodeJS projects before, but
node_modules contains 666 directories, which seems a little
excessive? shrug (and find node_modules/ -type d -iname "node_modules" -print|wc -l reports that 139 of those have their
own node_modules directory?!)

Welcome to npm hell! :) Concatenating all JavaScript, including
dependencies, in a single file allows us run some tools on top of it
that are able to remove unused code, at least to a certain degree, which
could provide non-trivial size reductions. I'm not sure how this will
play with native dependencies though, but we can try to figure something
out.

On Thu, Nov 03, 2016 at 05:49:24AM -0700, Andrew Scheller wrote:

So, as well as the /usr/share/etcher-electron/resources/app.asar (installed by the etcher-electron .deb) containing the entire scripts (2.6MB) directory (as mentioned in #632 (comment) ), it also includes other 'useless' stuff like node_modules/electron-winstaller-fixed (9.8MB), two copies of liblzma.dll (300KB), 7 .exe files (1.1MB), 5 .a files (1.3MB), 80 .o files (535KB), 3 .bz2 files (1.1MB).
Lots of scope for improvement ;-)

Also, I've never worked on any NodeJS projects before, but node_modules contains 666 directories, which seems a little excessive? shrug (and find node_modules/ -type d -iname "node_modules" -print|wc -l reports that 139 of those have their own node_modules directory?!)

You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#820 (comment)

Juan Cruz Viotti
Software Engineer

@lurch
Copy link
Contributor

lurch commented Nov 3, 2016

If I ignore "scripts", then it will ignore everything that contains that string

Doh! :-( That seems a bit short-sighted.

If we take care of this during the build phase by ourselves, it means we
can take a whitelisting approach instead.

I guess it depends whether it'd be easier to write our own version, or PR new features to the existing electron-packager ?

Welcome to npm hell! :)

LOL 😆

Does the .asar contain code for all 3 platforms, or is the .asar still different-per-platform? If it's the latter we may be able to do more optimisations too? (e.g. we probably don't need all the different icons in each asar?)

@tooomm
Copy link

tooomm commented Nov 3, 2016

When I first run the portable version provided recently, I was a bit shocked by the size of the "little" tool.
Decompressing also took some time, even startup of the application needs a fair long moment, too... much of it will hopefully improve with fewer files and a smaller footprint! 👍

@jviotti
Copy link
Contributor Author

jviotti commented Nov 3, 2016

I guess it depends whether it'd be easier to write our own version, or PR new features to the existing electron-packager ?

We are running a very old electron-packager version, so maybe we
should try upgrading and see if new capabilities for whitelisting have
been added.

Does the .asar contain code for all 3 platforms, or is the .asar still
different-per-platform? If it's the latter we may be able to do more
optimisations too? (e.g. we probably don't need all the different
icons in each
asar?)

Most of the code is cross-platform, by the very nature of NodeJS. The
code that is platform specific is decribed in package.jsons as
optionalDependencies, so when installing dependencies, NPM will
automatically ignore platform specific packages, like diskpart on
GNU/Linux, etc.

On Thu, Nov 03, 2016 at 07:31:41AM -0700, Andrew Scheller wrote:

If I ignore "scripts", then it will ignore everything that contains that string

Doh! :-( That seems a bit short-sighted.

If we take care of this during the build phase by ourselves, it means we
can take a whitelisting approach instead.

I guess it depends whether it'd be easier to write our own version, or PR new features to the existing electron-packager ?

Welcome to npm hell! :)

LOL 😆

Does the .asar contain code for all 3 platforms, or is the .asar still different-per-platform? If it's the latter we may be able to do more optimisations too? (e.g. we probably don't need all the different icons in each asar?)

You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#820 (comment)

Juan Cruz Viotti
Software Engineer

@jviotti
Copy link
Contributor Author

jviotti commented Nov 3, 2016

When I first run the portable version provided recently, I was a bit
shocked by the size of the "little" tool. Decompressing also took
some time, even startup of the application needs a fair long moment,
too... much of it will hopefully improve with fewer files and a
smaller footprint! 👍

Electron applications take forever to start, I agree. Hopefully the
Electron team makes optmisations on their side as well.

On Thu, Nov 03, 2016 at 08:04:17AM -0700, tooomm wrote:

When I first run the portable version provided recently, I was a bit shocked by the size of the "little" tool.
Decompressing also took some time, even startup of the application needs a fair long moment, too... much of it will hopefully improve with fewer files and a smaller footprint! 👍

You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#820 (comment)

Juan Cruz Viotti
Software Engineer

@lurch
Copy link
Contributor

lurch commented Nov 3, 2016

...and some directories (like node_modules/rx/dist) contain both minified and non-minified .js files, which wastes a couple of MB.

@jviotti jviotti modified the milestone: Backlog Nov 28, 2016
jviotti pushed a commit that referenced this issue Jan 11, 2017
Somehow some development dependencies slipped into the shrinkwrap file,
and in some cases, dependencies we don't use anymore didn't remove its
own now unneeded dependencies when running `npm uninstall`.

This PR carefully removes the packages that are not needed anymore,
which are a lot.

See: #820
Signed-off-by: Juan Cruz Viotti <jviotti@openmailbox.org>
jviotti pushed a commit that referenced this issue Jan 11, 2017
Somehow some development dependencies slipped into the shrinkwrap file,
and in some cases, dependencies we don't use anymore didn't remove its
own now unneeded dependencies when running `npm uninstall`.

This PR carefully removes the packages that are not needed anymore,
which are a lot.

See: #820
Signed-off-by: Juan Cruz Viotti <jviotti@openmailbox.org>
@jviotti jviotti changed the title Reduce final package size Minify and concatenate JS/CSS/HTML Oct 15, 2018
@jviotti
Copy link
Contributor Author

jviotti commented Aug 15, 2022

Closing stale issues

@jviotti jviotti closed this as completed Aug 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants