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

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