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

Add type declarations #267

Merged
merged 44 commits into from
Aug 12, 2023

Conversation

fabioh8010
Copy link
Contributor

@fabioh8010 fabioh8010 commented Jun 29, 2023

Details

This PR creates type declarations for Onyx functions and withOnyx HOC.

You can refer to this POC PR or checkout the POC branch to see more examples and test the typings.

Related Issues

#260

How it works

This library will use module augmentation in order to provide types from the consumer (the project who will use Onyx) to Onyx own functions.

To augment and add type-safety to Onyx functions and HOC, the developer needs to provides three types to Onyx: keys, collectionKeys, and values.

  • keys: An union type of all Onyx keys (excluding collection keys) used in the project.
  • collectionKeys: An union type of all Onyx collection keys (excluding normal keys) used in the project. This distinction is necessary in order to provide better type-safety to Onyx.
  • values: An object type which each key is a Onyx key, and each value is the data type that is going to be stored in that key. Both normal keys and collection keys has to be defined here.

The developer will need to augment Onyx library by overriding those types inside CustomTypeOptions interface.

Example

////// ONYXKEYS.ts
const ONYXKEYS = {
    ACCOUNT: 'account',
    COLLECTION: {
        DOWNLOAD: 'download_',
        REPORT: 'report_',
    },
    IS_LOADING_PAYMENT_METHODS: 'isLoadingPaymentMethods',
} as const;

type DeepValueOf<T> = T extends object ? DeepValueOf<T[keyof T]> : T;

type OnyxKeysMap = typeof ONYXKEYS;

// "download_" | "report_"
type OnyxCollectionKey = ValueOf<OnyxKeysMap['COLLECTION']>;

// "account" | "isLoadingPaymentMethods"
type OnyxKey = DeepValueOf<Omit<OnyxKeysMap, 'COLLECTION'>>;

type Account = {
    id: string;
    name?: string;
    items?: Array<{name: string}>;
};

type Download = {
    url: string;
};

type Report = {
    id: string;
    isArchived: boolean;
    data: {
        message: string;
        isRead?: boolean;
    };
};

type OnyxValues = {
    [ONYXKEYS.ACCOUNT]: Account;

    // Collection keys
    [ONYXKEYS.COLLECTION.DOWNLOAD]: Download;
    [ONYXKEYS.COLLECTION.REPORT]: Report;

    [ONYXKEYS.IS_LOADING_PAYMENT_METHODS]: boolean;
};

export default ONYXKEYS;
export type {OnyxKey, OnyxCollectionKey, OnyxValues};
////// global.d.ts
import {OnyxKey, OnyxCollectionKey, OnyxValues} from './ONYXKEYS';

// Here I'm augmenting react-native-onyx library.
declare module 'react-native-onyx' {
    interface CustomTypeOptions {
        keys: OnyxKey;
        collectionKeys: OnyxCollectionKey;
        values: OnyxValues;
    }
}

This process is optional and if the developer choose to NOT augment it, the library will then work with minimal type-safety and support.

When augmenting the library, all our type declarations will use the types provided by the developer to add type-safety and support to all Onyx functions and HOC.

Type declarations

Onyx.getAllKeys()

// "account" | "isLoadingPaymentMethods" | `download_${string}` | `report_${string}`
const keys = Onyx.getAllKeys();

Onyx.isSafeEvictionKey()

const isSafeEvictionKey = Onyx.isSafeEvictionKey(ONYXKEYS.ACCOUNT);
// const isSafeEvictionWrongKey = Onyx.isSafeEvictionKey('wrong key'); // raises an error, wrong key - correct

Onyx.removeFromEvictionBlockList()

Onyx.removeFromEvictionBlockList(ONYXKEYS.ACCOUNT, 1);
// Onyx.removeFromEvictionBlockList('wrong key', 1); // raises an error, wrong key - correct

Onyx.addToEvictionBlockList()

Onyx.addToEvictionBlockList(ONYXKEYS.ACCOUNT, 2);
// Onyx.addToEvictionBlockList('wrong key', 2); // raises an error, wrong key - correct

Onyx.connect()

Onyx.connect({
    key: `${ONYXKEYS.COLLECTION.DOWNLOAD}${'attachment1'}`,
    callback: (value) => { // value is Download | null
        if (!value) {
            return;
        }

        console.log(value.id);
    },
});

// Because "waitForCollectionCallback" is true, we will return
// a dictionary of Reports in callback.
Onyx.connect({
    key: ONYXKEYS.COLLECTION.REPORT,
    callback: (value) => { // "value" is Record<string, Report> | null
        if (!value) {
            return;
        }

        console.log(value.report1.id);
        console.log(value.report2.id);
    },
    waitForCollectionCallback: true,
});

