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

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