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

From: Murtuza Zabuawala <murtuza(dot)zabuawala(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: Surinder Kumar <surinder(dot)kumar(at)enterprisedb(dot)com>, Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>, George Gelashvili <ggelashvili(at)pivotal(dot)io>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>, 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-18 15:35:18
Message-ID: CAKKotZSO54_EFN4-FuK1A=-ZMs4Tryq3N53LD0McFD0Bb62pvQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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
>

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message pgAdmin 4 Jenkins 2017-07-18 15:57:53 Jenkins build is back to normal : pgadmin4-master-python34 #236
Previous Message Dave Page 2017-07-18 15:31:17 Re: [pgAdmin4]: Webpacking of static JS/CSS