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

Config type improvement #880

Closed
enepeti opened this issue Feb 12, 2021 · 5 comments
Closed

Config type improvement #880

enepeti opened this issue Feb 12, 2021 · 5 comments

Comments

@enepeti
Copy link

enepeti commented Feb 12, 2021

We are upgrading to v2 and checking the new Config system, and found that Config.get only support predefined types, and any. I think the type can be improved, so the return type of the get function can be specified with the following:

type ValueType<T extends ValueStringType, O = any> =
  T extends 'string' ? string :
  T extends 'number' ? number :
  T extends 'boolean' ? boolean :
  T extends 'boolean|string' ? boolean|string :
  T extends 'number|string' ? number|string :
  O;

export class Config {
  static get<T extends ValueStringType, O = any>(key: string, type: T, defaultValue: ValueType<T, O>): ValueType<T, O>;
  static get<T extends ValueStringType, O = any>(key: string, type?: T): ValueType<T, O> | undefined;

  static getOrThrow<T extends ValueStringType, O = any>(key: string, type?: T, msg?: string): ValueType<T, O>;
}

With these changes, the Config API can still be used exactly the same way like now, but when you use the 'any' type, you can specify the return type.

// these still work, and have the same typing as before
const foobar0 = Config.get('settings.foobar', 'boolean|string'); // foobar0 is of type boolean|string|undefined
const foobar1 = Config.get('settings.foobar', 'boolean', false); // foobar1 is of type boolean
const foobar2 = Config.getOrThrow('settings.foobar', 'boolean'); // foobar2 is of type boolean
const foobar3 = Config.getOrThrow('settings.foobar'); // foobar3 is of type any

// but it is possible to specify the return type when using any
interface FooBar {
  prop0?: string;
  prop1?: number;
  prop2?: Record<string, string>;
}
const foobar4 = Config.get<'any', FooBar>('settings.foobar'); // foobar4 is of type FooBar|undefined
const foobar5 = Config.get<'any', FooBar>('settings.foobar', 'any', {}); // foobar5 is of type FooBar
const foobar6 = Config.getOrThrow<'any', FooBar>('settings.foobar'); // foobar6 is of type FooBar

Maybe its also a good idea to change 'any' in ValueStringType, as you can define what you want instead of any. Or just leave it to show, that foal won't give any type conversion or type checking.

@LoicPoullain LoicPoullain added this to Backlog in Issue tracking via automation Feb 15, 2021
@LoicPoullain
Copy link
Member

LoicPoullain commented Feb 15, 2021

Thank you for your proposal. Could one of the following lines work?

const foobar = Config.get('settings.foobar') as FooBar;
// OR
const foobar: FooBar = Config.get('settings.foobar');

I feel like it would be more straightforward than extending and using the generic types.

const foobar = Config.get<'any', FooBar>('settings.foobar');

@enepeti
Copy link
Author

enepeti commented Feb 15, 2021

The lines you sent are working, yes.
I like to avoid casting if I can, but maybe that's just my preference :)
If you think casting, or defining the type is more straightforward then close this PR.

@LoicPoullain
Copy link
Member

Yes, I think it's a matter of taste here. I'd rather keep the casting option as it feels more straightforward to me and it doesn't introduce a "type feature" to maintain.

Another solution you might use also is inheriting the class to define your own method interface. Maybe something like this:

import { Config as FoalConfig } from '@foal/core';

export class Config extends FoalConfig {
  // some stuff here
}

Issue tracking automation moved this from Backlog to Done / Closed This Release Feb 17, 2021
@enepeti
Copy link
Author

enepeti commented Jul 20, 2022

Hi, it's me again sorry for commenting on my old issue, but I have a similar improvement idea :) If you think its worth it, I can create a new issue.

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);
};

@LoicPoullain
Copy link
Member

LoicPoullain commented Aug 17, 2022

@enepeti sorry for the late reply. Could you open a new issue for this so that we can discuss on this subject on a dedicated thread?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Issue tracking
  
Done / Closed This Release
Development

No branches or pull requests

2 participants