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.ku...@enterprisedb.com> wrote: > On Wed, Jul 19, 2017 at 4:27 PM, Dave Page <dp...@pgadmin.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.ku...@enterprisedb.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.zabuaw...@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 <dp...@pgadmin.org> wrote: >>>> >>>>> Thanks, applied. >>>>> >>>>> On Tue, Jul 18, 2017 at 4:12 PM, Surinder Kumar < >>>>> surinder.ku...@enterprisedb.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.va...@enterprisedb.com> wrote: >>>>>> >>>>>>> >>>>>>> >>>>>>> On Tue, Jul 18, 2017 at 7:46 PM, Dave Page <dp...@pgadmin.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.ku...@enterprisedb.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.ku...@enterprisedb.com> wrote: >>>>>>>>> >>>>>>>>>> On Mon, Jul 17, 2017 at 4:52 PM, Dave Page <dp...@pgadmin.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.ku...@enterprisedb.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.ku...@enterprisedb.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 < >>>>>>>>>>>>> smcal...@pivotal.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 >> > >
webpack_minor_issues.patch
Description: Binary data
