On Tue, Jul 11, 2017 at 10:29 PM, Surinder Kumar < surinder.ku...@enterprisedb.com> wrote:
> Robert, > > I have attached two patches: > > 1. webpack_bundles.patch > > 2. unvendor_files.patch > > in this email chain. > > If you didn't received those patches possibly due to large size of patch, > let me know i will send again. > Dave, I don't see these two patches too in the mail-chain. By any chance, it is stalled. -- Thanks, Ashesh > > > On Tue, Jul 11, 2017 at 10:24 PM, Robert Eckhardt <reckha...@pivotal.io> > wrote: > >> 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/backbone') >>>>>>>>>>>>> >>>>>>>>>>>>> 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 >>>>> >>>> >>> >> >