Add check for env variables presence #45
Conversation
index.js
Outdated
| let loadedConfig = dotenv.parse(fs.readFileSync(options.path)); | ||
| let allowedKeys = options.clientAllowedKeys || []; | ||
| let loadedConfig = dotenv.config({ path: options.path }); | ||
| if (loadedConfig.error && options.failOnMissingKey) { |
There was a problem hiding this comment.
@ondrejsevcik these two errors does not seem to be related - loadedConfig.error would mean that .env file was not found or has syntax error. What did you intend to achieve with this change?
There was a problem hiding this comment.
I was also confused, can you drop a comment in the source if it's meaningful?
index.js
Outdated
| }, {}); | ||
| if (options.failOnMissingKey) { | ||
| allowedKeys.map(key => { | ||
| if (!loadedConfig.parsed[key]) { |
There was a problem hiding this comment.
@ondrejsevcik this can break your build if key value is false but I believe there are use cases when false is a valid key. Should we compare with undefined?
better than nothing at all
|
thanks for the review, hopefully it's much clearer now. And I've also incorporated your suggestions. |
index.js
Outdated
|
|
||
| let allowedKeys = options.clientAllowedKeys || []; | ||
|
|
||
| allowedKeys.map(key => { |
There was a problem hiding this comment.
@ondrejsevcik minor - could you please use forEach here as it seems more appropriate?
|
LGTM! Thank you @ondrejsevcik for working on this! |
|
LGTM, thanks! |
Should solve #44