Re: [pgAdmin4][Patch]: RM_2596 - Query tool not working in Desktop Runtime on Mac OS X

Hi Ashesh,

The issue is still not fixed, the reason is:

In the first patch, I was setting `production` flag in `definePlugin` like:

const definePlugin = new webpack.DefinePlugin({
'process.env': {
'NODE_ENV': JSON.stringify('production'),
},
});

in development mode but we had the discussion that when webpack is built in
dev mode the flag should be 'development' otherwise `production`.

But this issue is fixed only when we are setting `production` in
definePlugin, setting `development` mode doesn't fix this.

I have logged an issue in React
<https://github.com/facebook/react/issues/10582>

Thoughts?

Thanks,
Surinder

On Thu, Aug 31, 2017 at 2:29 PM, Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com
> wrote:

> On Tue, Aug 29, 2017 at 5:55 PM, Surinder Kumar <
> surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
>
>> Hi Ashesh,
>>
>> Can you please review this patch please?
>>
> Thanks - committed!
>
> --
>
> Thanks & Regards,
>
> Ashesh Vashi
> EnterpriseDB INDIA: Enterprise PostgreSQL Company
> <http://www.enterprisedb.com/>
>
>
> *http://www.linkedin.com/in/asheshvashi*
> <http://www.linkedin.com/in/asheshvashi>
>
>>
>> Thanks,
>> Surinder
>>
>> On Wed, Aug 16, 2017 at 3:43 PM, Surinder Kumar <
>> surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
>>
>>> Hi,
>>>
>>> Updated patch contains changes:
>>>
>>> - Enable definePlugin for development environment as well. Just
>>> adding definePlugin in plugins array.
>>> The variable process.env.NODE_ENV is useful to write conditional
>>> code in pgAdmin4 JS modules.
>>>
>>> For example:
>>>
>>> if (process.env.NODE_ENV !== 'production') {
>>> // Write development environment specific code
>>> } else {
>>> // Write production only code.
>>> }
>>>
>>> Please review this patch and let me know for changes.
>>>
>>> Thanks,
>>> Surinder
>>> ​
>>>
>>> On Tue, Aug 1, 2017 at 11:32 AM, Surinder Kumar <
>>> surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hi Ashesh,
>>>>
>>>> 1. Now we are using `envType` variable in definePlugin which sets
>>>> environment variable NODE_ENV globally which is used by React to create
>>>> development or production build.
>>>> where:
>>>> envType - determine build type is either `production` or
>>>> `development`​ depending on the environment set in package.json > scripts.
>>>>
>>>> 2. In `UglifyJSPlugin`, i am setting compress > `warnings to false`,
>>>> because here warning flag is meant to display warnings on terminal while
>>>> creating build in production mode. so it is set to false.
>>>>
>>>> I didn't created an RM for #2 as it is minor change, if needed, i will
>>>> create.
>>>>
>>>> ​Reference to webpack definePlugin:
>>>> https://webpack.js.org/guides/production/#node-environment-variable​
>>>>
>>>> Please find updated patch with fixed review comments and review.
>>>>
>>>> Thanks,
>>>> Surinder
>>>>
>>>> On Mon, Jul 31, 2017 at 3:31 PM, Ashesh Vashi <
>>>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> On Fri, Jul 28, 2017 at 12:42 PM, Surinder Kumar <
>>>>> surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> I inspect the react code and in call stacks, found
>>>>>> `process.env.NODE_ENV` is undefined due to which 'SyntheticEvent.call' is
>>>>>> not callable.
>>>>>>
>>>>>> So, to fix this, i add 'definePlugin' to plugins for `dev`
>>>>>> environment in `webpack.config.js`. Initially it was added only for
>>>>>> `production` environment. but it is needed for both, because React code is
>>>>>> conditional based on environment variables set.
>>>>>>
>>>>>
>>>>>> Please find attached patch and review.
>>>>>>
>>>>> As discussed, you're setting 'production', even in the 'development'
>>>>> mode.
>>>>>
>>>>> Please understand the code, and share the updated patch.
>>>>> Also - share the references next time, so that - committer can
>>>>> understand the reason for these changes.
>>>>>
>>>>> -- Thanks, Ashesh
>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> Surinder
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>