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.

Sign up for GitHub

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

Jump to bottom

Add requiredWhen to validator spec #81

Closed
wants to merge 1 commit into from
Closed

Add requiredWhen to validator spec #81

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ Each validation function accepts an (optional) object with the following attribu
* `desc` - A string that describes the env var.
* `example` - An example value for the env var.
* `docs` - A url that leads to more detailed documentation about the env var.
* `requiredWhen` - A function that receives the raw env vars and must return a boolean. If true, this env var will be required. If false, this env var will be optional.


## Custom validators
Expand Down
5 changes: 5 additions & 0 deletions envalid.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ interface Spec<T> {
* A url that leads to more detailed documentation about the env var.
*/
docs?: string
/**
* A function that receives the full raw environment and returns a boolean
* indicating if this field is required or optional.
*/
requiredWhen?: (env: any) => boolean
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Out of curiosity, why any instead of NodeJS.ProcessEnv? Speaking for the whole file here...

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you then pass your own object? I don't use (or know) TS, but at least for tests it's useful to say envalid.cleanEnv({someFake: 'env'}, bla)

Copy link
Contributor Author

@neezer neezer May 8, 2018 •
edited

Choose a reason for hiding this comment

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

Looks like NodeJS.ProcessEnv will accept any indexable object where the key is a string and the value is either a string or undefined.

declare namespace NodeJS {
        // ...
        export interface ProcessEnv {
                [key: string]: string | undefined;
        }
        // ...
}

So your example of { someFake: 'env' } would compile, but

  • { someFake: null }
  • { someFake: 42 }
  • { someFake: false }
  • { someFake: [1, 2, 3] }
  • { someFake: { really: 'fake' } }
  • { someFake: fake => anotherFake }
  • { someFake: Symbol('gimli') }

... would not. This makes sense to me since env vars are read in to process.env as strings.

I think you'd probably get away with just changing envalid.d.ts, if you wanted to.

Should make this change in a different PR if y'all wanted it; just seemed like a good place to ask.

😉

Copy link
Collaborator

Choose a reason for hiding this comment

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

We've explicitly added support for { someFake: false } etc for testing purposes, so they should keep working...

But yeah, feel free to open up a separate issue!

}

interface ValidatorSpec<T> extends Spec<T> {
Expand Down
8 changes: 7 additions & 1 deletion example/env.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,11 @@ module.exports = envalid.cleanEnv(process.env, {

// For this example, the MESSAGE env var will be read from the .env
// file in this directory (so the default value won't be used):
MESSAGE: envalid.str({ default: 'Hello, world' })
MESSAGE: envalid.str({ default: 'Hello, world' }),

USE_TLS: envalid.bool({ defualt: false }),

// If USE_TLS is true, this will throw if no value is provided (required)
// If USE_TLS is false, this will not throw if no value is provided (optional)
TLS_KEY_PATH: envalid.str({ requiredWhen: env => !!env.USE_TLS })
})
6 changes: 6 additions & 0 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,12 @@ function cleanEnv(inputEnv, specs = {}, options = {}) {
throw new EnvMissingError(formatSpecDescription(spec))
}

if (typeof spec.requiredWhen === 'function') {
if (!spec.requiredWhen(env)) {
continue
}
}

if (rawValue === undefined) {
if (!usingFalsyDefault) {
throw new EnvMissingError(formatSpecDescription(spec))
Expand Down
8 changes: 6 additions & 2 deletions tests/envalid.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ const spec = {
json: json({
devDefault: { foo: 'bar' }
}),
url: url(),
url: url({
requiredWhen: () => true
}),
email: email({
example: 'example',
docs: 'http://example.com'
Expand All @@ -62,7 +64,9 @@ const inferredEnv = cleanEnv(
json: json({
devDefault: { foo: 'bar' }
}),
url: url(),
url: url({
requiredWhen: () => true
}),
email: email({
example: 'example',
docs: 'http://example.com'
Expand Down
22 changes: 21 additions & 1 deletion tests/test_basics.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const { createGroup, assert } = require('painless')
const { cleanEnv, EnvError, EnvMissingError, str, num, testOnly } = require('..')
const { cleanEnv, EnvError, EnvMissingError, str, num, bool, testOnly } = require('..')
const { assertPassthrough } = require('./utils')
const test = createGroup()
const makeSilent = { reporter: null }
Expand Down Expand Up @@ -205,3 +205,23 @@ test('testOnly', () => {
EnvMissingError
)
})

test('conditionally required env', () => {
// Throws when the env var isn't in the given choices:
const spec = {
FOO: str({ requiredWhen: rawEnv => !!rawEnv.REQUIRE_FOO }),
REQUIRE_FOO: bool({ default: false })
}

// FOO not required when REQUIRE_FOO is false
assert.deepEqual(cleanEnv({}, spec, makeSilent), { REQUIRE_FOO: false })

// FOO is still read when REQUIRE_FOO is false
assert.deepEqual(cleanEnv({ FOO: 'bar' }, spec, makeSilent), { REQUIRE_FOO: false, FOO: 'bar' })

// FOO is required when REQUIRE_FOO is true and FOO is not provided
assert.throws(() => cleanEnv({ REQUIRE_FOO: true }, spec, makeSilent), EnvMissingError)

// Works fine when REQUIRE_FOO is true and FOO is satisfied
assertPassthrough({ REQUIRE_FOO: true, FOO: 'bar' }, spec)
})