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

From: Surinder Kumar <surinder(dot)kumar(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
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 11:06:01
Message-ID: CAM5-9D8T3MBua-OE6bC1ALE+5BnoLi+h3CCqNMesbWUcGrdY1Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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
>

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message pgAdmin 4 Jenkins 2017-07-19 11:07:26 Build failed in Jenkins: pgadmin4-master-python26 #368
Previous Message Dave Page 2017-07-19 10:57:38 Re: [pgAdmin4]: Webpacking of static JS/CSS