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

​Hi

As previous patches gets stalled due to large size.
Please find attached patch.​

On Wed, Jul 12, 2017 at 12:23 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

>
>
> On 12 Jul 2017, at 02:56, Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>
> wrote:
>
> 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.
>
>
> They're way too big for the mailing lists (>13MB). Please have Surinder
> send you them privately.
>
​I think the patch​`webpack_bundles.patch` needs review and its size is now
around 500Kb. The other one is collection of deleted vendor files which
will be required when patch will be commited. So for now i will attach
first patch only.

>
>
> -- 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
>>>>>>
>>>>>
>>>>
>>>
>>
>