Add support for loading env values #23

nkahlor · 2021-02-26T18:13:25Z

Pre-Requisites

Yes, I updated Authors.md OR this is not my first contribution
Yes, I included and/or modified tests to cover relevent code OR my change is non-technical
Yes, I wrote this code entirely myself OR I properly attributed these changes in Third Party Notices

Description of Changes

Extends the capabilities of dotenv-flow, so the users of this package don't need to install both packages.

simple-env should play nice with .env files out of the box :^)

Related Issues

Closes Add support for loading env values

johnkahn

Could you move the parsing code to it's own file that is imported into index.ts? It's about doubled the length of the file, so I think it'll help readability.

Great work, I think this will definitely help us going forward as it's one less dependency in our projects. In the future, I would love to see us add support for multiple .env files similar to dotenv-flow since some of our projects will still need to use that for multiple environments.

src/index.ts

src/types/Options.ts

package.json

nkahlor

I think this should just about wrap it up for this PR, please let me know if there's anything else I missed and I'll try to get to it quickly!

.vscode/launch.json

src/test/index.test.ts

src/parser.ts

SpencerKaiser

Two big things in addition to a question (and more feedback from @johnkahn):

Make sure to update the README with info about how to use it (make sure to make restrictions clear and add a snippet to show an example of it being used)
Since this is a rather significant feature addition, make sure to bump the minor version in both of the package files

This is going to be an awesome addition!!! Thanks for doing this 😄

src/types/Options.ts

johnkahn

A couple README updates.

Also reading through the example I want to make sure that the implementation here matches what dotenv does so it's an easy swap for people. Looks like we have some extra stuff (like // comments) and we're missing some things. @SpencerKaiser what is your thought on changing envPath to experimentalEnvPath and merging as is? Then changing it to envPath once it matches?

https://github.com/motdotla/dotenv#rules

README.md

Co-authored-by: John Kahn <[email protected]>

nkahlor · 2021-03-18T15:52:07Z

A couple README updates.

Also reading through the example I want to make sure that the implementation here matches what dotenv does so it's an easy swap for people. Looks like we have some extra stuff (like // comments) and we're missing some things. @SpencerKaiser what is your thought on changing envPath to experimentalEnvPath and merging as is? Then changing it to envPath once it matches?

https://github.com/motdotla/dotenv#rules

I don't think adding // comments is a bad thing

But we definitely want our parser to handle the same cases as DotEnv so that it's easy for people to swap to our package painlessly. I'll re-work it a bit to cover all those cases.

SpencerKaiser

Just a few quick things and then I think we can merge!

SpencerKaiser · 2021-03-22T15:02:34Z

README.md

+
+If you set a path to an `.env` file, `simple-env` will parse the file and import the contents into the environment automatically. These will be available in the `process.env` object.
+
+> If you don't specify a path, `simple-env` **will not** automatically load any `.env` files. This is to avoid a breaking change.


Suggested change

> If you don't specify a path, `simple-env` **will not** automatically load any `.env` files. This is to avoid a breaking change.

> :warning: **NOTE**: If you don't specify a path, `simple-env` **will not** automatically load any `.env` files.

SpencerKaiser · 2021-03-22T15:03:14Z

README.md

