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

Typed config.get based on default.json #1136

Open
enepeti opened this issue Aug 19, 2022 · 10 comments
Open

Typed config.get based on default.json #1136

enepeti opened this issue Aug 19, 2022 · 10 comments

Comments

@enepeti
Copy link

enepeti commented Aug 19, 2022

Hi, I'm opening a new issue as you asked it in #880
I took a deep dive into the Typescript type system and was able to come up with a type (lot of ideas came from the type-challanges) for the Config.get function:

  • based on the default.json the key parameter is typed, it only accepts strings that are defined in the config file in the same format as the Config.get method accept keys (e.g.: 'port', 'setting.debug')
  • based on the default.json the return type is automatically typed, you don't have to provide any type information to the function

As you might notice, the solution heavily relies on the default.json, if we want to read a value which is missing from that file, typescript will throw an error. In my opinion this is more of a positive outcome than a negative, as it forces to define a default value for every key that the application will try to read, also making the defaultValue parameter obsolete (defaults should be defined in default.json).
Another limitation is in a json file we can only have string, boolean, number, array and object types, the type the function returns with can only be these. So if we want to have a more restricted type (e.g. Stripe API has an Interval type which is a subset of string ("weekly" | "daily" | "manual" | "monthly") and we want to use the value straight from the config) we either need to cast the type, be able to specify the return type of the function similar to the current solution, or have some utility to rewrite the type of the config (which I included in the solution).
But in my opinion with these limitations we would get a type-safe, easy-to-use Config functionality.
I'm really interested in your opinion (and I don't have a clear idea, how to include this in the framework)

Some screenshots to show how it works:
intellisense can show you all the available keys
intellisense can show you all the available keys

typescript error if key doesn't exists, value automatically typed correctly
typescript error if key doesn't exists, value automatically typed correctly

works with deep keys with dot notation, correctly returns with complex object types
works with deep keys with dot notation, correctly returns with complex object types

I tried to add as many comments as possible, as it is not an easy read :). (it also uses some of the newest features of typescript, I used version 4.7.3)

import { Config } from '@foal/core';
import Stripe from 'stripe';

import config from 'config/default.json';

type BaseConfigType = typeof config;

/**
 * SetType<T, K, V>: object
 *   T: object type to modify
 *   K: string key to modify, can be in dot notation format to modify deep key
 *   V: type that key to have
 *   modifies T, overrides the key that K refers to to have the type V
 *   if K is not in T, then T is returned
 */
// first check if K has a dot in it. if yes H should be the part before the first dot R should be the part after
type SetType<T extends object, K extends string, V> = K extends `${infer H}.${infer R}`
  ? // we create a new object type
    {
      // the keys should be the same as in T, the type should be the same
      // expect for H: if T[H] is an object then recursively call SetType with that object and the rest of K (R)
      [Key in keyof T]: H extends Key ? (T[H] extends object ? SetType<T[H], R, V> : T[H]) : T[Key];
    }
  : // if K doesn't hava a dot in it then we still create a new object type
    {
      // the keys should be the same as in T, the type should be the same, expect for K: it should be V
      [Key in keyof T]: K extends Key ? V : T[Key];
    };

/**
 * SetTypes<T, KV>: object
 *   T: object to modify
 *   KV: array of key-value doubles, keys are strings can be in dot notation format, values can be any type
 *   modifies T, overrides each key in KV to have the corresponding type in KV
 */
// let's check if KV is an empty array. if not H should be the first item in the array and R should be the rest (could be an empty array as well)
type SetTypes<T extends object, KV extends [string, unknown][]> = KV extends [
  infer H extends [string, unknown],
  ...infer R extends [string, unknown][],
]
  ? // for each H item in the array we update T with the help of the SetType type (H[0] is the key, H[1] is the value)
    // then we recursively call SetTypes with the updated object and the rest of the array (R)
    SetTypes<SetType<T, H[0], H[1]>, R>
  : // if KV is empty just return with T, the input object
    T;

// examples of rewriting types in the config
export type ConfigType = SetTypes<
  BaseConfigType,
  [
    ['stripe.interval', Stripe.AccountCreateParams.Settings.Payouts.Schedule.Interval],
    ['stripe.weeklyanchor', Stripe.AccountCreateParams.Settings.Payouts.Schedule.WeeklyAnchor],
  ]
>;

/**
 * WithPrefix<Pre, Key>: string
 *   Pre: string to prefix Key with
 *   Key: string or number
 */
// first lets check if Pre is never (we use never instead of empty string when we don't want to add any prefix)
type WithPrefix<Pre extends string, Key extends string | number> = [Pre] extends [never]
  ? // if there is no prefix just convert Key to a string (needed if it is a number)
    `${Key}`
  : // if Pre isn't empty check if Key is a number
  Key extends number
  ? // if Key is a number, instead of dot notation use square brackets (this will handle arrays)
    `${Pre}[${Key}]`
  : // if Key is a string use dot notation
    `${Pre}.${Key}`;

