Incorrect types for values using environment variables
DonIsaac opened this issue · 3 comments
Describe the bug:
Types generated for properties using environment variables are incorrect. Environment variable configs should use string | undefined
as that's what @types/node
uses for environment variables. However, during interface generation, each environment variable is either string
or undefined
, depending on whether or not it's defined at the time. When it is, the property will have a type of string
. Otherwise, it will have a type of undefined
.
Suspected Cause
Type generation follows the following process:
- Load configs for the appropriate environment (based on values for relevant environment variables)
- Merge them together
- Resolve values for config properties using environment variables
- Pass the resulting object to
json-to-ts
e.g.
// config/default.json
{
"foo": "bar"
}
// config/env/dev.json
{
"foo": "@@FOO"
}
When process.env.FOO
is undefined, mergeAllConfigs
results in the final object:
{
"foo": undefined
}
json-to-ts
, seeing undefined
, resolves the type for that particular config to undefined
.
Possible Solutions
Solution 1: use null
instead of undefined
(worse solution)
json-to-ts
handles null
values, but not undefined
values, by making their types optional and any
. The below test from their codebase demonstrates this behavior.
it("should work with multiple key words and optional fields", function() {
const json = {
"hello world": null
};
const expected = `
interface RootObject {
'hello world'?: any;
}`;
const actual = JsonToTS(json).pop();
assert.strictEqual(expected.trim(), actual.trim());
});
This is better than undefined
as this typing is correct, but it's not very specific.
Solution 2: use optional property name syntax
json-to-ts
has undocumented behavior for creating optional types based on property names. Its implemented in this function. Properties ending in --?
will have optional types, and the --?
suffix gets removed from the resulting type. e.g.
{
"foo--?": "foo"
}
Turns into
interface RootObject {
foo?: string
}
We could use this to our advantage. During type generation, we could insert dummy values into values that use environment variables and add the --?
suffix to the key. This would result in the correct type. The downside is that this solution relies on undocumented behavior.
Steps To Reproduce:
Create a bare-bones default config
// config/default.json
{
"foo": "@@FOO"
}
Do not set a value for FOO
in your terminal. Then, run the CLI. The following type declaration will be created:
// config/Config.d.ts
/* tslint:disable */
/* eslint-disable */
declare module "node-config-ts" {
interface IConfig {
foo: undefined
}
export const config: Config
export type Config = IConfig
}
Next, set foo and run the CLI again.
export FOO=foo
node ./bin/cli
The following type declaration will be created:
// config/Config.d.ts
/* tslint:disable */
/* eslint-disable */
declare module "node-config-ts" {
interface IConfig {
foo: string
}
export const config: Config
export type Config = IConfig
}
Expected behavior:
The type declaration below should be generated:
// config/Config.d.ts
/* tslint:disable */
/* eslint-disable */
declare module "node-config-ts" {
interface IConfig {
foo: string | undefined
}
export const config: Config
export type Config = IConfig
}
Typescript Version:
Tested on 3.7.5
and 4.9.4
. Should not affect the behavior.
node-config-ts version:
3.1.0
and 3.2.0
As a sidenote: json-to-ts
appears to be abandoned. There hasn't been a code change in over 7 months. See this Snyk Advisor report
I would like to propose something completely different here —
We should create types first and then have readers that read data from various sources (files, env etc.) and merge them together to create a value of the expected type.
It would work something like this
import {config, Source} from 'node-config-ts'
type Color = "RED" | "BLUE" | "GREEN"
type Config = {
port: number,
color: Color
}
// Generates a config with some default values
const auto: Source<Config> = config.automatic.as<Config>()
// Reads from default.json
const default: Source<Config> = config.file("./default.json").as<Config>()
// Reads from production.json
const production: Source<Config> = config.file("./production.json").as<Config>()
// Read from env variables
const env: Source<Config> = config.env.as<Config>()
// Merge all the configurations
const merged: Source<Config> = auto.orElse(default).orElse(production).orElse(env)
console.log(await merged.generate()) // Will print the final config