Onyx.disconnect()

Onyx.disconnect(1000);
Onyx.disconnect(1000, ONYXKEYS.IS_LOADING_PAYMENT_METHODS);
// Onyx.disconnect(1000, 'wrong key'); // raises an error, wrong key - correct

Onyx.set()

Onyx.set(ONYXKEYS.ACCOUNT, {id: 'account1'});
Onyx.set(ONYXKEYS.IS_LOADING_PAYMENT_METHODS, true);
Onyx.set(`${ONYXKEYS.COLLECTION.DOWNLOAD}${'attachment1'}`, {url: 'download_url'});
// Onyx.set(ONYXKEYS.ACCOUNT, 'wrong value'); // raises an error, wrong value - correct

Onyx.multiSet()

Limitations

  • We have to add as const to the collection keys due to limitations in TS typing.
Onyx.multiSet({
    [ONYXKEYS.ACCOUNT]: {id: 'id2'},
    [`${ONYXKEYS.COLLECTION.DOWNLOAD}${'attachment1'}` as const]: {url: 'download_url'},
    // [ONYXKEYS.IS_LOADING_PAYMENT_METHODS]: 'wrong value', // raises an error, wrong value - correct
});

Onyx.merge()

Onyx.merge(ONYXKEYS.ACCOUNT, {name: 'user name'});
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${'report'}`, {data: {isRead: true}});
// Onyx.merge(ONYXKEYS.ACCOUNT, 'something'); // raises an error, wrong value - correct

Onyx.clear()

Onyx.clear();
Onyx.clear([ONYXKEYS.ACCOUNT, ONYXKEYS.ACTIVE_CLIENTS, `${ONYXKEYS.COLLECTION.DOWNLOAD}${'attachment1'}`]);
// Onyx.clear(['wrong key']); // raises an error, wrong key - correct

Onyx.mergeCollection()

Limitations

  • We have to add as const to the keys due to limitations in TS typing.
Onyx.mergeCollection(ONYXKEYS.COLLECTION.DOWNLOAD, {
    [`${ONYXKEYS.COLLECTION.DOWNLOAD}${'attachment1'}` as const]: {url: 'download_url'},
    [`${ONYXKEYS.COLLECTION.DOWNLOAD}${'attachment3'}` as const]: {url: 'download_url3'},
    // [`${ONYXKEYS.COLLECTION.REPORT}${'report1'}` as const]: {id: 'account'}, // raises an error, wrong key - correct
    // [`${ONYXKEYS.COLLECTION.DOWNLOAD}${'attachment2'}` as const]: false, // raises an error, wrong value - correct
});

// Onyx.mergeCollection(ONYXKEYS.ACCOUNT, {
//     [`${ONYXKEYS.ACCOUNT}${'account1'}` as const]: {id: 'account'},
// });  // raises an error, not a collection - correct

Onyx.update()

Limitations

  • We have to add as const to the collection keys in mergeCollection due to limitations in TS typing.
Onyx.update([
    {
        onyxMethod: 'set',
        key: `${ONYXKEYS.COLLECTION.REPORT}${'report1'}`,
        value: {id: 'id1', isArchived: false, data: {message: 'message1'}},
    },
    {
        onyxMethod: 'merge',
        key: ONYXKEYS.ACCOUNT,
        value: {id: 'id1'},
    },
    // {
    //     onyxMethod: 'merge',
    //     key: ONYXKEYS.IS_LOADING_PAYMENT_METHODS,
    //     value: {id: 'id1'}, // raises an error, wrong value - correct
    // },
    // {
    //     onyxMethod: 'mergeCollection',
    //     key: ONYXKEYS.ACCOUNT,
    //     value: {},
    // }, // raises an error, not a collection - correct
    {
        onyxMethod: 'mergeCollection',
        key: ONYXKEYS.COLLECTION.REPORT,
        value: {
            [`${ONYXKEYS.COLLECTION.REPORT}${'report1'}` as const]: {data: {isRead: true}},
            // [`${ONYXKEYS.COLLECTION.DOWNLOAD}${'report2'}` as const]: {data: {isRead: false}}, // raises an error, wrong key - correct
        },
    },
]);

Onyx.init()

Limitations

  • We have to add as const to the collection keys due to limitations in TS typing.
Onyx.init({
    keys: ONYXKEYS,
    initialKeyStates: {
        [ONYXKEYS.ACCOUNT]: {id: 'id1'},
        [`${ONYXKEYS.COLLECTION.DOWNLOAD}${'attachment1'}` as const]: {url: 'download_url'},
        // [ONYXKEYS.IS_LOADING_PAYMENT_METHODS]: 'wrong value', // raises an error, wrong value - correct
    },
    safeEvictionKeys: [ONYXKEYS.ACCOUNT],
});

Onyx.registerLogger()

// "level" is "alert" | "info", "message" is string
Onyx.registerLogger(({level, message}) => {});

withOnyx()

How it works

  • withOnyx HOC will accept two types, Props and OnyxProps.
  • Each prop in OnyxProps need to be supplied to the HOC with its associated mapping object. The mapping object can have this properties:
    • key: Can be string or function. Specifies the Onyx key to supply the Onyx value to the prop. If passing a function, the parameter of it will be the Props of the component.
    • canEvict: Can be boolean or function. Specifies if the Onyx key can be evicted. If passing a function, the parameter of it will be the Props of the component.
    • initWithStoredValues: boolean. If set to false, then no data will be prefilled into the component.
    • selector: function. This will be used to subscribe to a subset of an Onyx key's data. The sourceData and withOnyx state are passed to the selector and should return the simplified data.
  • The component must be typed with the intersection of both Props and OnyxProps because all props specified in OnyxProps will be injected in the component.
  • The result component type will have all its OnyxProps stripped because they were injected by the HOC and we don't want whoever uses it to need to pass those props to it.

Limitations

Given all the possible combinations in the Mapping object of each Onyx prop, we have the following limitations:

  • Onyx prop with string key:
    • onyxProp: {
          key: ONYXKEYS.ACCOUNT,
      },
    • The type of the Onyx value that is associated with key must match with the type of onyxProp prop, otherwise an error will be thrown. ✅
  • Onyx prop with string key and selector:
    • onyxProp: {
          key: ONYXKEYS.ACCOUNT,
          selector: (value: Account | null): string => value?.id ?? '',
      },
    • The function signature and return type of selector must match with the type of onyxProp prop, otherwise an error will be thrown. ✅
  • Onyx prop with function key:
    • onyxProp: {
          key: ({reportId}) => ONYXKEYS.ACCOUNT,
      },
    • The return type of key function must be a valid Onyx key and type of the Onyx value associated with key must match with the type of onyxProp prop, otherwise an error will be thrown. ✅
  • Onyx prop with function key and selector:
    • onyxProp: {
          key: ({reportId}) => ONYXKEYS.ACCOUNT,
          selector: (value: Account | null) => value?.id ?? '',
      },
    • The function signature and return type of selector must match with the type of onyxProp prop, otherwise an error will be thrown. ✅
  • Onyx prop with string collection key:
    • onyxProp: {
          key: ONYXKEYS.COLLECTION.REPORT,
      },
    • The type of the Onyx value that is associated with key must match with the type of onyxProp prop, otherwise an error will be thrown. ✅
    • When specifying a collection key without a suffix the returned data structure from Onyx is a object with all the records of that collection, the key being the record ID and the value being the record itself. Currently the typings are not being able to differentiate collection key without a suffix with a key with suffix, which will result in a mismatch between Onyx value and onyxProp type. ❌ WORKING ON THIS
  • Onyx prop with string collection key and selector:
    • onyxProp: {
          key: `${ONYXKEYS.COLLECTION.REPORT}${`report1`}`,
          selector: (value: Report | null) => value?.isArchived ?? false,
          
          // FIXME: don't raises an error - incorrect
          // selector: (value: Account | null) => false,
      },
    • The return type of selector must match with the type of onyxProp prop, otherwise an error will be thrown. ✅
  • Onyx prop with function collection key:
    • onyxProp: {
          key: ({reportId}) => `${ONYXKEYS.COLLECTION.REPORT}${reportId}`,
      },
    • The type of the Onyx value that is associated with key must match with the type of onyxProp prop, otherwise an error will be thrown. ✅
  • Onyx prop with function collection key and selector:
    • onyxProp: {
          key: ({reportId}) => `${ONYXKEYS.COLLECTION.REPORT}${reportId}`,
          selector: (value: Report | null) => value?.isArchived ?? false,
      
          // FIXME: don't raises an error - incorrect
          // selector: (value: Account | null) => false,
      },
    • The return type of selector must match with the type of onyxProp prop, otherwise an error will be thrown. ✅

Overall, here are the limitations and issues with withOnyx right now:

  • In some cases the selector is losing partially its type-safety, to overcome this we can explicity specify its function signature to ensure type safety.
  • When using a collection key directly e.g. ONYXKEYS.COLLECTION.REPORT, Onyx will return an object with all records e.g. {"report_0": {"id":"0", "value":"report_0"}, "report_1": {"id":"1", "value":"report_1"}}, translating into this TS typing Record<string, Report | null>. At the moment we don't have means to differentiate between ONYXKEYS.COLLECTION.REPORT and ${ONYXKEYS.COLLECTION.REPORT}${reportId} which would only return Report | null, causing mismatches between the Onyx value and the Onyx prop. I'm looking for a fix for that.

Example

////// Component.tsx
type OnyxProps = {
    report: Report | null;
    accountId: string;
    isLoadingPaymentMethods: boolean;
};

type Props = OnyxProps & {
    reportId: string;
};

function Component({ reportId, report, accountId, isLoadingPaymentMethods}: Props) {
    // component's code...
}

export default withOnyx<Props, OnyxProps>({
    report: {
        key: ({reportId}) => `${ONYXKEYS.COLLECTION.REPORT}${reportId}`,
    },
    accountId: {
        key: ONYXKEYS.ACCOUNT,
        selector: (value: Account | null): string => value?.id ?? 'default account id',
    },
    isLoadingPaymentMethods: {
        key: ONYXKEYS.IS_LOADING_PAYMENT_METHODS,
    },
})(Component);
////// Playground.tsx
import Component from './Component';

function Playground() {
    return <Component reportId="report id" />;
}

lib/Onyx.d.ts Show resolved Hide resolved
lib/Onyx.d.ts Outdated Show resolved Hide resolved
lib/Onyx.d.ts Outdated Show resolved Hide resolved
lib/Onyx.d.ts Outdated Show resolved Hide resolved
lib/Onyx.d.ts Outdated Show resolved Hide resolved
lib/Onyx.d.ts Outdated Show resolved Hide resolved
lib/Onyx.d.ts Outdated Show resolved Hide resolved
.eslintignore Outdated Show resolved Hide resolved
lib/Logger.d.ts Outdated Show resolved Hide resolved
lib/Onyx.d.ts Outdated Show resolved Hide resolved
lib/Onyx.d.ts Outdated Show resolved Hide resolved
@fabioh8010
Copy link
Contributor Author

@hayata-suenaga @blazejkustra I fixed some types based on your suggestions and improved other ones like connect, mergeCollection and update to cover some missing cases. I still have one case in update that is not solved yet, but besides that I will start typing withOnyx tomorrow!

lib/Onyx.d.ts Outdated Show resolved Hide resolved
@fabioh8010

This comment was marked as outdated.

lib/withOnyx.d.ts Outdated Show resolved Hide resolved
lib/withOnyx.d.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@neil-marcellini neil-marcellini left a comment

Choose a reason for hiding this comment

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

Looking good

lib/Onyx.d.ts Outdated Show resolved Hide resolved
lib/types.d.ts Outdated Show resolved Hide resolved
lib/types.d.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@neil-marcellini neil-marcellini left a comment

Choose a reason for hiding this comment

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

👍

lib/types.d.ts Show resolved Hide resolved
Copy link
Contributor

@blazejkustra blazejkustra left a comment

Choose a reason for hiding this comment

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

LGTM! 🥇

lib/Onyx.d.ts Outdated Show resolved Hide resolved
lib/Onyx.d.ts Outdated Show resolved Hide resolved
lib/Onyx.d.ts Outdated Show resolved Hide resolved
lib/types.d.ts Outdated Show resolved Hide resolved
neil-marcellini
neil-marcellini previously approved these changes Aug 7, 2023
Copy link
Contributor

@neil-marcellini neil-marcellini left a comment

Choose a reason for hiding this comment

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

I think it's good to go!

@hayata-suenaga
Copy link
Contributor

will review this tonight

Copy link
Contributor

@hayata-suenaga hayata-suenaga left a comment

Choose a reason for hiding this comment

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

🥳 🥳 🥳 🥳 🥳

@hayata-suenaga
Copy link
Contributor

hayata-suenaga commented Aug 9, 2023

Memo

We can merge this PR now.
After this PR is merged, the new version (release) of Onyx library is created and published to npm. So, we have to bump the version of Onyx in App's package.jsonhere so that we can use these new type definition files in App. The version bump should be done in the App setup PR.

There is a big change in Onyx recently, however, and the Onyx version in App is being bumped to reflect that change (this PR bumps the version). Let's wait until that PR is merged to merge the App setup PR that will bump the Onyx version further.

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

Successfully merging this pull request may close these issues.

5 participants