/**
 * ObjectKeyPaths<T, Pre>: string (union)
 *   T: object to generate key paths from
 *   Pre: string to prefix key paths with, we use never instead of empty string to begin with
 *   generates all the possible keys in T, with dot notation to deep keys, all the keys prefixed with Pre
 */
type ObjectKeyPaths<T extends object, Pre extends string = never> =
  // Pre is always a key in object if used correctly
  // never | A = A that's why we start with never instead of empty string
  // A | A = A so unioning the same keys multiple times won't cause an issue
  | Pre
  // the idea is to create an object where keys are the same as in T, but values(types) are strings: the keys prefixed with Pre
  // when we created the correct object we can create a union of the values with indexing the object with all the possible keys
  // e.g: this works with tuples(arrays) ['a', 'b', 'c'][number] = 'a' | 'b' | 'c'
  | {
      // we use the same keys as in T, except:
      //  if T is an object we only use string and number keys (filtering symbols)
      //  if T is an array we only use number keys (filtering symbols and functions that are on every array like map, forEach etc.)
      //  this can cause that if we create an object based on an array, then we add some non number keys to it those keys wont be in the end result
      //  but for our use case, this can't happen, because we will read types from a json file
      [Key in T extends unknown[] ? keyof T & number : keyof T & (string | number)]: T[Key] extends object
        ? // if the type of the current key is an object, then we should recursively call ObjectKeyPaths with that object
          // adding the current key prefixed with Pre as the new prefix
          ObjectKeyPaths<T[Key], WithPrefix<Pre, Key>>
        : // if the type isn't an object then just prefix key with Pre
          WithPrefix<Pre, Key>;
      // we use the same logic to index the object when we created the keys for the object
    }[T extends unknown[] ? keyof T & number : keyof T & (string | number)];

type ConfigKeys = ObjectKeyPaths<ConfigType>;

/**
 * Get<T, K>: ?
 *   T: object to get the type from
 *   K: key to read, can be in dot notation to read deep key
 *   gets the type of K from T
 */
// first check if K has a dot in it, if yes then H should be the part before the first dot R should be the rest
type Get<T extends object, K extends string> = K extends `${infer H}.${infer R}`
  ? // check if H is a key in T and that key refers to an object
    T[H & keyof T] extends object
    ? // if yes recursively go a level deeper with that object and the rest of K (R)
      Get<T[H & keyof T], R>
    : // else K is not a proper key of T so there is no type to get
      never
  : // if K has no dot in it, then it must be at the root level, we return it with that type
    T[K & keyof T];

export const get = <K extends ConfigKeys>(key: K): Get<ConfigType, K> => {
  return Config.getOrThrow(key);
};
@kingdun3284
Copy link
Member

kingdun3284 commented Aug 19, 2022

Thanks @enepeti !
I have similar problem too with the Config system lacking of type hinting.
My suggested solution will be something similar to vite js.
We can have a file in root directory called foal-env.d.ts which will be consumed by the Config system
and export an interface like:

interface FoalEnv{
    settings:{
       serverPath:string,
       ...
    }
}

and developers can extend their own config interface to this file.

And there are already some type utilities library to support object string path with hinting. E.g. https://millsp.github.io/ts-toolbelt/modules/function_autopath.html
What do you think @LoicPoullain ?

@LoicPoullain
Copy link
Member

Idea2: Wish to support typescript interface for Configfile with hinted path. For example:

Config.get<P extends FoalsDefaultConfigInterface,K extends string=never>(path:AutoPath<P,K>,...others)

Originally posted by @kingdun3284 in #1027 (comment)

@enepeti
Copy link
Author

enepeti commented Aug 29, 2022

@kingdun3284 thank you for your reply! I'm trying to explore the possibilities of typescript, and learn as much as I can, that's why I try to implement stuff myself, not checking for already made solutions :). I'll check the library and I think if it has the same functionality, we should definitely use that instead of a diy solution.

But I'm not sure about having the foal-env.d.ts file as it feels like you have another file that you need to maintain to have correct types in the Config.get function instead of just adding stuff to the Config file. Also you can't have type restriction in a JSON file, so you can define keys in the d.ts file which would not have a value in the config files.

@LoicPoullain
Copy link
Member

It’s true that it is tedious not to have auto-completion for the configuration path. And having type inference could be interested as well. There are some concerns that I have through:

Path guess

based on the default.json the key parameter is typed, it only accepts strings that are defined in the config file in the same format as the Config.get method accept keys (e.g.: 'port', 'setting.debug')

As you might notice, the solution heavily relies on the default.json, if we want to read a value which is missing from that file, typescript will throw an error. In my opinion this is more of a positive outcome than a negative, as it forces to define a default value for every key that the application will try to read, also making the defaultValue parameter obsolete (defaults should be defined in default.json).

Yes, this is a limitation and I’d rather not to have to specify all the config keys in the default.json. One situation where it is particularly handy is with the default framework config values:

  • There are a lot (settings.jwt.cookie.name, etc) and including them all in default.json could be a bit messy.
  • When a new minor version is release with new features, it sometimes includes new config keys.
  • Some configuration keys are exclusive. For example, for the JWT hooks, the configuration keys settings.jwt.privateKey and settings.jwt.secret cannot co-exist.

