Subject: [pgAdmin4][Patch]: Load module's JS files only when required

From: Surinder Kumar <[email-masked]>
Date: Mon, 22 May 2017 11:20:26 +0530

Hi

As per pgAdmin4 design, template JS files can either be preloaded or load
when a specific node expands (by adding for e.g.: when: 'server').

The JS files of several modules found to be loaded when pgAdmin4 loads
which results in increasing:

   - the number of http requests
   - latency(greater request time)
   - pgAdmin4 load time

*Tested on Firefox:*

Before applying patch

   - http requests - 143
   - Content size - 3.4 MB
   - Load time: 4.1s (onload: 524ms)

After applying patch

   - http requests: 68
   - Content size: 2.1 MB
   - Load time: 2.84s (onload: 481ms)

Also, I found the http request for gravtar takes much time(depending on
internet speed) to load which increases load time.
https://secure.gravatar.com/avatar/d30aea269994f01256a99a8e7154a328?s=100&d=retro&r=g

Shouldn't the gravatar image be replaced with custom image ? I don't see
its any use except displaying for user.

*Patch changes:*

1) Load following modules JS when 'servers' node expands:

   -  Roles
   -  FileManager
   -  Backup
   -  Datagrid
   -  Grant wizard
   -  Import/Export, Maintenance & restore
   -  Sqleditor

2) Load Debugger JS when database node expands

3) Load Flotr2 when render function is called in dashboard.js

4) Load BigNumber library when sorting is performed on Statistics tab in
backgrid.pgadmin.js

5) Remove 'backgrid.select.all' from dependency list(user_management.js) as
it had no use.

After applying this patch, Menu items such "Query tool", "Grant wizard"
don't appear in Tools menu.
I have logged an RM2424 <https://redmine.postgresql.org/issues/2424> to
track this.

Please find attached patch and review.

Thanks
Surinder

-- 
Sent via pgadmin-hackers mailing list ([email-masked])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

---

From: Dave Page <[email-masked]>
Date: Mon, 22 May 2017 11:52:34 +0100

Hi

On Monday, May 22, 2017, Surinder Kumar <[email-masked]
<javascript:_e(%7B%7D,'cvml',[email-masked]');>> wrote:

> This is great work!

However (sorry!) - I'm planning on working on an alternative change on my
flight in a couple of hours. Joao has broken the Grunt code out of the
History tab patch for me to work with - the idea is something like:

- We continue to migrate all the JS out of templates and into static files
whereever possible, using the client-side translation.

- We then have a set of Grunt tasks that will collect all the static JS,
minimise it, and pack it into a single file to load at startup.

I'm currently thinking that non-debug/package builds will pre-pack
everything for installation. The aim is to have JS HTTP requests be down in
single figures to maximise loading time.

The are various issues to work through however; at least, how do we handle
debug builds, and how do we re-pack everything if a user installs a new
module (or updates something).

We'd also need to get rid of requirejs.

> its any use except displaying for user.

Maybe. Doesn't it load asynchronously though?

> 5) Remove 'backgrid.select.all' from dependency list(user_management.js)
> as it had no use.
>

OK. Lets hold fire on this for now though, until I can PoC the work above.

Of course, any thoughts or comments are welcome.

-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

---

From: Surinder Kumar <[email-masked]>
Date: Mon, 22 May 2017 18:09:56 +0530

Hi

On Mon, May 22, 2017 at 4:22 PM, Dave Page <[email-masked]> wrote:

> Hi
>
> On Monday, May 22, 2017, Surinder Kumar <[email-masked]>
> wrote:
>
>> Hi
>>
​Yes. It will be then easier to cover most of the JS code for minify if
client side ​translation is used.

> minimise it, and pack it into a single file to load at startup.
​I wrote few grunt tasks using r.js optimizer this weekend which:

1. Finds all static JS file recursively into the modules using regex
pattern and then creates the tasks to minify each JS file and store its
'min.js' in same directory and loads min.js when current_app.debug is False
else returns JS file.

2. A task to merge given list of CSS files into a single file and then
minify using 'grunt-contrib-cssmin' task and then insert that file with
<script> tags in base.html file.

> single figures to maximise loading time.
​That will make pgAdmin4 much faster.

> module (or updates something).
​I don't know much about builds but the debug/package builds can be
switched using 'current_app.debug' config parameter.
Using Regex Pattern will help to search JS files throughout the App
directory to perform minify task.

> We'd also need to get rid of requirejs.
​Any alternative to requirejs ?​

---