+    someRequiredSecret: 'SOME_REQUIRED_SECRET',
+  },
+  options: {
+    envPath: "./.env"


This should work as well. Do you think the original is better?

Suggested change

envPath: "./.env"

envPath: ".env"

SpencerKaiser · 2021-03-22T15:05:25Z

README.md

+  }
+});
+```
+Each variable needs to be declared on a separate line.


Let's add all of this to a new sub-section called Correct .env Syntax (or something like it). To keep the README simple, can we add all of that to a <details> tag?

SpencerKaiser · 2021-03-22T15:05:56Z

.gitignore

@@ -1,3 +1,5 @@
+.vscode


Including that file was intentional; revert this

SpencerKaiser · 2021-03-22T15:27:29Z

@johnkahn I'm ok with the rules as of now; while I don't want to encourage // as a comment within .env, we can't use that line as a variable anyways, so we should still ignore it. As long as we're inclusive of the use-cases dotenv covers, I think we'll be good to go for people swapping.

And re: experimentalEnvPath, keep in mind that bumping that to envPath would be a breaking change, so we'd need to bump the minor version again.

johnkahn · 2021-03-23T19:57:11Z

@SpencerKaiser I was wondering if having //SOMETHING="something" would create process.env['//SOMETHING'] though 🤔 That was my main thing is that if that works in dotenv we may want to keep it that way for now. Plus editors don't highlight // as a comment like # does

And for experimentalEnvPath we would put in the description and the README that it can change or be removed at any time, so it's more of a use at your own risk. It's how I've seen React handles experimental features, but I'm cool with not doing it.

Nicholas Kahlor added 5 commits February 26, 2021 09:20

use dotenv flow to read in environment

749e3eb

clean up code

f534154

include a unit test

20dcc2f

full coverage

f4d78df

fix test name

39e4ad4

nkahlor marked this pull request as ready for review February 26, 2021 18:18

reinvent the wheel a bit, just for kicks

a07693d

johnkahn suggested changes Mar 1, 2021

View reviewed changes

src/index.ts Outdated Show resolved Hide resolved

src/index.ts Outdated Show resolved Hide resolved

src/index.ts Outdated Show resolved Hide resolved

src/types/Options.ts Outdated Show resolved Hide resolved

src/types/Options.ts Outdated Show resolved Hide resolved

johnkahn suggested changes Mar 5, 2021

View reviewed changes

package.json Outdated Show resolved Hide resolved

nkahlor added 6 commits March 8, 2021 18:32

Merge branch 'main' into main

4feb523

remove dependency

046ccb2

merge filename options

dc3747b

unit tests, yay

d663bb4

change options name to be more general

71b28be

fix formatting

1d49d3d

nkahlor commented Mar 10, 2021

View reviewed changes

.vscode/launch.json Outdated Show resolved Hide resolved

nkahlor requested a review from johnkahn March 10, 2021 23:46

nkahlor commented Mar 10, 2021

View reviewed changes

src/test/index.test.ts Outdated Show resolved Hide resolved

johnkahn reviewed Mar 17, 2021

View reviewed changes

src/parser.ts Show resolved Hide resolved

SpencerKaiser suggested changes Mar 17, 2021

View reviewed changes

src/types/Options.ts Outdated Show resolved Hide resolved

nkahlor added 5 commits March 17, 2021 19:44

cleaning up

a1910ea

remove .vscode

aabef0b

Update index.test.ts

f10df70

DotEnv parsing instructions

7d7566e

Merge branch 'main' of https://github.com/nkahlor/simple-env into main

e8711ca

nkahlor requested review from johnkahn and SpencerKaiser March 18, 2021 03:25

bump minor version

b1058ce

johnkahn suggested changes Mar 18, 2021

View reviewed changes

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

Update README.md

cf06e65

Co-authored-by: John Kahn <[email protected]>

Update README.md

e2d6dd2

Co-authored-by: John Kahn <[email protected]>

SpencerKaiser suggested changes Mar 22, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for loading env values #23

Add support for loading env values #23

nkahlor commented Feb 26, 2021

johnkahn left a comment •

edited

Loading

nkahlor left a comment

SpencerKaiser left a comment

johnkahn left a comment

nkahlor commented Mar 18, 2021

SpencerKaiser left a comment

SpencerKaiser Mar 22, 2021

SpencerKaiser Mar 22, 2021

SpencerKaiser Mar 22, 2021

SpencerKaiser Mar 22, 2021

SpencerKaiser commented Mar 22, 2021

johnkahn commented Mar 23, 2021 •

edited

Loading


		If you set a path to an `.env` file, `simple-env` will parse the file and import the contents into the environment automatically. These will be available in the `process.env` object.

		> If you don't specify a path, `simple-env` will not automatically load any `.env` files. This is to avoid a breaking change.

	> If you don't specify a path, `simple-env` will not automatically load any `.env` files. This is to avoid a breaking change.
	> :warning: NOTE: If you don't specify a path, `simple-env` will not automatically load any `.env` files.

Add support for loading env values #23

Are you sure you want to change the base?

Add support for loading env values #23

Conversation

nkahlor commented Feb 26, 2021

Pre-Requisites

Description of Changes

Related Issues

johnkahn left a comment • edited Loading

Choose a reason for hiding this comment

nkahlor left a comment

Choose a reason for hiding this comment

SpencerKaiser left a comment

Choose a reason for hiding this comment

johnkahn left a comment

Choose a reason for hiding this comment

nkahlor commented Mar 18, 2021

SpencerKaiser left a comment

Choose a reason for hiding this comment

SpencerKaiser Mar 22, 2021

Choose a reason for hiding this comment

SpencerKaiser Mar 22, 2021

Choose a reason for hiding this comment

SpencerKaiser Mar 22, 2021

Choose a reason for hiding this comment

SpencerKaiser Mar 22, 2021

Choose a reason for hiding this comment

SpencerKaiser commented Mar 22, 2021

johnkahn commented Mar 23, 2021 • edited Loading

johnkahn left a comment •

edited

Loading

johnkahn commented Mar 23, 2021 •

edited

Loading