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 10:57:38 |
Message-ID: | CA+OCxownDp-RfwYos=9COpGXvsz08iA2d-vB6bjPqYEqD4fk6g@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgadmin-hackers |
That was a fun one to spot I'm sure!
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
From | Date | Subject | |
---|---|---|---|
Next Message | Surinder Kumar | 2017-07-19 11:06:01 | Re: [pgAdmin4]: Webpacking of static JS/CSS |
Previous Message | Dave Page | 2017-07-19 10:57:14 | pgAdmin 4 commit: Fix typo in filename. |