b3ross / dotenvi

A simple library for generating dotenv files
MIT License
17 stars 7 forks source link

`optional` functionality broken #43

Closed goldcaddy77 closed 4 years ago

goldcaddy77 commented 4 years ago

Version:

development:
  OPTIONAL_VARIABLE: ## Optional variable syntax.  Undefined variables will otherwise cause failures
    value: ${env:SOME_POSSIBLY_UNDEFINED_VARIABLE}
    optional: true

Running the following command: yarn dotenvi -s development yields the following error:

Environment variable SOME_POSSIBLY_UNDEFINED_VARIABLE is undefined
Could not write .env file: Error: Resolver env didn't return any value
    at Rewriter.<anonymous> (/Users/dancaddigan/Code/goldcaddy77/warthog-starter/node_modules/dotenvi/dist/rewriter.js:48:35)
    at Generator.next (<anonymous>)
    at fulfilled (/Users/dancaddigan/Code/goldcaddy77/warthog-starter/node_modules/dotenvi/dist/rewriter.js:4:58)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
b3ross commented 4 years ago

@goldcaddy77, how’s it going! Super busy at the moment so I probably won’t have a chance to look into this anytime soon.

goldcaddy77 commented 4 years ago

Hey, long time! Any chance you could take a look and point me in the right direction? We're currently pinning at an old version because we use the optional feature.

b3ross commented 4 years ago

Bah, it looks like I introduced a bug with my recursive support: https://github.com/b3ross/dotenvi/blob/master/src/rewriter.ts#L36

https://github.com/b3ross/dotenvi/commit/bfccb6aa1b4d042411cad6133f8ee717a0085eb1#diff-720b6b2262c522ebea28a93c74808fbd

Now I rely on the value existing when returned from a resolver.

One simple fix I think would be to pass in to rewriteValue whether the variable is optional (should be available in the InputDocument), and if so, don't throw an exception.

We should also add a test case for handling optionals here: https://github.com/b3ross/dotenvi/blob/bfccb6aa1b4d042411cad6133f8ee717a0085eb1/src/rewriter.test.js#L22

b3ross commented 4 years ago

Let me know if that doesn't makes sense ☝️

goldcaddy77 commented 4 years ago

Ahhh, gotcha - just saw #50 - thanks for the quick turnaround!