Re: [pgAdmin4]: Webpacking of static JS/CSS - Mailing list pgadmin-hackers

​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@pgadmin.org> wrote:

On 12 Jul 2017, at 02:56, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

On Tue, Jul 11, 2017 at 10:29 PM, Surinder Kumar <surinder.kumar@enterprisedb.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@pivotal.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.kumar@enterprisedb.com> wrote:

On Tue, Jul 11, 2017 at 10:18 PM, Robert Eckhardt <reckhardt@pivotal.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.page@enterprisedb.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.kumar@enterprisedb.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.kumar@enterprisedb.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

Please find updated patch.

Thanks

Surinder

On Tue, Jul 11, 2017 at 2:13 PM, Dave Page <dave.page@enterprisedb.com> wrote:

Nice!

On Tue, Jul 11, 2017 at 9:42 AM, Neel Patel <neel.patel@enterprisedb.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.kumar@enterprisedb.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.kumar@enterprisedb.com> wrote:

On Tue, Jul 11, 2017 at 1:22 PM, Dave Page <dpage@pgadmin.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.kumar@enterprisedb.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

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

3) Acitree - it is not available in yarn repository. logged an request

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. The latest version of actiree has issues so code is used form 4.5.0-rc.7 tag. 

Thanks to bestander for helping this out. link to thread

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@pivotal.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