From: Dave Page <[email-masked]>
Date: Mon, 22 May 2017 22:33:13 +0100

Hi

On Monday, May 22, 2017, Surinder Kumar <[email-masked]>
wrote:

> Hi
>
> On Mon, May 22, 2017 at 4:22 PM, Dave Page <[email-masked]
> <javascript:_e(%7B%7D,'cvml',[email-masked]');>> wrote:
>
>> Hi
>>
>>> As per pgAdmin4 design, template JS files can either be preloaded or
>>> load when a specific node expands (by adding for e.g.: when: 'server').
>>>
>> - We continue to migrate all the JS out of templates and into static
>> files whereever possible, using the client-side translation.
>>

Unfortunately, I looked at this on my flight and it looks like it'll be a
mammoth task. Many/most JS files also currently use the url_for template
function - so we need to figure out how to replace that with JS code first.

> <script> tags in base.html file.

There's a script in the tools directory to do something similar already.

I am already starting to wonder if Grunt is the best way to do this though.
It might be easier (though not quite as efficient) to have the Python
module all return their static/template JS code at initialisation,
effectively dynamically building a single packed file containing nearly
everything.

>> The are various issues to work through however; at least, how do we
>> handle debug builds, and how do we re-pack everything if a user installs a
>> new module (or updates something).
>>

Yeah, though I think there are more considerations we haven't yet thought
of.

> ​Any alternative to requirejs ?​

If everything is packed together, then we don't need anything like it.

I think this needs more discussion.

---

From: Joao Pedro De Almeida Pereira <[email-masked]>
Date: Tue, 23 May 2017 14:45:09 -0400

Hello

​I wrote few grunt tasks using r.js optimizer this weekend
>
>  How did it compare to webpack?

> effectively dynamically building a single packed file containing nearly

We could. Are you saying that the app should bundle js every run? To be
fair, this is what we're currently doing with Grunt, but it feels like we
should change this for non-development use.

> of.

Something we aren't clear on is the end-user use case of dropping modules
into existing installations. Is this achieved via installer?

George & Joao

On Mon, May 22, 2017 at 5:33 PM, Dave Page <[email-masked]> wrote:

>>> However (sorry!) - I'm planning on working on an alternative change on
>>> my flight in a couple of hours. Joao has broken the Grunt code out of the
> I am already starting to wonder if Grunt is the best way to do this
> though. It might be easier (though not quite as efficient) to have the
> Python module all return their static/template JS code at initialisation,
>>>> Shouldn't the gravatar image be replaced with custom image ? I don't
>>>> see its any use except displaying for user.
>>>>
>>>
>>>> 4) Load BigNumber library when sorting is performed on Statistics tab
>>>> in backgrid.pgadmin.js
>>>>
>>>> 5) Remove 'backgrid.select.all' from dependency
>>>> list(user_management.js) as it had no use.
>>>>
>>>> I have logged an RM2424 <https://redmine.postgresql.org/issues/2424>
>>>> to track this.
>>>>
>>>

---

From: Dave Page <[email-masked]>
Date: Tue, 23 May 2017 16:21:55 -0400

Hi

On Tue, May 23, 2017 at 2:45 PM, Joao Pedro De Almeida Pereira
<[email-masked]> wrote:
> Hello
>
>
>>> I wrote few grunt tasks using r.js optimizer this weekend
>

For every non-debug run, it should check for changes in files, perhaps
by comparing the mtime of the packed file against all of the files it
would contain. However, that begs the question of what happens in an
installed instance, where the user running it likely doesn't have
permissions to re-pack the files if they're in the installation
directory. Perhaps packing should be done into a temp directory for
each user?

For debug runs, it should use the unpacked files in each module.

> into existing installations. Is this achieved via installer?

That's the intention, yes. It could be that the installer would
re-pack everything by calling a script we can supply, or the mechanism
I suggested above could handle it on next launch.

I'm actually thinking that maybe it would be easier to start with the
CSS files....

---

From: Dave Page <[email-masked]>
Date: Thu, 25 May 2017 17:32:26 -0400

> I'm actually thinking that maybe it would be easier to start with the

Which i did yesterday. However, it soon became obvious that that isn't
so easy either, as the CSS files contain url references which need to
be adjusted, which isn't trivial.

However, I did find a flask-webpack module, which looks interesting:

https://github.com/nickjj/flask-webpack
https://nickjanetakis.com/blog/manage-your-assets-with-flask-webpack

Surinder; when you have some free time, could you look into a simple
PoC with the CSS files using that please?

---

