Surinder, Sorry I'm missing the email can you tell me the name please.
-- Rob On Tue, Jul 11, 2017 at 12:51 PM, Surinder Kumar < surinder.ku...@enterprisedb.com> wrote: > On Tue, Jul 11, 2017 at 10:18 PM, Robert Eckhardt <reckha...@pivotal.io> > wrote: > >> The last email on this chain from Surinder says that it isn't working on >> IE and he will submit another patch. Are we missing that patch? Would you >> like us to look at the previous patch? >> > Yes previous patch includes fix related to IE. > >> >> -- Rob >> >> >> On Jul 11, 2017 11:37 AM, "Dave Page" <dave.p...@enterprisedb.com> wrote: >> >>> Pivotal team; you guys are far more familiar with webpack etc. than we >>> are; could you review please? >>> >>> Thanks! >>> >>> On Tue, Jul 11, 2017 at 4:24 PM, Surinder Kumar < >>> surinder.ku...@enterprisedb.com> wrote: >>> >>>> Hi >>>> >>>> 1) Created a separated patch for Un-vendored libraries and another one >>>> related to webpack bundle files so it is easy to review. >>>> >>>> 2) Removed commented out code and dead code. >>>> >>>> 3) Feature test cases are fixed. All are running. >>>> I have to add `time.sleep` of 1 second in method >>>> 'fill_codemirror_area_with' as sometimes it work and sometimes don't. >>>> >>>> 4. Removed unused libraries from package.json >>>> >>>> Please find updated patch and review. >>>> >>>> Thanks, >>>> Surinder >>>> >>>> >>>> >>>> On Tue, Jul 11, 2017 at 3:11 PM, Surinder Kumar < >>>> surinder.ku...@enterprisedb.com> wrote: >>>> >>>>> Hi >>>>> >>>>> This patch doesn't work in windows IE 11 due to error `'Promise' is >>>>> undefined >>>>> `. >>>>> >>>>> The dependency package 'babel-polyfill' is required to run ES6 code >>>>> with webpack and has to load before at entry point of app. >>>>> related thread <https://github.com/webpack/webpack/issues/4254> >>>>> >>>>> Please find updated patch. >>>>> >>>>> Thanks >>>>> Surinder >>>>> >>>>> On Tue, Jul 11, 2017 at 2:13 PM, Dave Page <dave.p...@enterprisedb.com >>>>> > wrote: >>>>> >>>>>> Nice! >>>>>> >>>>>> On Tue, Jul 11, 2017 at 9:42 AM, Neel Patel < >>>>>> neel.pa...@enterprisedb.com> wrote: >>>>>> >>>>>>> Hi Dave, >>>>>>> >>>>>>> I have tested Surinder's patch in my local machine with Qt 5.9.1 >>>>>>> Webkit on Windows and we are getting improvements with this patch. :) >>>>>>> >>>>>>> Below performance tested with Qt 5.9.1 with annulen webkit v 5.212.0 >>>>>>> Alpha2. >>>>>>> >>>>>>> *Before Webpack patch:-* >>>>>>> >>>>>>> It took ~20 seconds. This 20 seconds includes when user double click >>>>>>> on application ( timing of python server start + page load ) >>>>>>> >>>>>>> *After Webpack patch:-* >>>>>>> >>>>>>> It took ~13 seconds ( timing of python server start + page load ). >>>>>>> >>>>>>> We got ~7 seconds improvement with webpack on same machine & same Qt >>>>>>> configuration and this will be useful in windows performance issue as >>>>>>> well >>>>>>> :) >>>>>>> >>>>>>> Thanks, >>>>>>> Neel Patel >>>>>>> >>>>>>> On Tue, Jul 11, 2017 at 1:27 PM, Surinder Kumar < >>>>>>> surinder.ku...@enterprisedb.com> wrote: >>>>>>> >>>>>>>> Hi >>>>>>>> >>>>>>>> I found this patch won't work on IE, so i have fixed it and will >>>>>>>> send updated patch. >>>>>>>> >>>>>>>> On Tue, Jul 11, 2017 at 1:25 PM, Surinder Kumar < >>>>>>>> surinder.ku...@enterprisedb.com> wrote: >>>>>>>> >>>>>>>>> >>>>>>>>> On Tue, Jul 11, 2017 at 1:22 PM, Dave Page <dp...@pgadmin.org> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>>> When you say "un-vendorising", do you mean removing them from the >>>>>>>>>> vendor directory and adding them to packages.json so they're pulled >>>>>>>>>> in via >>>>>>>>>> npm/yarn? (if so, good :-) ) >>>>>>>>>> >>>>>>>>> Yes. >>>>>>>>> >>>>>>>>>> >>>>>>>>>> On Tue, Jul 11, 2017 at 7:34 AM, Surinder Kumar < >>>>>>>>>> surinder.ku...@enterprisedb.com> wrote: >>>>>>>>>> >>>>>>>>>>> Hi >>>>>>>>>>> >>>>>>>>>>> *Detailed changes:* >>>>>>>>>>> >>>>>>>>>>> 1) Created a file bundle/app.js which loads `browser.js` and >>>>>>>>>>> then 'tools.datagrid' and its other dependents are configured to >>>>>>>>>>> load in >>>>>>>>>>> '`imports-loader` >>>>>>>>>>> >>>>>>>>>>> 2) bundle/codemirror.js - it generates a bundled JS which is >>>>>>>>>>> used wherever required in various modules. >>>>>>>>>>> >>>>>>>>>>> 3) file_utils.js - It bundles the file manager's utility JS file >>>>>>>>>>> and loaded from `file manager/index.html` >>>>>>>>>>> >>>>>>>>>>> 4) lib_css - It contains list of vendor CSS to be bundled. >>>>>>>>>>> >>>>>>>>>>> 5) pgadmin_css - It contains list of overrides CSS which are >>>>>>>>>>> bundled and which has to load after vendors CSS is loaded. >>>>>>>>>>> >>>>>>>>>>> *Various Webpack configuration parameters:* >>>>>>>>>>> >>>>>>>>>>> 1) externals: List of template files to be loaded dynamically >>>>>>>>>>> when a call is made by dependent modules. These files are excluded >>>>>>>>>>> from the >>>>>>>>>>> bundled JS. >>>>>>>>>>> >>>>>>>>>>> 2) resolve > alias - This resolves the path of module JS which >>>>>>>>>>> is referred in other modules; For example: >>>>>>>>>>> 'backbone': path.join(__dirname, './node_modules/backbone/backb >>>>>>>>>>> one') >>>>>>>>>>> >>>>>>>>>>> 3) `backbone` is used in 'define(['backbone'])' calls and its >>>>>>>>>>> path is resolved to above absolute path. >>>>>>>>>>> >>>>>>>>>>> 4) 'pgLibs': List of files to bundle into `pgadmin_commons.js`. >>>>>>>>>>> The purpose is to separate files other than browser node modules JS >>>>>>>>>>> in >>>>>>>>>>> `pgadmin_commons.js` and browser node modules JS in >>>>>>>>>>> `app.bundle.js`. It >>>>>>>>>>> will help in debugging the code during development. >>>>>>>>>>> >>>>>>>>>>> *Un-vendor modules:* >>>>>>>>>>> >>>>>>>>>>> All modules are un-vendored except: >>>>>>>>>>> - requireJS >>>>>>>>>>> - Backgrid JS - it gives reference error when importing >>>>>>>>>>> backgrid.css from node_modules in bundle/lib.css even if the path is >>>>>>>>>>> correct. I logged an issue: >>>>>>>>>>> Opened an issue >>>>>>>>>>> <https://github.com/webpack-contrib/css-loader/issues/567> >>>>>>>>>>> >>>>>>>>>>> - React JS - I didn't un-vendor React JS because the pivotal >>>>>>>>>>> developers submitted a patch to fix issue of QtWebEngine. Once the >>>>>>>>>>> patch is >>>>>>>>>>> merged in React repo, we will un-vendor. >>>>>>>>>>> >>>>>>>>>>> *Other issues faced:* >>>>>>>>>>> >>>>>>>>>>> 1) Backbone JS: I didn't upgraded backbone JS to latest because >>>>>>>>>>> it affects the application code mainly `Preferences module`. >>>>>>>>>>> >>>>>>>>>>> 2) jQuery: I didn't upgraded it to latest because it is gives >>>>>>>>>>> error in loading wcIframe of wcDocker in Query tool. I submit a >>>>>>>>>>> PR <https://github.com/WebCabin/wcDocker/pull/118>. >>>>>>>>>>> >>>>>>>>>>> 3) Acitree - it is not available in yarn repository. logged an >>>>>>>>>>> request <https://github.com/dragosu/jquery-aciTree/issues/15> >>>>>>>>>>> However i am managing it on my github account by forking this >>>>>>>>>>> repo. >>>>>>>>>>> >>>>>>>>>>> Specified the repo github link to `acitree` in package.json with >>>>>>>>>>> tag to pull from >>>>>>>>>>> 4.5.0-rc.7 >>>>>>>>>>> <https://github.com/imsurinder90/jquery-aciTree.git#4.5.0-rc.7>. >>>>>>>>>>> The latest version of actiree has issues so code is used form >>>>>>>>>>> 4.5.0-rc.7 >>>>>>>>>>> tag. >>>>>>>>>>> >>>>>>>>>>> Thanks to bestander <https://github.com/bestander> for helping >>>>>>>>>>> this out. link to thread >>>>>>>>>>> <https://github.com/yarnpkg/yarn/issues/1747#issuecomment-312502008> >>>>>>>>>>> >>>>>>>>>>> *Plugins used:* >>>>>>>>>>> >>>>>>>>>>> - optimize-css-assets-webpack-plugin: its job is to optimise >>>>>>>>>>> the CSS bundle like minifying CSS and removing comments from it. >>>>>>>>>>> [used only >>>>>>>>>>> in production] >>>>>>>>>>> >>>>>>>>>>> - uglifyJS - It minimise the bundled JS, and remove console >>>>>>>>>>> statements from code. [used only in production] >>>>>>>>>>> >>>>>>>>>>> - definePlugin - It helps in minimising the `React' production >>>>>>>>>>> bundle. As react has conditional code based on 'NODE_ENV' variable. >>>>>>>>>>> [used >>>>>>>>>>> only in production] >>>>>>>>>>> >>>>>>>>>>> - providePlugin - It makes the variable defined through out the >>>>>>>>>>> application context. For example: jQuery object can be accessed >>>>>>>>>>> wherever >>>>>>>>>>> it is used but not included in `define` calls. >>>>>>>>>>> >>>>>>>>>>> - CommonsChunkPlugin - it helps in separating vendor like code, >>>>>>>>>>> common dependent modules used in multiple modules. it extracts out >>>>>>>>>>> in a >>>>>>>>>>> file. >>>>>>>>>>> >>>>>>>>>>> - extractTextPlugin - it is used in combination with >>>>>>>>>>> 'css-loader' and 'style-loader'. It helps in extracting CSS and >>>>>>>>>>> moved into >>>>>>>>>>> files. >>>>>>>>>>> >>>>>>>>>>> - webpack-bundle-analyzer - it helps in analysing the bundled JS >>>>>>>>>>> files like size of bundled JS and which JS files are included in >>>>>>>>>>> bundle. It >>>>>>>>>>> is commented out. [Use only when need to analyse] >>>>>>>>>>> >>>>>>>>>>> *Loaders used:* >>>>>>>>>>> >>>>>>>>>>> - shim-loader: It manages the module dependency, uses the same >>>>>>>>>>> configuration used in requireJS >>>>>>>>>>> >>>>>>>>>>> - imports-loader: It also used to loaded dependent modules which >>>>>>>>>>> are defined in its `use` setting. >>>>>>>>>>> >>>>>>>>>>> - file-loader - It helps in extracting font, image files into a >>>>>>>>>>> separate directory. >>>>>>>>>>> >>>>>>>>>>> - css-loader - Imports css files included in modules using >>>>>>>>>>> `require` and `import` calls. >>>>>>>>>>> >>>>>>>>>>> *TODO:* >>>>>>>>>>> >>>>>>>>>>> 1) Automatically handle static and template JS files: This is >>>>>>>>>>> already being discussed. Once it is sorted out, we will change >>>>>>>>>>> webpack >>>>>>>>>>> configuration accordingly. >>>>>>>>>>> >>>>>>>>>>> 2) Implementing Caching: I will look into this once an initial >>>>>>>>>>> patch is commited. and later on add as improvement. >>>>>>>>>>> >>>>>>>>>>> 3) Source maps: It will help in debugging bundled JS code in >>>>>>>>>>> production environment. >>>>>>>>>>> >>>>>>>>>>> 4) Feature tests are failing: I am currently looking into it. >>>>>>>>>>> Query tool functionality is working fine, the issue is it is unable >>>>>>>>>>> to find >>>>>>>>>>> code mirror. >>>>>>>>>>> >>>>>>>>>>> *Steps to run:* >>>>>>>>>>> >>>>>>>>>>> After applying patch:* git apply --binary /path/to/patch* >>>>>>>>>>> >>>>>>>>>>> run* `yarn install`* >>>>>>>>>>> then run: >>>>>>>>>>> >>>>>>>>>>> **In development mode: >>>>>>>>>>> *yarn run bundle:dev* >>>>>>>>>>> >>>>>>>>>>> In production mode: >>>>>>>>>>> *yarn run bundle:prod* >>>>>>>>>>> >>>>>>>>>>> *Performance comparison:* >>>>>>>>>>> >>>>>>>>>>> On Mac's Chrome - Before bundle it was taking ~9sec to load >>>>>>>>>>> page. After bundle it took 3.5secs (with no cache) >>>>>>>>>>> >>>>>>>>>>> Please review the patch. >>>>>>>>>>> >>>>>>>>>>> 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. >>>>>>>>>>>> >>>>>>>>>>>> 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. >>>>>>>>>>>> >>>>>>>>>>>> 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?) >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>>> *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? >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> Thanks, >>>>>>>>>>>> >>>>>>>>>>>> George & Sarah >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> -- >>>>>>>>>> Dave Page >>>>>>>>>> Blog: http://pgsnake.blogspot.com >>>>>>>>>> Twitter: @pgsnake >>>>>>>>>> >>>>>>>>>> EnterpriseDB UK: http://www.enterprisedb.com >>>>>>>>>> The Enterprise PostgreSQL Company >>>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> Dave Page >>>>>> VP, Chief Architect, Tools & Installers >>>>>> EnterpriseDB: http://www.enterprisedb.com >>>>>> The Enterprise PostgreSQL Company >>>>>> >>>>>> Blog: http://pgsnake.blogspot.com >>>>>> Twitter: @pgsnake >>>>>> >>>>> >>>>> >>>> >>> >>> >>> -- >>> Dave Page >>> VP, Chief Architect, Tools & Installers >>> EnterpriseDB: http://www.enterprisedb.com >>> The Enterprise PostgreSQL Company >>> >>> Blog: http://pgsnake.blogspot.com >>> Twitter: @pgsnake >>> >> >
