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

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?

-- Rob

On Jul 11, 2017 11:37 AM, "Dave Page" <dave(dot)page(at)enterprisedb(dot)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(dot)kumar(at)enterprisedb(dot)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(dot)kumar(at)enterprisedb(dot)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(dot)page(at)enterprisedb(dot)com>
>>> wrote:
>>>
>>>> Nice!
>>>>
>>>> On Tue, Jul 11, 2017 at 9:42 AM, Neel Patel <
>>>> neel(dot)patel(at)enterprisedb(dot)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(dot)kumar(at)enterprisedb(dot)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(dot)kumar(at)enterprisedb(dot)com> wrote:
>>>>>>
>>>>>>>
>>>>>>> On Tue, Jul 11, 2017 at 1:22 PM, Dave Page <dpage(at)pgadmin(dot)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(dot)kumar(at)enterprisedb(dot)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 <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.
>>>>>>>>>>
>>>>>>>>>> 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
>