Based on this, we might want to define some parameters only for production in production.json such a cookie domain.

Maybe the foal-env.d.ts interface could be a solution to this. Developers then would have to also include the framework settings.

Typescript inference

Another limitation is in a json file we can only have string, boolean, number, array and object types, the type the function returns with can only be these. So if we want to have a more restricted type (e.g. Stripe API has an Interval type which is a subset of string ("weekly" | "daily" | "manual" | "monthly") and we want to use the value straight from the config) we either need to cast the type, be able to specify the return type of the function similar to the current solution, or have some utility to rewrite the type of the config (which I included in the solution).

Yes, this would not include types such as ’foo’|’bar’ or false|string[] and will only work for all variables if all configuration is defined in default.json

Type conversion

Another point that I see here is that, by only inferring the TypeScript type, we don’t check nor convert the JS type which is currently done in the framework. For example, if we specify { settings: { debug: env(SETTINGS_DEBUG) } } and use Config.get(‘settings.debug’, ‘boolean’), the value "true" won’t be converted to a boolean and the framework won’t ever check that the configuration value has the correct (JS) type.

@LoicPoullain
Copy link
Member

I'm also a bit worried about the stability of very complex TypeScript types such as AutoPath and we should not have breaking changes introduced between two minors. These types are also difficult to test.

But maybe, we could end up on something more customizable that any one could adjust. For example, based on the example here, we could add more generic types on Config.get:

Config.get<P extends string, V extends ExpectedTypeBasedOnSecondParameter, ...>(key: P, ...)

Config.get<AutoPath<some types>, AutoKey(some types)>()

@LoicPoullain
Copy link
Member

Or maybe something like this:

function getConfig(key: AutoPath<some types>, type, ...) {
  return Config.get(key, type)
}

@kingdun3284
Copy link
Member

kingdun3284 commented Sep 2, 2022

Or maybe something like this:

function getConfig(key: AutoPath<some types>, type, ...) {
  return Config.get(key, type)
}

It seems that AutoPath is a bit buggy in latest typescript version,what if writting a proxy object for that purpose?We can do the type casting ourself too.

let ConfigObj:FoalsEnv//interface from foals-env.d.ts
const handler=(path:string[]=[])=>({
   get: (target, key) => {
                const value=Config.get([...path,key].join("."));
                if(typeof value==="object")
                  return new Proxy({},handler([...path,key]))
               else return value
   },
})
ConfigObj=new Proxy({},handler())  
console.log(ConfigObj.settings.serverPath)

@kingdun3284
Copy link
Member

kingdun3284 commented Sep 8, 2022

Or maybe something like this:

function getConfig(key: AutoPath<some types>, type, ...) {
  return Config.get(key, type)
}

I have thought about another better design using proxy pattern as well.

const TARGET_SYMBOL = Symbol('proxied-target');
function toPath<T extends object>(obj: T): T {
    function createProxy(path: any[] = []) {
        const proxy = new Proxy(path, {
            get: (target, key) => {
                if (key == TARGET_SYMBOL) return target;
                path.push(key);
                return proxy;
            },
        });
        return proxy;
    }
    return createProxy();
}

function typedConfig<T>(path: (obj: FoalsEnv) => T): T {
    return Config.get((path(toPath(obj)) as any)[TARGET_SYMBOL].join("."));
}

to use it, simply

typedConfig((config)=>config.settings.serverPath)

@kingdun3284
Copy link
Member

Also, it would be better to support typescript config file as well.

@LoicPoullain
Copy link
Member

LoicPoullain commented Oct 25, 2023

I may have a way to make it work all together. With this solution the goals are to:

  • simplify the way we access the configuration
  • have full Typescript support (whether it is for the key or for the value)
  • make sure that the JS types match in all cases the TS types (validation)
  • and support enum validation in the future.

Each Foal project would have a new src/config.ts where we would be able to define the schema of the application configuration:

import { Config, IConfigSchema } from '@foal/core';

const configSchema = {
  database: {
    uri: { type: 'string', required: true }
  },
  myOtherCustomConfig: {
    type: 'boolean|string',
  }
} satisfies IConfigSchema;

export const config = Config.load(configSchema);

Then anywhere in the application code, the configuration would be accessible like a regular object:

import { config } from '../relative-path';

// Application configuration
console.log(config.database.uri) // TypeScript type: string
console.log(config.myOtherCustomConfig) // TypeScript type: boolean | string | undefined

// Framework configuration (optional)
console.log(config.settings.social.facebook.clientId) // TypeScript type: string

The properties of the config object would always be defined. When reading a configuration value, Foal would look at the config schema, fetch the configuration normally (config/* files, .env, etc) and validate and convert the value based on the schema. If any, the errors thrown would be the same as those of Config.get.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Backlog
Development

No branches or pull requests

3 participants