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

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