Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add context, add read-only process env option #72

Closed

Conversation

WoodyWoodsta
Copy link

This PR addresses #55, as well as provides a route to fixing vitejs/vite#6626

Updated Options

  1. ignoreProcessEnv: added read-only option, which will read from process.env but not write to it.

New Options

  1. context: optionally pass your own environment context to be used if ignoreProcessEnv: true.

@benmccann
Copy link

+1 for the idea of being able to read, but not write to process.env. I'd love to hear your thoughts on what the ideal API is when you get a chance to review this @motdotla


switch (config.ignoreProcessEnv) {
case 'read-only':
environment = rfdc()(process.env)

Choose a reason for hiding this comment

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

Could add a TODO to drop this library and use structuredClone when only Node 18 is supported

Choose a reason for hiding this comment

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

Actually, process.env won't be a nested object will it? So this could just use Object.assign rather than an external Library?

Copy link
Author

@WoodyWoodsta WoodyWoodsta Jul 17, 2022

Choose a reason for hiding this comment

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

Wasn't aware of structuredClone until now - whether we need a TODO for this or not is up to @motdotla.

As for the depth of process.env, theoretically it will be flat, but technically one could mutate it beforehand to be more than one level deep, and it's probably a point for tricky bugs if this library wasn't defensive against that.

Copy link
Author

@WoodyWoodsta WoodyWoodsta Jul 17, 2022

Choose a reason for hiding this comment

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

I stand corrected! You're absolutely right about not needing the deep clone.

@benmccann
Copy link

@WoodyWoodsta is context needed to fix the Vite issue? If so, why? I'm curious your usecase for it

@WoodyWoodsta
Copy link
Author

WoodyWoodsta commented Jul 17, 2022

@benmccann context was added to address one of the class of issues in #55. It made sense to group the changes into this PR to produce pathways as below:

  • ignoreProcessEnv: true: process.env -> expand -> process.env, parsed
  • ignoreProcessEnv: 'read-only': process.env -> expand -> parsed
  • ignoreProcessEnv: false: context -> expand -> parsed
  • ignoreProcessEnv: false: nothing -> expand -> parsed

@motdotla
Copy link
Owner

motdotla commented Feb 11, 2024

we removed ignoreProcessEnv and instead you can now bring your own processEnv. this gives you full flexibility. combine with dotenv.processEnv to never write or read from process.env.

HELLO=World
const dotenv = require('dotenv')
const dotenvExpand = require('dotenv-expand')

const myEnv = {}
dotenvExpand.expand({ processEnv: myEnv, parsed: dotenv.config({ processEnv: {} }).parsed })

console.log(process.env.HELLO) // undefined
console.log(myEnv.HELLO) // World

source: https://github.com/motdotla/dotenv-expand?tab=readme-ov-file#processenv

@motdotla motdotla closed this Feb 11, 2024
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.

3 participants