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

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
>>
>
>