From: Surinder Kumar <[email-masked]>
Date: Fri, 26 May 2017 09:31:57 +0530

> PoC with the CSS files using that please?
​Ok. I will look

---

From: Dave Page <[email-masked]>
Date: Sun, 28 May 2017 20:15:16 -0400

On Fri, May 26, 2017 at 12:01 AM, Surinder Kumar
<[email-masked]> wrote:
> Ok. I will look

BTW; if it looks like it will take a while to implement (as I
suspect), we should consider polishing off your initial patch as an
interim step. Any thoughts on how long RM2424 would take to resolve?

---

From: Surinder Kumar <[email-masked]>
Date: Mon, 5 Jun 2017 14:52:36 +0530

Hi Dave,

Please find attached patch for minifying CSS files and optimize images
using Webpack.

*Steps to run:*

1) After applying patch, go to web directory and run npm install on
terminal to install packages which are used to parse/minify CSS files.

2) Run npm run build which creates dist folder inside web/pgadmin/static/
directory to store processed images, fonts and css files.

3. Set DEBUG = False in config_local.py and then run python pgAdmin.py to
start server.
I kept generated main.css and overrides.css conditional in base.html to
load them only when debug mode is False

After running "npm run build", following files/directories are generated:

1) main.css -  about 20 vendor CSS files are packed into this file and more
importantly the paths to images are preserved.

2) overrides.css - it contains bootstrap.overrides.css and pgadmin.css,
they has to be packed separately and loaded after all CSS files are loaded
because the purpose of these files is to override the vendor or modules css.

3) img - it contains the images used in CSS files. The name of image files
can also be hashed names for caching purpose which we can use.

4) fonts - it contains the fonts used in fontawesome.css and other css
files.

This is a simple patch to demonstrate how CSS files will be minified with
Webpack.

For now it minifies only vendor CSS files, I will add modules static files
in the list later on.

Any thoughts on minifying template CSS files which are built dynamically
and loaded with dependency on other modules?

Also, I looked into Flask-webpack which generates bundled assets using
Webpack(webpack.config.js) and provide additionally global template tags to
use in Jinja templates.

But it might not work with latest version of Webpack as this repo is not
updated since last 2 years. I need to check with latest version and I will
update on this.

Thanks
Surinder

---

From: Dave Page <[email-masked]>
Date: Tue, 6 Jun 2017 14:39:33 +0100

Hi

On Mon, Jun 5, 2017 at 10:22 AM, Surinder Kumar
<[email-masked]> wrote:
> Hi Dave,
>
> Please find attached patch for minifying CSS files and optimize images using
> Webpack.
>
> Steps to run:
>
> 1) After applying patch, go to web directory and run npm install on terminal
> to install packages which are used to parse/minify CSS files.
>
> I kept generated main.css and overrides.css conditional in base.html to load
> them only when debug mode is False
>
>
> 2) overrides.css - it contains bootstrap.overrides.css and pgadmin.css, they
> has to be packed separately and loaded after all CSS files are loaded

I think this is a good, simple method. It handles debug v.s release,
and of course, any plugin modules can include their own images/CSS
without even having to worry about webpacking if installed later.

> For now it minifies only vendor CSS files, I will add modules static files

I think we need to do that to get a better idea of the benefits. We
also need to get some of the JS code in there as well (Ashesh should
be able to help with that - he told me he's de-templatised a lot of
that now).

> Any thoughts on minifying template CSS files which are built dynamically and
> loaded with dependency on other modules?

Let's look at why they are templates. Is that required, or could they
be made static?

> updated since last 2 years. I need to check with latest version and I will

Given how straightforward this seems to be, I'm not sure it's needed.

Joao, any comments?

Nice work - thanks!

---

From: Joao Pedro De Almeida Pereira <[email-masked]>
Date: Wed, 7 Jun 2017 14:10:04 -0400

Hello Dave,
By the description on the email, it looks like the process is very similar
to the Grunt process that we were talking about, but without the patch, it
is a bit hard to get more specifics.
Can you forward the patch so we can take a look at it.

We believe that every effort done to eliminate templated Javascript and CSS
files is well worth it, and should be followed.

Thanks
Joao & Sarah

On Tue, Jun 6, 2017 at 9:39 AM, Dave Page <[email-masked]> wrote:

> using
> terminal
> they

---

From: Dave Page <[email-masked]>
Date: Thu, 8 Jun 2017 12:43:59 +0100

Hi

> files is well worth it, and should be followed.

Odd - the patch was sent to you as well. Oh well, here it is.