Skip to content

Conversation

@ajgagnon
Copy link

This lets the user pass custom ENV variables to webpack's Define plugin. The variable must start with WPACKIO_ to be included to prevent leaking sensitive env variables client-side. 😁

Copy link
Owner

@swashata swashata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kindly make these changes. I think, in the client side code, we should use something like console.log(process.env.WPACKIO_ENV). That's how CRA does it.

this.customEnv = {};
Object.keys(process?.env || {}).forEach(key => {
if (key.startsWith('WPACKIO_')) {
this.customEnv[key] = JSON.stringify(process?.env?.[key]);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct me if I am wrong, but shouldn't this be

this.customEnv[`process.env.${key}`] = JSON.stringify(process?.env?.[key]);


// check in class
// @ts-ignore (since this is private). Check it's set.
expect(JSON.parse(cwc.customEnv.WPACKIO_ENV)).toBe('test');
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once we change the key of customEnv, we need to update here too

expect(JSON.parse(cwc.customEnv['process.env.WPACKIO_ENV'])).toBe('test');

@ajgagnon
Copy link
Author

ajgagnon commented Dec 9, 2020

Sorry for the delay on this. I ended up breaking my wrist and getting surgery, so typing is a pain. Hopefully I should be able to contribute in a few weks.

@swashata
Copy link
Owner

No worries at all. Take rest and get well soon 🙂

swashata added a commit that referenced this pull request Apr 26, 2021
@swashata swashata mentioned this pull request Apr 26, 2021
7 tasks
swashata added a commit that referenced this pull request Apr 26, 2021
@swashata
Copy link
Owner

This isn't needed anymore, since v6 has the functionality.

@swashata swashata closed this Apr 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants