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

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.

On Tue, Jul 11, 2017 at 10:24 PM, Robert Eckhardt <reckhardt(at)pivotal(dot)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(dot)kumar(at)enterprisedb(dot)com> wrote:
>
>> On Tue, Jul 11, 2017 at 10:18 PM, Robert Eckhardt <reckhardt(at)pivotal(dot)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(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
>>>>
>>>
>>
>