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

From: Surinder Kumar <surinder(dot)kumar(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: Sarah McAlear <smcalear(at)pivotal(dot)io>, George Gelashvili <ggelashvili(at)pivotal(dot)io>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>, Matthew Kleiman <mkleiman(at)pivotal(dot)io>
Subject: Re: [pgAdmin4]: Webpacking of static JS/CSS
Date: 2017-07-11 07:57:24
Message-ID: CAM5-9D_C+-29XT80_9RZ9E3WHYrW9WrFZGyG5D=v4q7KMmkStQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2017-07-11 08:12:12 pgAdmin 4 commit: Ensure dependencies are packaged in the tarball
Previous Message Surinder Kumar 2017-07-11 07:55:03 Re: [pgAdmin4]: Webpacking of static JS/CSS