Re: [pgAdmin4][Patch]: Run MakeFile to generate builds in production mode - Mailing list pgadmin-hackers

From Surinder Kumar
Subject Re: [pgAdmin4][Patch]: Run MakeFile to generate builds in production mode
Date
Msg-id CAM5-9D9paKp2dcmnT+BaFpRpO7+ydcMy0rcKAG5MTydfdT5zZQ@mail.gmail.com
Whole thread Raw
In response to Re: [pgAdmin4][Patch]: Run MakeFile to generate builds in production mode  (Dave Page <dpage@pgadmin.org>)
Responses Re: [pgAdmin4][Patch]: Run MakeFile to generate builds in production mode
List pgadmin-hackers
On Wed, Sep 13, 2017 at 2:52 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Sep 13, 2017 at 10:06 AM, Surinder Kumar <surinder.kumar@enterprisedb.com> wrote:

On Wed, Sep 13, 2017 at 2:18 PM, Dave Page <dpage@pgadmin.org> wrote:
Thanks, committed - though I did add a bundle-dev target to the Makefile for convenience.

I'm a little concerned though that the query history still doesn't work in dev mode. Is that fixable?
​That's an issue with ReactJS and I had logged a ticket​
 
​on their repository but didn't get any response from them.
The solution is pass `production` flag in place of `envType` in `dev` mode too.

const definePlugin = new webpack.DefinePlugin({
​  ​
'process.env': {
​     ​
'NODE_ENV': JSON.stringify(envType),
​ // In dev mode envType is `development`​

​  ​
},
});

What would be the effect of that? No source map being generated for React?
​Only React is using definePlugin's '
NODE_ENV'
​ variable​
for writing debug-only statements in React code.
​ It will have no effect on functionality. It is used to reduce React bundle size by eliminating debug-only code when run in `production` mode.

React code is like:

function implementSomeReactBehavior() {     // do actual work part 1      if(process.env.NODE_ENV !== "production") {         // do debug-only work, like recording perf stats     }      // do actual work part 2  
}

We are not generating source maps for vendor libraries including React. We generate source maps for pgAdmin4 modules only for debugging purpose.

 



On Tue, Sep 12, 2017 at 11:57 AM, Surinder Kumar <surinder.kumar@enterprisedb.com> wrote:
Hi Dave,

I had discussed this with Ashesh and according to him, running `yarn run bundle` should always build prod bundle. So, I made changes accordingly.

Following changes are made in package.json script commands:

1) Running `yarn run bundle` will build prod bundle.

2) Running `yarn run bundle:dev` will build dev bundle.

No changes are required in Makefile and build.sh files.

Please review and let me know for changes.

Thanks,
Surinder

On Tue, Sep 12, 2017 at 2:39 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

This is over-complicating things. As I said, we never build packages in dev mode - there's just no need.

All we need is easy-to-use Makefile targets that allow us to do either a prod or a dev bundle. The packages should always use prod bundles.

I assume with the (to-be-updated) version of this patch, webpack_config_changes.patch is still required?

On Tue, Sep 12, 2017 at 6:28 AM, Surinder Kumar <surinder.kumar@enterprisedb.com> wrote:
Hi

As per the review comment given by Ashesh:

1) The test cases with target `check:` must run in `prod` mode by default.

2) Add another target `check-dev:` to run test cases in `dev` mode.

So, the patch is updated with fixed review comments.

Please find an updated patch and review.

Thanks,
Surinder


On Tue, Sep 12, 2017 at 10:28 AM, Surinder Kumar <surinder.kumar@enterprisedb.com> wrote:
Hi Dave,

I have added a new target to run build in 'dev' mode. By default, it will run in production mode.

By dev mode it means, run `yarn run bundle:dev` and by prod mode, run `yarn run bundle:prod`.

In Mac bundle, added new target `make appbundle-dev` to generate build with command `yarn run bundle:dev`. The default target `make appbundle` will run `yarn run bundle:prod`.

In Windows bundle, If Make.bat file is executed with an argument `dev` like `./Make.bat x86 dev`, it will call `yarn run bundle:dev` otherwise it will run `yarn run bundle:prod`

Please find attached patch and review.

Thanks,
Surinder

On Fri, Sep 8, 2017 at 12:28 PM, Dave Page <dpage@pgadmin.org> wrote:
No, let's just have two targets in one makefile. It's only for our convenience.

-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK:http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 8 Sep 2017, at 05:34, Surinder Kumar <surinder.kumar@enterprisedb.com> wrote:

Hi

On Thu, Sep 7, 2017 at 8:48 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Thu, Sep 7, 2017 at 7:28 AM, Surinder Kumar <surinder.kumar@enterprisedb.com> wrote:
Hi,

When we run Webpack in production mode, it performs optimization on code while in development we don't optimize generated JS and CSS bundles as dev mode is for developer use.

So we should run Webpack bundle in production mode when we are generating bundles for release mode.

Don't we also need a bundle target for dev mode? The patch changes "make bundle" to run "yarn run bundle:prod"
​yes I think so.
We can have two Makefiles - one for dev mode and other for production mode.
 

In the second patch:

1) Enabled "sourced maps" in production mode as well which will help in debugging issues.

2) Removed "yarn run linter" script when Webpack runs in production mode because it is for developer only to check if there are any syntax errors in JS modules.

3) Removed redundant script command "yarn run bundle" as "yarn run bundle:dev" does the same thing.

Please find an attached patch.

Thanks,
Surinder




--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company






--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company




--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company




--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

pgadmin-hackers by date:

Previous
From: Dave Page
Date:
Subject: Re: [pgAdmin4][Patch]: Run MakeFile to generate builds in production mode
Next
From: Dave Page
Date:
Subject: pgAdmin 4 commit: Update cast icons with improved design.