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

7.3.0 breaking change? #185

Closed
veeramarni opened this issue Mar 3, 2022 · 11 comments
Closed

7.3.0 breaking change? #185

veeramarni opened this issue Mar 3, 2022 · 11 comments

Comments

@veeramarni
Copy link

veeramarni commented Mar 3, 2022

We noticed errors related JSON type after our build takes 7.3.0 version

import { cleanEnv, json, str } from 'envalid';
import { ITierEnum } from './policies';

export const config = cleanEnv(process.env, {
    POLICIES: json<IPolicies>({
        default: JSON.stringify({
            businessPolicies: {
                [ITierEnum.free]: {
                    users: 1,
                    monthly: 0,
                    annually: 0,
                },
                [ITierEnum.basic]: {
                    users: 10,
                    monthly: 5,
                    annually: 24,
                },

}

./policies

/** The tier of the Organization */
export declare const enum ITierEnum {
    free = "free",
    basic = "basic",
    premium = "premium",
    enterprise = "enterprise"
}

Error

TypeError: Cannot destructure property 'basic' of 'config_1.config.POLICIES.businessPolicies' as it is undefined.
@af
Copy link
Owner

af commented Mar 4, 2022

I can't seem to reproduce this with my minimal test case (below). What is the value of ITierEnum in your case? I'm guessing it's a string enum?

const env = envalid.cleanEnv(process.env, {
  POLICIES: envalid.json({
    default: JSON.stringify({
      bP: {
        free: { users: 1, monthly: 0, annually: 0 },
        basic: { users: 1, monthly: 0, annually: 0 },
      }
    })
  }),
})

@veeramarni
Copy link
Author

My configuration works in 7.2 but not anymore in 7.3.

/** The tier of the Organization */
export declare const enum ITierEnum {
    free = "free",
    basic = "basic",
    premium = "premium",
    enterprise = "enterprise"
}

@af
Copy link
Owner

af commented Mar 12, 2022

Hmm, the following test case still works for me using the enums:

const enum ITierEnum {
    free = "free",
    basic = "basic",
    premium = "premium",
    enterprise = "enterprise"
}

const env = envalid.cleanEnv(process.env, {
  POLICIES: envalid.json({
    default: JSON.stringify({
      bP: {
        [ITierEnum.free]: { users: 1, monthly: 0, annually: 0 },
        [ITierEnum.basic]: { users: 1, monthly: 0, annually: 0 },
      }
    })
  }),

Could you share some more of the traceback of that error? Are you using any custom middlewares?

@Pinqvin
Copy link

Pinqvin commented Mar 15, 2022

I just updated envalid to version 7.3.0 and noticed some environment variables relying on default values breaking with the update. I think it's related to this change: ef5a895

Previously, a default value for a custom validator doing eg. JSON parsing would be ran through that validator, causing issues if you tried to define the default value as an object (but satisfying TypeScript typings for the validator). Instead, we had to define the default values as strings, so that they could be properly parsed by the validator.

@af
Copy link
Owner

af commented Mar 19, 2022

@Pinqvin could you post a minimal repro case here?

@Pinqvin
Copy link

Pinqvin commented Mar 21, 2022

https://github.com/Pinqvin/envalid-repro With [email protected] the values you get for TEST and TEST_CUSTOM are objects, but when you upgrade to 7.3.0, you get string values out. This is what lead to unexpected runtime behaviour, when I upgraded to 7.3.0

@af
Copy link
Owner

af commented Mar 23, 2022

Thank you @Pinqvin for that, I think you are correct and this is unfortunately a breaking change that I should have highlighted:

default values are not passed through validation logic, they are default *output* values.

So if you have default values for json variables, the default value should be want you want to appear in the output, not an unparsed string value. There is more context on #176. Sorry for the inconvenience here, hope you're able to migrate your default values over without issues

@af af closed this as completed Mar 23, 2022
@veeramarni
Copy link
Author

veeramarni commented Mar 23, 2022

@af Is that an intended change? When we pass a JSON stringify to env, we expect it to be an Object value instead of a string. Is that an intended change in v7.3? I think this is a bug to be addressed. This has been a big setback for using JSON type.

@af
Copy link
Owner

af commented Mar 25, 2022

The change should only apply to provided default values, the actual strings passed in through the env vars should still be converted from string -> object via JSON.parse as before. If that is not happening for you somehow please let me know

@veeramarni
Copy link
Author

@af Is not the output of the config is string not object? It doesn't relate to the input default value or not.

@af
Copy link
Owner

af commented Mar 28, 2022

@veeramarni the output of the config should be the same as before, if the value is being provided from an actual env var. For example:

given this foo.ts:

const env = envalid.cleanEnv(process.env, {
  K: envalid.json(),
})
console.log(typeof env.K, env.K)

Running K='{"foo": 234}' ts-node foo.ts prints object { foo: 234 } as expected for me. If you have a repro case where it doesn't work like that please share it.

Regarding the initial issue you posted here, if you remove the JSON.stringify() in your default value I would think your example should work as expected on 7.3.0?

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

No branches or pull requests

3 participants