On Wed, Sep 13, 2017 at 2:52 PM, Dave Page <dp...@pgadmin.org> wrote:
>
>
> On Wed, Sep 13, 2017 at 10:06 AM, Surinder Kumar <
> surinder.ku...@enterprisedb.com> wrote:
>
>>
>>
>> On Wed, Sep 13, 2017 at 2:18 PM, Dave Page <dp...@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.ku...@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 <dp...@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.ku...@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.ku...@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 <dp...@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.ku...@enterprisedb.com> wrote:
>>>>>>>>
>>>>>>>> Hi
>>>>>>>>
>>>>>>>> On Thu, Sep 7, 2017 at 8:48 PM, Dave Page <dp...@pgadmin.org>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> Hi
>>>>>>>>>
>>>>>>>>> On Thu, Sep 7, 2017 at 7:28 AM, Surinder Kumar <
>>>>>>>>> surinder.ku...@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
>
