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

On Tue, Jul 11, 2017 at 10:29 PM, Surinder Kumar <
surinder(dot)kumar(at)enterprisedb(dot)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 <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/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 <
>>>>>>>>>>>>> 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
>>>>>
>>>>
>>>
>>
>