Hi Ashesh,
Can you please review this patch please?
Thanks,
Surinder
On Wed, Aug 16, 2017 at 3:43 PM, Surinder Kumar <
surinder.ku...@enterprisedb.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.ku...@enterprisedb.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.va...@enterprisedb.com> wrote:
>>
>>> On Fri, Jul 28, 2017 at 12:42 PM, Surinder Kumar <
>>> surinder.ku...@enterprisedb.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
>>>>
>>>
>>>
>>
>
