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

From: Surinder Kumar <surinder(dot)kumar(at)enterprisedb(dot)com>
To: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Cc: Dave Page <dpage(at)pgadmin(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 10:21:24
Message-ID: CAM5-9D--1WDz4MLogHcTSvrRWt-Hr3Xav+JtNQLypfahY3XtQQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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.zabuawala@
enterprisedb.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
>>
>
>

Attachment Content-Type Size
fix_library_reference.patch application/octet-stream 819 bytes

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2017-07-19 10:43:49 pgAdmin 4 commit: Add controls and shortcuts for commenting/uncommentin
Previous Message Murtuza Zabuawala 2017-07-19 08:56:17 Re: [pgAdmin4][Patch]: Allow user to Comment/Uncomment code in query editor