Re: [pgAdmin4]: Webpacking of static JS/CSS

From: Dave Page <dpage(at)pgadmin(dot)org>
To: Surinder Kumar <surinder(dot)kumar(at)enterprisedb(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>, Murtuza Zabuawala <murtuza(dot)zabuawala(at)enterprisedb(dot)com>, Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>, George Gelashvili <ggelashvili(at)pivotal(dot)io>, Matthew Kleiman <mkleiman(at)pivotal(dot)io>, Sarah McAlear <smcalear(at)pivotal(dot)io>
Subject: Re: [pgAdmin4]: Webpacking of static JS/CSS
Date: 2017-07-19 14:03:50
Message-ID: CA+OCxozit_ie8CZC=OLm1i2MQVVWB03J0hGGWUyZf3aKTp2xhA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Thanks, applied.

On Wed, Jul 19, 2017 at 2:40 PM, Surinder Kumar <
surinder(dot)kumar(at)enterprisedb(dot)com> wrote:

> Hi,
>
> PFA patch for few code review comments given by Ashesh.
>
> Thanks,
> Surinder
>
>
> On Wed, Jul 19, 2017 at 4:36 PM, Surinder Kumar <
> surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
>
>> On Wed, Jul 19, 2017 at 4:27 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>
>>> That was a fun one to spot I'm sure!
>>>
>> ​Indeed, i had setup pgAdmin evn on Linux(as it works on Mac) and then i
>> did `ls path/to/jquery.contextmenu.js`, it was spotted :)
>>
>>>
>>> Thanks, committed.
>>>
>>> On Wed, Jul 19, 2017 at 11:21 AM, Surinder Kumar <
>>> surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hi
>>>>
>>>> Fix library path reference for `jquery.contextmenu'. This issue was
>>>> only reproducible on Linux machine.
>>>> So, i setup pgAdmin4 on Ubuntu VM and tested the patch and it works.
>>>>
>>>> Please find attached patch.
>>>>
>>>> Thanks,
>>>> Surinder
>>>>
>>>> On Tue, Jul 18, 2017 at 9:05 PM, Murtuza Zabuawala <
>>>> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> Great job Surinder, Load time ~2 sec on browser :)
>>>>>
>>>>> [image: Inline image 1]
>>>>>
>>>>>
>>>>> --
>>>>> Regards,
>>>>> Murtuza Zabuawala
>>>>> EnterpriseDB: http://www.enterprisedb.com
>>>>> The Enterprise PostgreSQL Company
>>>>>
>>>>> On Tue, Jul 18, 2017 at 9:01 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>>
>>>>>> Thanks, applied.
>>>>>>
>>>>>> On Tue, Jul 18, 2017 at 4:12 PM, Surinder Kumar <
>>>>>> surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
>>>>>>
>>>>>>> Hi
>>>>>>>
>>>>>>> 1. As Slickgrid has dependency of `jQuery-ui`, it was missed. now
>>>>>>> added.
>>>>>>> 2. Column sorting for collection nodes sometimes failing when
>>>>>>> clicked on different collection nodes.
>>>>>>>
>>>>>>> Please find attached patch.
>>>>>>>
>>>>>>> Thanks
>>>>>>> Surinder
>>>>>>>
>>>>>>> On Tue, Jul 18, 2017 at 8:20 PM, Khushboo Vashi <
>>>>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On Tue, Jul 18, 2017 at 7:46 PM, Dave Page <dpage(at)pgadmin(dot)org>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> Thanks - applied!
>>>>>>>>>
>>>>>>>>> Awesome work - on an average of 3 tests on my Mac, load time
>>>>>>>>> reduced from 11.55s with v1.6 to 5.53s with GIT Head.
>>>>>>>>>
>>>>>>>> ​Thanks to all​
>>>>>>>
>>>>>>>>
>>>>>>>>> Surinder, great work...
>>>>>>>>
>>>>>>>>
>>>>>>>>> On Mon, Jul 17, 2017 at 5:57 PM, Surinder Kumar <
>>>>>>>>> surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
>>>>>>>>>
>>>>>>>>>> Hi
>>>>>>>>>>
>>>>>>>>>> Now all test cases are executing.
>>>>>>>>>> Please find updated patch.
>>>>>>>>>>
>>>>>>>>>> Thanks
>>>>>>>>>> Surinder
>>>>>>>>>>
>>>>>>>>>> On Mon, Jul 17, 2017 at 6:57 PM, Surinder Kumar <
>>>>>>>>>> surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>
>>>>>>>>>>> On Mon, Jul 17, 2017 at 4:52 PM, Dave Page <dpage(at)pgadmin(dot)org>
>>>>>>>>>>> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> Hi
>>>>>>>>>>>>
>>>>>>>>>>>> No errors now, but do you know why JS tests are being skipped?
>>>>>>>>>>>>
>>>>>>>>>>> ​No errors/warning in console even after settings `logLevel:
>>>>>>>>>>> config.LOG_DEBUG`. I am still debugging.
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> PhantomJS 2.1.1 (Mac OS X 0.0.0): Executed 4 of 216 (skipped
>>>>>>>>>>>> 212) SUCCESS (0.085 secs / 0.046 secs)
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks!
>>>>>>>>>>>>
>>>>>>>>>>>> On Mon, Jul 17, 2017 at 12:07 PM, Surinder Kumar <
>>>>>>>>>>>> surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> ​Hi Dave,
>>>>>>>>>>>>>
>>>>>>>>>>>>> I didn't removed the vendor modules when i ran regression test
>>>>>>>>>>>>> cases, so modules were being referenced from vendor dir and passed for me.
>>>>>>>>>>>>> Now I have fixed path references and test cases are working.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Please find attached patch.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks
>>>>>>>>>>>>> Surinder​
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Mon, Jul 17, 2017 at 3:18 PM, Surinder Kumar <
>>>>>>>>>>>>> surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Hi
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I'm currently working on first TODO: "Automatically handle
>>>>>>>>>>>>>> static and template JS files"
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> As discussed with Ashesh, currently the paths to module id
>>>>>>>>>>>>>> are written manually in webpack.config.js, instead the path defined in
>>>>>>>>>>>>>> moudle's `def get_own_javascript()` should be used.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> So, we will be generating a paths.json file which will
>>>>>>>>>>>>>> contain:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 1. resolve > alias - path with reference to module id.(Static
>>>>>>>>>>>>>> files)
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 2. externals - list of modules to be loaded dynamically on
>>>>>>>>>>>>>> demand(Template files)
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 3. Shim module dependency
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 4. List of JS modules to be loaded in specified order.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> *Implementation:*
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> To generate `paths.json` file, we will be using `Flask's
>>>>>>>>>>>>>> test_client` to make an http request internally within the app context so
>>>>>>>>>>>>>> we can call `current_app.javascripts` property and return the list of JS
>>>>>>>>>>>>>> paths and write those into paths.json file and then use it in
>>>>>>>>>>>>>> webpack.shim.js before the execution of `yarn run bundle` in
>>>>>>>>>>>>>> `javascript_bundler.py`
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> *For example:*
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> @app.route('/get_script_paths')
>>>>>>>>>>>>>> def get_script_paths():
>>>>>>>>>>>>>> from flask import current_app
>>>>>>>>>>>>>> from pgadmin.utils.ajax import make_json_response
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> return make_json_response(data=current_app.javascripts)
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> if config.DEBUG:
>>>>>>>>>>>>>> with app.test_client() as client:
>>>>>>>>>>>>>> import simplejson as json
>>>>>>>>>>>>>> list_scripts = client.get('/get_script_paths')
>>>>>>>>>>>>>> scripts = json.loads(list_scripts.data)
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> javascriptBundler = JavascriptBundler()
>>>>>>>>>>>>>> javascriptBundler.bundle(scripts['data'])
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> This also needs little change in module dependency we defined
>>>>>>>>>>>>>> using 'When': 'node_name' in `def get_own_javascripts(...)` method
>>>>>>>>>>>>>> the module specified(name: module_name) is loaded when module
>>>>>>>>>>>>>> given in `When` is expanded in node. Since we are using Webpack in which
>>>>>>>>>>>>>> behaviour to load module is little different.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Now in webpack we are using `imports-loader` to load specific
>>>>>>>>>>>>>> modules. So this is how it should work.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 1. First load all modules which do not have dependency on any
>>>>>>>>>>>>>> node like 'about', 'dashboard', 'server-group', 'server' etc.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 2. Load module such as `Databases` node first before its
>>>>>>>>>>>>>> child nodes are loaded.
>>>>>>>>>>>>>> Similarly load `Schemas` node before its child nodes are
>>>>>>>>>>>>>> loaded as they are dependent on parent node.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>> Surinder
>>>>>>>>>>>>>> On Wed, Jul 5, 2017 at 8:22 PM, Sarah McAlear <
>>>>>>>>>>>>>> smcalear(at)pivotal(dot)io> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Hello,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> *​Things to discuss:*
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> How to differentiate between a static and template JS
>>>>>>>>>>>>>>>> ​​
>>>>>>>>>>>>>>>> .
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> What is the advantage of webpacking templated JS? It seems
>>>>>>>>>>>>>>> as though this creates a system in which the bundled dependencies have to
>>>>>>>>>>>>>>> refer back to the backend to load the templates.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>> ​Templated JS will not be part of generated bundle JS, they
>>>>>>>>>>>>>> will load externally( an extra request will be made to server For example:
>>>>>>>>>>>>>> translations.js)
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> If there is a performance win in packing templated JS then
>>>>>>>>>>>>>>> looking at it makes sense. Otherwise it may make sense to put off until it
>>>>>>>>>>>>>>> is clear that the templated files should be dealt with by either
>>>>>>>>>>>>>>> de-templating them or bundling them where there is a clear reason.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>> ​Template JS cannot be bundled, so i extract the <Jinja> code
>>>>>>>>>>>>>> from template files and put into a separate file - ABC.js (also moved
>>>>>>>>>>>>>> template files to static directory) and then load ABC.js dynamically as
>>>>>>>>>>>>>> dependency of other modules.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> However, we're wondering about possible performance
>>>>>>>>>>>>>>> penalties with templating larger files (as opposed to templating
>>>>>>>>>>>>>>> on-demand.) Since jinja templates can execute arbitrary python, this could
>>>>>>>>>>>>>>> get time expensive and further slow things like initial page-load.
>>>>>>>>>>>>>>> Another concern is: what happens when a template gets out of
>>>>>>>>>>>>>>> date (e.g. if browser.js had previously filled in the content for
>>>>>>>>>>>>>>> 'panel_item.content' and had been cached, would it render a new version
>>>>>>>>>>>>>>> with the new values when needed? Or is it possible that we would get old
>>>>>>>>>>>>>>> content?)
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>> ​That file will always gets new content when loaded
>>>>>>>>>>>>>> dynamically, the content is not cached.​
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> *Taks remaining:*
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> ​1. ​
>>>>>>>>>>>>>>>> Fix local variables which are declared without using var,
>>>>>>>>>>>>>>>> have to check in each file
>>>>>>>>>>>>>>>> ​ by​
>>>>>>>>>>>>>>>> running eslint (For now, i will fix only errors which are
>>>>>>>>>>>>>>>> giving error in browser).
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> ​2. ​
>>>>>>>>>>>>>>>> Move non-template files from ’templates’ to ’static’
>>>>>>>>>>>>>>>> directory. List of
>>>>>>>>>>>>>>>> ​ pending​
>>>>>>>>>>>>>>>> modules is here:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> - Tools (mostly all modules - 9 modules)
>>>>>>>>>>>>>>>> - Browser nodes - 3 modules(resource group, roles,
>>>>>>>>>>>>>>>> tablespace)
>>>>>>>>>>>>>>>> - ​About
>>>>>>>>>>>>>>>> ​​
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Also can we move
>>>>>>>>>>>>>>>> ​'​
>>>>>>>>>>>>>>>> dashboard, statistic
>>>>>>>>>>>>>>>> ​s​
>>>>>>>>>>>>>>>> , preferences and help
>>>>>>>>>>>>>>>> ​'​
>>>>>>>>>>>>>>>> modules inside misc to preserve modularity as pgAdmin is
>>>>>>>>>>>>>>>> modular
>>>>>>>>>>>>>>>> ​ ?​
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Is there anything from a organization stance you discussed
>>>>>>>>>>>>>>> in the previous email that needs to be done to make this usable and
>>>>>>>>>>>>>>> consistent?
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>> ​No​
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> George & Sarah
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> --
>>>>>>>>>>>> Dave Page
>>>>>>>>>>>> Blog: http://pgsnake.blogspot.com
>>>>>>>>>>>> Twitter: @pgsnake
>>>>>>>>>>>>
>>>>>>>>>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>>>>>>>>>> The Enterprise PostgreSQL Company
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> Dave Page
>>>>>>>>> Blog: http://pgsnake.blogspot.com
>>>>>>>>> Twitter: @pgsnake
>>>>>>>>>
>>>>>>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>>>>>>> The Enterprise PostgreSQL Company
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Dave Page
>>>>>> Blog: http://pgsnake.blogspot.com
>>>>>> Twitter: @pgsnake
>>>>>>
>>>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>>>> The Enterprise PostgreSQL Company
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EnterpriseDB UK: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>
>>
>

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

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

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message pgAdmin 4 Jenkins 2017-07-19 14:27:28 Build failed in Jenkins: pgadmin4-master-python26 #369
Previous Message Dave Page 2017-07-19 14:03:24 pgAdmin 4 commit: Webpacking cleanups