From 737b7c53eed739fbd30ee3deb2807e16cf2e7e51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Thu, 25 Apr 2024 11:56:49 +0100 Subject: [PATCH 01/14] Migrate file to TS --- lib/{withOnyx.js => withOnyx.tsx} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename lib/{withOnyx.js => withOnyx.tsx} (100%) diff --git a/lib/withOnyx.js b/lib/withOnyx.tsx similarity index 100% rename from lib/withOnyx.js rename to lib/withOnyx.tsx From c784b784830dce85eeb079603919962bce7192cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Thu, 25 Apr 2024 19:50:36 +0100 Subject: [PATCH 02/14] Start migration of withOnyx --- lib/utils.ts | 48 ++++++- lib/withOnyx.tsx | 360 ++++++++++++++++++++++++++++++++++------------- 2 files changed, 306 insertions(+), 102 deletions(-) diff --git a/lib/utils.ts b/lib/utils.ts index e29e472fe..a020b4158 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -117,4 +117,50 @@ function formatActionName(method: string, key?: OnyxKey): string { return key ? `${method.toUpperCase()}/${key}` : method.toUpperCase(); } -export default {isEmptyObject, fastMerge, formatActionName, removeNestedNullValues}; +function pick(obj: Record, condition: string | string[] | ((entry: [string, T]) => boolean)): Record { + const result: Record = {}; + + const entries = Object.entries(obj); + for (let i = 0; i < entries.length; i++) { + const [key, value] = entries[i]; + + if (Array.isArray(condition)) { + if (condition.includes(key)) { + result[key] = value; + } + } else if (typeof condition === 'string') { + if (key === condition) { + result[key] = value; + } + } else if (condition(entries[i])) { + result[key] = value; + } + } + + return result; +} + +function omit(obj: Record, condition: string | string[] | ((entry: [string, T]) => boolean)): Record { + const result: Record = {}; + + const entries = Object.entries(obj); + for (let i = 0; i < entries.length; i++) { + const [key, value] = entries[i]; + + if (Array.isArray(condition)) { + if (!condition.includes(key)) { + result[key] = value; + } + } else if (typeof condition === 'string') { + if (key !== condition) { + result[key] = value; + } + } else if (!condition(entries[i])) { + result[key] = value; + } + } + + return result; +} + +export default {isEmptyObject, fastMerge, formatActionName, removeNestedNullValues, pick, omit}; diff --git a/lib/withOnyx.tsx b/lib/withOnyx.tsx index 2ea6dbb50..011898002 100644 --- a/lib/withOnyx.tsx +++ b/lib/withOnyx.tsx @@ -5,11 +5,147 @@ */ import PropTypes from 'prop-types'; import React from 'react'; +import type {IsEqual} from 'type-fest'; import _ from 'underscore'; import Onyx from './Onyx'; +import OnyxUtils from './OnyxUtils'; import * as Str from './Str'; +import type {CollectionKeyBase, ExtractOnyxCollectionValue, KeyValueMapping, OnyxCollection, OnyxEntry, OnyxKey, OnyxValue, Selector, WithOnyxConnectOptions} from './types'; import utils from './utils'; -import OnyxUtils from './OnyxUtils'; + +/** + * Represents the base mapping options between an Onyx key and the component's prop. + */ +type BaseMapping = { + canEvict?: boolean | ((props: Omit) => boolean); + initWithStoredValues?: boolean; + allowStaleData?: boolean; +}; + +type CollectionBaseMapping = { + initialValue?: OnyxCollection; +}; + +type EntryBaseMapping = { + initialValue?: OnyxEntry; +}; + +/** + * Represents the string / function `key` mapping option between an Onyx key and the component's prop. + * + * If `key` is `string`, the type of the Onyx value that is associated with `key` must match with the type of the component's prop, + * otherwise an error will be thrown. + * + * If `key` is `function`, the return type of `key` function must be a valid Onyx key and the type of the Onyx value associated + * with `key` must match with the type of the component's prop, otherwise an error will be thrown. + * + * @example + * ```ts + * // Onyx prop with `string` key + * onyxProp: { + * key: ONYXKEYS.ACCOUNT, + * }, + * + * // Onyx prop with `function` key + * onyxProp: { + * key: ({reportId}) => ONYXKEYS.ACCOUNT, + * }, + * ``` + */ +type BaseMappingKey = IsEqual extends true + ? { + key: TOnyxKey | ((props: Omit & Partial) => TOnyxKey); + } + : never; + +/** + * Represents the string `key` and `selector` mapping options between an Onyx key and the component's prop. + * + * The function signature and return type of `selector` must match with the type of the component's prop, + * otherwise an error will be thrown. + * + * @example + * ```ts + * // Onyx prop with `string` key and selector + * onyxProp: { + * key: ONYXKEYS.ACCOUNT, + * selector: (value: Account | null): string => value?.id ?? '', + * }, + * ``` + */ +// eslint-disable-next-line @typescript-eslint/no-unused-vars +type BaseMappingStringKeyAndSelector = { + key: TOnyxKey; + selector: Selector; +}; + +/** + * Represents the function `key` and `selector` mapping options between an Onyx key and the component's prop. + * + * The function signature and return type of `selector` must match with the type of the component's prop, + * otherwise an error will be thrown. + * + * @example + * ```ts + * // Onyx prop with `function` key and selector + * onyxProp: { + * key: ({reportId}) => ONYXKEYS.ACCOUNT, + * selector: (value: Account | null) => value?.id ?? '', + * }, + * ``` + */ +type BaseMappingFunctionKeyAndSelector = { + key: (props: Omit & Partial) => TOnyxKey; + selector: Selector; +}; + +/** + * Represents the mapping options between an Onyx key and the component's prop with all its possibilities. + */ +type Mapping = BaseMapping & + EntryBaseMapping & + ( + | BaseMappingKey> + | BaseMappingStringKeyAndSelector + | BaseMappingFunctionKeyAndSelector + ); + +/** + * Represents the mapping options between an Onyx collection key without suffix and the component's prop with all its possibilities. + */ +type CollectionMapping = BaseMapping & + CollectionBaseMapping & + ( + | BaseMappingKey> + | BaseMappingStringKeyAndSelector, TOnyxKey> + | BaseMappingFunctionKeyAndSelector, TOnyxKey> + ); + +/** + * Represents an union type of all the possible Onyx key mappings. + * Each `OnyxPropMapping` will be associated with its respective Onyx key, ensuring different type-safety for each object. + */ +type OnyxPropMapping = { + [TOnyxKey in OnyxKey]: Mapping; +}[OnyxKey]; + +/** + * Represents an union type of all the possible Onyx collection keys without suffix mappings. + * Each `OnyxPropCollectionMapping` will be associated with its respective Onyx key, ensuring different type-safety for each object. + */ +type OnyxPropCollectionMapping = { + [TOnyxKey in CollectionKeyBase]: CollectionMapping; +}[CollectionKeyBase]; + +type MapOnyxToState = { + [TOnyxProp in keyof TOnyxProps]: OnyxPropMapping | OnyxPropCollectionMapping; +}; + +type WithOnyxProps = Omit; + +type WithOnyxState = TOnyxProps & { + loading: boolean; +}; // This is a list of keys that can exist on a `mapping`, but are not directly related to loading data from Onyx. When the keys of a mapping are looped over to check // if a key has changed, it's a good idea to skip looking at these properties since they would have unexpected results. @@ -17,36 +153,49 @@ const mappingPropertiesToIgnoreChangesTo = ['initialValue', 'allowStaleData']; /** * Returns the display name of a component - * - * @param {object} component - * @returns {string} */ -function getDisplayName(component) { +function getDisplayName(component: React.ComponentType): string { return component.displayName || component.name || 'Component'; } /** * Removes all the keys from state that are unrelated to the onyx data being mapped to the component. * - * @param {Object} state of the component - * @param {Object} onyxToStateMapping the object holding all of the mapping configuration for the component - * @returns {Object} + * @param state of the component + * @param onyxToStateMapping the object holding all of the mapping configuration for the component */ -const getOnyxDataFromState = (state, onyxToStateMapping) => _.pick(state, _.keys(onyxToStateMapping)); +const getOnyxDataFromState = (state: WithOnyxState, onyxToStateMapping: MapOnyxToState) => + utils.pick(state, Object.keys(onyxToStateMapping)) as WithOnyxState; -export default function (mapOnyxToState, shouldDelayUpdates = false) { +export default function ( + mapOnyxToState: MapOnyxToState, + shouldDelayUpdates = false, +): React.ComponentType> { // A list of keys that must be present in tempState before we can render the WrappedComponent - const requiredKeysForInit = _.chain(mapOnyxToState) - .omit((config) => config.initWithStoredValues === false) + const requiredKeysForInit = utils + .omit(mapOnyxToState, (entry) => entry[1].initWithStoredValues === false) .keys() .value(); - return (WrappedComponent) => { + // const requiredKeysForInit = _.chain(mapOnyxToState) + // .omit((config) => config.initWithStoredValues === false) + // .keys() + // .value(); + + return (WrappedComponent: React.ComponentType): React.ComponentType> => { const displayName = getDisplayName(WrappedComponent); - class withOnyx extends React.Component { - pendingSetStates = []; - constructor(props) { + class withOnyx extends React.Component, WithOnyxState> { + pendingSetStates: Array>> = []; + + shouldDelayUpdates: boolean; + + activeConnectionIDs: Record; + + tempState: WithOnyxState; + + constructor(props: WithOnyxProps) { super(props); + this.shouldDelayUpdates = shouldDelayUpdates; this.setWithOnyxState = this.setWithOnyxState.bind(this); this.flushPendingSetStates = this.flushPendingSetStates.bind(this); @@ -55,35 +204,33 @@ export default function (mapOnyxToState, shouldDelayUpdates = false) { // disconnected. It is a key value store with the format {[mapping.key]: connectionID}. this.activeConnectionIDs = {}; - const cachedState = _.reduce( - mapOnyxToState, - (resultObj, mapping, propertyName) => { - const key = Str.result(mapping.key, props); - let value = OnyxUtils.tryGetCachedValue(key, mapping); - if (!value && mapping.initialValue) { - value = mapping.initialValue; - } + const cachedState = Object.keys(mapOnyxToState).reduce>((resultObj, propName) => { + const mapping = mapOnyxToState[propName as keyof TOnyxProps]; - /** - * If we have a pending merge for a key it could mean that data is being set via Onyx.merge() and someone expects a component to have this data immediately. - * - * @example - * - * Onyx.merge('report_123', value); - * Navigation.navigate(route); // Where "route" expects the "value" to be available immediately once rendered. - * - * In reality, Onyx.merge() will only update the subscriber after all merges have been batched and the previous value is retrieved via a get() (returns a promise). - * So, we won't use the cache optimization here as it will lead us to arbitrarily defer various actions in the application code. - */ - if ((value !== undefined && !OnyxUtils.hasPendingMergeForKey(key)) || mapping.allowStaleData) { - // eslint-disable-next-line no-param-reassign - resultObj[propertyName] = value; - } + const key = Str.result(mapping.key, props) as OnyxKey; + let value = OnyxUtils.tryGetCachedValue(key, mapping as Partial>) as OnyxValue; + if (!value && mapping.initialValue) { + value = mapping.initialValue; + } - return resultObj; - }, - {}, - ); + /** + * If we have a pending merge for a key it could mean that data is being set via Onyx.merge() and someone expects a component to have this data immediately. + * + * @example + * + * Onyx.merge('report_123', value); + * Navigation.navigate(route); // Where "route" expects the "value" to be available immediately once rendered. + * + * In reality, Onyx.merge() will only update the subscriber after all merges have been batched and the previous value is retrieved via a get() (returns a promise). + * So, we won't use the cache optimization here as it will lead us to arbitrarily defer various actions in the application code. + */ + if ((value !== undefined && !OnyxUtils.hasPendingMergeForKey(key)) || mapping.allowStaleData) { + // eslint-disable-next-line no-param-reassign + resultObj[propName as keyof TOnyxProps] = value; + } + + return resultObj; + }, {} as WithOnyxState); // If we have all the data we need, then we can render the component immediately cachedState.loading = _.size(cachedState) < requiredKeysForInit.length; @@ -98,17 +245,21 @@ export default function (mapOnyxToState, shouldDelayUpdates = false) { const onyxDataFromState = getOnyxDataFromState(this.state, mapOnyxToState); // Subscribe each of the state properties to the proper Onyx key - _.each(mapOnyxToState, (mapping, propertyName) => { - if (_.includes(mappingPropertiesToIgnoreChangesTo, propertyName)) { + Object.keys(mapOnyxToState).forEach((propName) => { + const mapping = mapOnyxToState[propName as keyof TOnyxProps]; + + if (_.includes(mappingPropertiesToIgnoreChangesTo, propName)) { return; } - const key = Str.result(mapping.key, {...this.props, ...onyxDataFromState}); - this.connectMappingToOnyx(mapping, propertyName, key); + + const key = Str.result(mapping.key, {...this.props, ...onyxDataFromState}) as OnyxKey; + this.connectMappingToOnyx(mapping, propName as keyof TOnyxProps, key); }); + this.checkEvictableKeys(); } - componentDidUpdate(prevProps, prevState) { + componentDidUpdate(prevProps: WithOnyxProps, prevState: WithOnyxState) { // The whole purpose of this method is to check to see if a key that is subscribed to Onyx has changed, and then Onyx needs to be disconnected from the old // key and connected to the new key. // For example, a key could change if KeyB depends on data loading from Onyx for KeyA. @@ -116,9 +267,11 @@ export default function (mapOnyxToState, shouldDelayUpdates = false) { const onyxDataFromState = getOnyxDataFromState(this.state, mapOnyxToState); const prevOnyxDataFromState = getOnyxDataFromState(prevState, mapOnyxToState); - _.each(mapOnyxToState, (mapping, propName) => { + Object.keys(mapOnyxToState).forEach((propName) => { + const mapping = mapOnyxToState[propName as keyof TOnyxProps]; + // Some properties can be ignored because they aren't related to onyx keys and they will never change - if (_.includes(mappingPropertiesToIgnoreChangesTo, propName)) { + if (mappingPropertiesToIgnoreChangesTo.includes(propName)) { return; } @@ -129,30 +282,33 @@ export default function (mapOnyxToState, shouldDelayUpdates = false) { // (eg. if a user switches chats really quickly). In this case, it's much more stable to always look at the changes to prevProp and prevState to derive the key. // The second case cannot be used all the time because the onyx data doesn't change the first time that `componentDidUpdate()` runs after loading. In this case, // the `mapping.previousKey` must be used for the comparison or else this logic never detects that onyx data could have changed during the loading process. - const previousKey = isFirstTimeUpdatingAfterLoading ? mapping.previousKey : Str.result(mapping.key, {...prevProps, ...prevOnyxDataFromState}); - const newKey = Str.result(mapping.key, {...this.props, ...onyxDataFromState}); + const previousKey = (isFirstTimeUpdatingAfterLoading ? mapping.previousKey : Str.result(mapping.key, {...prevProps, ...prevOnyxDataFromState})) as OnyxKey; + const newKey = Str.result(mapping.key, {...this.props, ...onyxDataFromState}) as OnyxKey; if (previousKey !== newKey) { Onyx.disconnect(this.activeConnectionIDs[previousKey], previousKey); delete this.activeConnectionIDs[previousKey]; - this.connectMappingToOnyx(mapping, propName, newKey); + this.connectMappingToOnyx(mapping, propName as keyof TOnyxProps, newKey); } }); + this.checkEvictableKeys(); } componentWillUnmount() { // Disconnect everything from Onyx - _.each(mapOnyxToState, (mapping) => { - const key = Str.result(mapping.key, {...this.props, ...getOnyxDataFromState(this.state, mapOnyxToState)}); + Object.keys(mapOnyxToState).forEach((propName) => { + const mapping = mapOnyxToState[propName as keyof TOnyxProps]; + + const key = Str.result(mapping.key, {...this.props, ...getOnyxDataFromState(this.state, mapOnyxToState)}) as OnyxKey; Onyx.disconnect(this.activeConnectionIDs[key], key); }); } - setStateProxy(modifier) { + setStateProxy(modifier: Partial>) { if (this.shouldDelayUpdates) { this.pendingSetStates.push(modifier); } else { - this.setState(modifier); + this.setState(modifier as WithOnyxState); } } @@ -174,7 +330,7 @@ export default function (mapOnyxToState, shouldDelayUpdates = false) { * @param {String} statePropertyName * @param {*} val */ - setWithOnyxState(statePropertyName, val) { + setWithOnyxState(statePropertyName: T, val: WithOnyxState[T]) { const prevValue = this.state[statePropertyName]; // If the component is not loading (read "mounting"), then we can just update the state @@ -190,14 +346,14 @@ export default function (mapOnyxToState, shouldDelayUpdates = false) { return; } - this.setStateProxy({[statePropertyName]: val}); + this.setStateProxy({[statePropertyName]: val as WithOnyxState[T]}); return; } this.tempState[statePropertyName] = val; // If some key does not have a value yet, do not update the state yet - const tempStateIsMissingKey = _.some(requiredKeysForInit, (key) => _.isUndefined(this.tempState[key])); + const tempStateIsMissingKey = requiredKeysForInit.some((key) => this.tempState[key as keyof TOnyxProps] === undefined); if (tempStateIsMissingKey) { return; } @@ -207,35 +363,35 @@ export default function (mapOnyxToState, shouldDelayUpdates = false) { // Full of hacky workarounds to prevent the race condition described above. this.setState((prevState) => { - const finalState = _.reduce( - stateUpdate, - (result, value, key) => { - if (key === 'loading') { - return result; - } - - const initialValue = mapOnyxToState[key].initialValue; - - // If initialValue is there and the state contains something different it means - // an update has already been received and we can discard the value we are trying to hydrate - if (!_.isUndefined(initialValue) && !_.isUndefined(prevState[key]) && prevState[key] !== initialValue) { - // eslint-disable-next-line no-param-reassign - result[key] = prevState[key]; - - // if value is already there (without initial value) then we can discard the value we are trying to hydrate - } else if (!_.isUndefined(prevState[key])) { - // eslint-disable-next-line no-param-reassign - result[key] = prevState[key]; - } else { - // eslint-disable-next-line no-param-reassign - result[key] = value; - } + const finalState = Object.keys(stateUpdate).reduce>((result, _key) => { + const key = _key as keyof TOnyxProps; + + if (key === 'loading') { return result; - }, - {}, - ); + } + + const initialValue = mapOnyxToState[key].initialValue; + + // If initialValue is there and the state contains something different it means + // an update has already been received and we can discard the value we are trying to hydrate + if (initialValue !== undefined && prevState[key] !== undefined && prevState[key] !== initialValue) { + // eslint-disable-next-line no-param-reassign + result[key] = prevState[key]; + + // if value is already there (without initial value) then we can discard the value we are trying to hydrate + } else if (!_.isUndefined(prevState[key])) { + // eslint-disable-next-line no-param-reassign + result[key] = prevState[key]; + } else { + // eslint-disable-next-line no-param-reassign + result[key] = stateUpdate[key]; + } + + return result; + }, {} as WithOnyxState); finalState.loading = false; + return finalState; }); } @@ -249,13 +405,15 @@ export default function (mapOnyxToState, shouldDelayUpdates = false) { // We will add this key to our list of recently accessed keys // if the canEvict function returns true. This is necessary criteria // we MUST use to specify if a key can be removed or not. - _.each(mapOnyxToState, (mapping) => { - if (_.isUndefined(mapping.canEvict)) { + Object.keys(mapOnyxToState).forEach((propName) => { + const mapping = mapOnyxToState[propName as keyof TOnyxProps]; + + if (mapping.canEvict === undefined) { return; } - const canEvict = Str.result(mapping.canEvict, this.props); - const key = Str.result(mapping.key, this.props); + const canEvict = Str.result(mapping.canEvict, this.props) as boolean; + const key = Str.result(mapping.key, this.props) as OnyxKey; if (!OnyxUtils.isSafeEvictionKey(key)) { throw new Error(`canEvict can't be used on key '${key}'. This key must explicitly be flagged as safe for removal by adding it to Onyx.init({safeEvictionKeys: []}).`); @@ -272,15 +430,14 @@ export default function (mapOnyxToState, shouldDelayUpdates = false) { /** * Takes a single mapping and binds the state of the component to the store * - * @param {object} mapping - * @param {string|function} mapping.key key to connect to. can be a string or a + * @param mapping.key key to connect to. can be a string or a * function that takes this.props as an argument and returns a string - * @param {string} statePropertyName the name of the state property that Onyx will add the data to - * @param {boolean} [mapping.initWithStoredValues] If set to false, then no data will be prefilled into the + * @param statePropertyName the name of the state property that Onyx will add the data to + * @param [mapping.initWithStoredValues] If set to false, then no data will be prefilled into the * component - * @param {string} key to connect to Onyx with + * @param key to connect to Onyx with */ - connectMappingToOnyx(mapping, statePropertyName, key) { + connectMappingToOnyx(mapping: MapOnyxToState[keyof TOnyxProps], statePropertyName: keyof TOnyxProps, key: OnyxKey) { // Remember what the previous key was so that key changes can be detected when data is being loaded from Onyx. This will allow // dependent keys to finish loading their data. // eslint-disable-next-line no-param-reassign @@ -290,7 +447,7 @@ export default function (mapOnyxToState, shouldDelayUpdates = false) { this.activeConnectionIDs[key] = Onyx.connect({ ...mapping, key, - statePropertyName, + statePropertyName: statePropertyName as string, withOnyxInstance: this, displayName, }); @@ -304,14 +461,15 @@ export default function (mapOnyxToState, shouldDelayUpdates = false) { this.shouldDelayUpdates = false; this.pendingSetStates.forEach((modifier) => { - this.setState(modifier); + this.setState(modifier as WithOnyxState); }); + this.pendingSetStates = []; } render() { // Remove any null values so that React replaces them with default props - const propsToPass = _.omit(this.props, _.isNull); + const propsToPass = utils.omit(this.props, (entry) => entry[1] === null); if (this.state.loading) { return null; @@ -319,8 +477,7 @@ export default function (mapOnyxToState, shouldDelayUpdates = false) { // Remove any internal state properties used by withOnyx // that should not be passed to a wrapped component - let stateToPass = _.omit(this.state, 'loading'); - stateToPass = _.omit(stateToPass, _.isNull); + const stateToPass = utils.omit(this.state as WithOnyxState, (entry) => entry[0] === 'loading' || entry[1] === null); const stateToPassWithoutNestedNulls = utils.removeNestedNullValues(stateToPass); // Spreading props and state is necessary in an HOC where the data cannot be predicted @@ -348,6 +505,7 @@ export default function (mapOnyxToState, shouldDelayUpdates = false) { forwardedRef: undefined, }; withOnyx.displayName = `withOnyx(${displayName})`; + return React.forwardRef((props, ref) => { const Component = withOnyx; return ( From 708f4c8bb28e09eb3601a493879279c083897a8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Fri, 10 May 2024 14:58:54 +0100 Subject: [PATCH 03/14] Fix more types --- lib/types.ts | 4 +++ lib/withOnyx.tsx | 65 +++++++++++++++++++++++++++++++----------------- 2 files changed, 46 insertions(+), 23 deletions(-) diff --git a/lib/types.ts b/lib/types.ts index 62a4ec1c5..ccc47774b 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -390,6 +390,9 @@ type InitOptions = { debugSetState?: boolean; }; +// eslint-disable-next-line @typescript-eslint/no-explicit-any +type GenericFunction = (...args: any[]) => any; + export type { CollectionKey, CollectionKeyBase, @@ -418,4 +421,5 @@ export type { Mapping, OnyxUpdate, InitOptions, + GenericFunction, }; diff --git a/lib/withOnyx.tsx b/lib/withOnyx.tsx index 011898002..ac785feec 100644 --- a/lib/withOnyx.tsx +++ b/lib/withOnyx.tsx @@ -10,7 +10,19 @@ import _ from 'underscore'; import Onyx from './Onyx'; import OnyxUtils from './OnyxUtils'; import * as Str from './Str'; -import type {CollectionKeyBase, ExtractOnyxCollectionValue, KeyValueMapping, OnyxCollection, OnyxEntry, OnyxKey, OnyxValue, Selector, WithOnyxConnectOptions} from './types'; +import type { + CollectionKeyBase, + ExtractOnyxCollectionValue, + GenericFunction, + KeyValueMapping, + OnyxCollection, + OnyxEntry, + OnyxKey, + OnyxValue, + Selector, + WithOnyxConnectOptions, + WithOnyxInstance, +} from './types'; import utils from './utils'; /** @@ -110,6 +122,11 @@ type Mapping ); +type WithOnyxMapping = Mapping & { + connectionID: number; + previousKey?: OnyxKey; +}; + /** * Represents the mapping options between an Onyx collection key without suffix and the component's prop with all its possibilities. */ @@ -172,14 +189,12 @@ export default function ( shouldDelayUpdates = false, ): React.ComponentType> { // A list of keys that must be present in tempState before we can render the WrappedComponent - const requiredKeysForInit = utils - .omit(mapOnyxToState, (entry) => entry[1].initWithStoredValues === false) - .keys() - .value(); - // const requiredKeysForInit = _.chain(mapOnyxToState) - // .omit((config) => config.initWithStoredValues === false) - // .keys() - // .value(); + const requiredKeysForInit = Object.keys( + utils.omit( + mapOnyxToState as Record[keyof TOnyxProps]>, + (entry: [string, MapOnyxToState[keyof TOnyxProps]]) => entry[1].initWithStoredValues === false, + ), + ); return (WrappedComponent: React.ComponentType): React.ComponentType> => { const displayName = getDisplayName(WrappedComponent); @@ -205,9 +220,9 @@ export default function ( this.activeConnectionIDs = {}; const cachedState = Object.keys(mapOnyxToState).reduce>((resultObj, propName) => { - const mapping = mapOnyxToState[propName as keyof TOnyxProps]; + const mapping = mapOnyxToState[propName as keyof TOnyxProps] as WithOnyxMapping; - const key = Str.result(mapping.key, props) as OnyxKey; + const key = Str.result(mapping.key as GenericFunction, props) as OnyxKey; let value = OnyxUtils.tryGetCachedValue(key, mapping as Partial>) as OnyxValue; if (!value && mapping.initialValue) { value = mapping.initialValue; @@ -246,13 +261,13 @@ export default function ( // Subscribe each of the state properties to the proper Onyx key Object.keys(mapOnyxToState).forEach((propName) => { - const mapping = mapOnyxToState[propName as keyof TOnyxProps]; + const mapping = mapOnyxToState[propName as keyof TOnyxProps] as WithOnyxMapping; if (_.includes(mappingPropertiesToIgnoreChangesTo, propName)) { return; } - const key = Str.result(mapping.key, {...this.props, ...onyxDataFromState}) as OnyxKey; + const key = Str.result(mapping.key as GenericFunction, {...this.props, ...onyxDataFromState}) as OnyxKey; this.connectMappingToOnyx(mapping, propName as keyof TOnyxProps, key); }); @@ -268,7 +283,7 @@ export default function ( const prevOnyxDataFromState = getOnyxDataFromState(prevState, mapOnyxToState); Object.keys(mapOnyxToState).forEach((propName) => { - const mapping = mapOnyxToState[propName as keyof TOnyxProps]; + const mapping = mapOnyxToState[propName as keyof TOnyxProps] as WithOnyxMapping; // Some properties can be ignored because they aren't related to onyx keys and they will never change if (mappingPropertiesToIgnoreChangesTo.includes(propName)) { @@ -282,8 +297,10 @@ export default function ( // (eg. if a user switches chats really quickly). In this case, it's much more stable to always look at the changes to prevProp and prevState to derive the key. // The second case cannot be used all the time because the onyx data doesn't change the first time that `componentDidUpdate()` runs after loading. In this case, // the `mapping.previousKey` must be used for the comparison or else this logic never detects that onyx data could have changed during the loading process. - const previousKey = (isFirstTimeUpdatingAfterLoading ? mapping.previousKey : Str.result(mapping.key, {...prevProps, ...prevOnyxDataFromState})) as OnyxKey; - const newKey = Str.result(mapping.key, {...this.props, ...onyxDataFromState}) as OnyxKey; + const previousKey = ( + isFirstTimeUpdatingAfterLoading ? mapping.previousKey : Str.result(mapping.key as GenericFunction, {...prevProps, ...prevOnyxDataFromState}) + ) as OnyxKey; + const newKey = Str.result(mapping.key as GenericFunction, {...this.props, ...onyxDataFromState}) as OnyxKey; if (previousKey !== newKey) { Onyx.disconnect(this.activeConnectionIDs[previousKey], previousKey); delete this.activeConnectionIDs[previousKey]; @@ -297,9 +314,9 @@ export default function ( componentWillUnmount() { // Disconnect everything from Onyx Object.keys(mapOnyxToState).forEach((propName) => { - const mapping = mapOnyxToState[propName as keyof TOnyxProps]; + const mapping = mapOnyxToState[propName as keyof TOnyxProps] as WithOnyxMapping; - const key = Str.result(mapping.key, {...this.props, ...getOnyxDataFromState(this.state, mapOnyxToState)}) as OnyxKey; + const key = Str.result(mapping.key as GenericFunction, {...this.props, ...getOnyxDataFromState(this.state, mapOnyxToState)}) as OnyxKey; Onyx.disconnect(this.activeConnectionIDs[key], key); }); } @@ -406,14 +423,14 @@ export default function ( // if the canEvict function returns true. This is necessary criteria // we MUST use to specify if a key can be removed or not. Object.keys(mapOnyxToState).forEach((propName) => { - const mapping = mapOnyxToState[propName as keyof TOnyxProps]; + const mapping = mapOnyxToState[propName as keyof TOnyxProps] as WithOnyxMapping; if (mapping.canEvict === undefined) { return; } - const canEvict = Str.result(mapping.canEvict, this.props) as boolean; - const key = Str.result(mapping.key, this.props) as OnyxKey; + const canEvict = Str.result(mapping.canEvict as GenericFunction, this.props) as boolean; + const key = Str.result(mapping.key as GenericFunction, this.props) as OnyxKey; if (!OnyxUtils.isSafeEvictionKey(key)) { throw new Error(`canEvict can't be used on key '${key}'. This key must explicitly be flagged as safe for removal by adding it to Onyx.init({safeEvictionKeys: []}).`); @@ -438,17 +455,19 @@ export default function ( * @param key to connect to Onyx with */ connectMappingToOnyx(mapping: MapOnyxToState[keyof TOnyxProps], statePropertyName: keyof TOnyxProps, key: OnyxKey) { + const onyxMapping = mapOnyxToState[statePropertyName as keyof TOnyxProps] as WithOnyxMapping; + // Remember what the previous key was so that key changes can be detected when data is being loaded from Onyx. This will allow // dependent keys to finish loading their data. // eslint-disable-next-line no-param-reassign - mapOnyxToState[statePropertyName].previousKey = key; + onyxMapping.previousKey = key; // eslint-disable-next-line rulesdir/prefer-onyx-connect-in-libs this.activeConnectionIDs[key] = Onyx.connect({ ...mapping, key, statePropertyName: statePropertyName as string, - withOnyxInstance: this, + withOnyxInstance: this as unknown as WithOnyxInstance, displayName, }); } From ccc8dd5f2917c772e9800c10fbb8188a25b8fbf5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Fri, 10 May 2024 16:04:23 +0100 Subject: [PATCH 04/14] Fix more types --- lib/withOnyx.tsx | 27 ++++++++++----------------- types/modules/react.d.ts | 6 ++++++ 2 files changed, 16 insertions(+), 17 deletions(-) create mode 100644 types/modules/react.d.ts diff --git a/lib/withOnyx.tsx b/lib/withOnyx.tsx index ac785feec..88a3381ce 100644 --- a/lib/withOnyx.tsx +++ b/lib/withOnyx.tsx @@ -3,7 +3,7 @@ * something in Onyx (a key/value store). That way, as soon as data in Onyx changes, the state will be set and the view * will automatically change to reflect the new data. */ -import PropTypes from 'prop-types'; +import type {ForwardedRef} from 'react'; import React from 'react'; import type {IsEqual} from 'type-fest'; import _ from 'underscore'; @@ -158,7 +158,7 @@ type MapOnyxToState = { [TOnyxProp in keyof TOnyxProps]: OnyxPropMapping | OnyxPropCollectionMapping; }; -type WithOnyxProps = Omit; +type WithOnyxProps = Omit & {forwardedRef?: ForwardedRef}; type WithOnyxState = TOnyxProps & { loading: boolean; @@ -200,13 +200,16 @@ export default function ( const displayName = getDisplayName(WrappedComponent); class withOnyx extends React.Component, WithOnyxState> { + // eslint-disable-next-line react/static-property-placement + static displayName: string; + pendingSetStates: Array>> = []; shouldDelayUpdates: boolean; activeConnectionIDs: Record; - tempState: WithOnyxState; + tempState: WithOnyxState | undefined; constructor(props: WithOnyxProps) { super(props); @@ -241,7 +244,7 @@ export default function ( */ if ((value !== undefined && !OnyxUtils.hasPendingMergeForKey(key)) || mapping.allowStaleData) { // eslint-disable-next-line no-param-reassign - resultObj[propName as keyof TOnyxProps] = value; + resultObj[propName as keyof TOnyxProps] = value as WithOnyxState[keyof TOnyxProps]; } return resultObj; @@ -363,14 +366,14 @@ export default function ( return; } - this.setStateProxy({[statePropertyName]: val as WithOnyxState[T]}); + this.setStateProxy({[statePropertyName]: val} as WithOnyxState); return; } this.tempState[statePropertyName] = val; // If some key does not have a value yet, do not update the state yet - const tempStateIsMissingKey = requiredKeysForInit.some((key) => this.tempState[key as keyof TOnyxProps] === undefined); + const tempStateIsMissingKey = requiredKeysForInit.some((key) => this.tempState?.[key as keyof TOnyxProps] === undefined); if (tempStateIsMissingKey) { return; } @@ -488,7 +491,7 @@ export default function ( render() { // Remove any null values so that React replaces them with default props - const propsToPass = utils.omit(this.props, (entry) => entry[1] === null); + const propsToPass = utils.omit(this.props as Omit, (entry) => entry[1] === null); if (this.state.loading) { return null; @@ -513,16 +516,6 @@ export default function ( } } - withOnyx.propTypes = { - forwardedRef: PropTypes.oneOfType([ - PropTypes.func, - // eslint-disable-next-line react/forbid-prop-types - PropTypes.shape({current: PropTypes.object}), - ]), - }; - withOnyx.defaultProps = { - forwardedRef: undefined, - }; withOnyx.displayName = `withOnyx(${displayName})`; return React.forwardRef((props, ref) => { diff --git a/types/modules/react.d.ts b/types/modules/react.d.ts new file mode 100644 index 000000000..3e2e8fb37 --- /dev/null +++ b/types/modules/react.d.ts @@ -0,0 +1,6 @@ +import type React from 'react'; + +declare module 'react' { + // eslint-disable-next-line @typescript-eslint/ban-types + function forwardRef(render: (props: P, ref: React.ForwardedRef) => React.ReactElement | null): (props: P & React.RefAttributes) => React.ReactElement | null; +} From d55adb6eeab75f02e1eeb9d2f4655450008d3e36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Mon, 13 May 2024 14:36:41 +0100 Subject: [PATCH 05/14] Remove remaining underscore usage from withOnyx --- lib/withOnyx.tsx | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/withOnyx.tsx b/lib/withOnyx.tsx index da3e7e554..bd1412685 100644 --- a/lib/withOnyx.tsx +++ b/lib/withOnyx.tsx @@ -6,7 +6,6 @@ import type {ForwardedRef} from 'react'; import React from 'react'; import type {IsEqual} from 'type-fest'; -import _ from 'underscore'; import Onyx from './Onyx'; import OnyxUtils from './OnyxUtils'; import * as Str from './Str'; @@ -251,7 +250,7 @@ export default function ( }, {} as WithOnyxState); // If we have all the data we need, then we can render the component immediately - cachedState.loading = _.size(cachedState) < requiredKeysForInit.length; + cachedState.loading = Object.keys(cachedState).length < requiredKeysForInit.length; // Object holding the temporary initial state for the component while we load the various Onyx keys this.tempState = cachedState; @@ -266,7 +265,7 @@ export default function ( Object.keys(mapOnyxToState).forEach((propName) => { const mapping = mapOnyxToState[propName as keyof TOnyxProps] as WithOnyxMapping; - if (_.includes(mappingPropertiesToIgnoreChangesTo, propName)) { + if (mappingPropertiesToIgnoreChangesTo.includes(propName)) { return; } @@ -399,7 +398,7 @@ export default function ( result[key] = prevState[key]; // if value is already there (without initial value) then we can discard the value we are trying to hydrate - } else if (!_.isUndefined(prevState[key])) { + } else if (prevState[key] !== undefined) { // eslint-disable-next-line no-param-reassign result[key] = prevState[key]; } else { From 81d30441c0ea88f90c8bf20844453f14ac497e9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Wed, 15 May 2024 18:13:48 +0100 Subject: [PATCH 06/14] Address review comments and reduce assertions --- lib/utils.ts | 68 +++++++++++++++++++++++------------------- lib/withOnyx.tsx | 77 ++++++++++++++++++++++++++++++------------------ 2 files changed, 86 insertions(+), 59 deletions(-) diff --git a/lib/utils.ts b/lib/utils.ts index 54700fd7b..fb64cea7e 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -138,22 +138,31 @@ function checkCompatibilityWithExistingValue(value: unknown, existingValue: unkn }; } -function pick(obj: Record, condition: string | string[] | ((entry: [string, T]) => boolean)): Record { - const result: Record = {}; - +/** + * Filters an object based on a condition and an inclusion flag. + * + * @param obj - The object to filter. + * @param condition - The condition to apply. + * @param include - If true, include entries that match the condition; otherwise, exclude them. + * @returns The filtered object. + */ +function filterObject(obj: Record, condition: string | string[] | ((entry: [string, TValue]) => boolean), include: boolean): Record { + const result: Record = {}; const entries = Object.entries(obj); + for (let i = 0; i < entries.length; i++) { const [key, value] = entries[i]; + let shouldInclude: boolean; if (Array.isArray(condition)) { - if (condition.includes(key)) { - result[key] = value; - } + shouldInclude = condition.includes(key); } else if (typeof condition === 'string') { - if (key === condition) { - result[key] = value; - } - } else if (condition(entries[i])) { + shouldInclude = key === condition; + } else { + shouldInclude = condition(entries[i]); + } + + if (include ? shouldInclude : !shouldInclude) { result[key] = value; } } @@ -161,27 +170,26 @@ function pick(obj: Record, condition: string | string[] | ((entry: return result; } -function omit(obj: Record, condition: string | string[] | ((entry: [string, T]) => boolean)): Record { - const result: Record = {}; - - const entries = Object.entries(obj); - for (let i = 0; i < entries.length; i++) { - const [key, value] = entries[i]; - - if (Array.isArray(condition)) { - if (!condition.includes(key)) { - result[key] = value; - } - } else if (typeof condition === 'string') { - if (key !== condition) { - result[key] = value; - } - } else if (!condition(entries[i])) { - result[key] = value; - } - } +/** + * Picks entries from an object based on a condition. + * + * @param obj - The object to pick entries from. + * @param condition - The condition to determine which entries to pick. + * @returns The object containing only the picked entries. + */ +function pick(obj: Record, condition: string | string[] | ((entry: [string, TValue]) => boolean)): Record { + return filterObject(obj, condition, true); +} - return result; +/** + * Omits entries from an object based on a condition. + * + * @param obj - The object to omit entries from. + * @param condition - The condition to determine which entries to omit. + * @returns The object containing only the remaining entries after omission. + */ +function omit(obj: Record, condition: string | string[] | ((entry: [string, TValue]) => boolean)): Record { + return filterObject(obj, condition, false); } export default {isEmptyObject, fastMerge, formatActionName, removeNestedNullValues, checkCompatibilityWithExistingValue, pick, omit}; diff --git a/lib/withOnyx.tsx b/lib/withOnyx.tsx index bd1412685..d34c33670 100644 --- a/lib/withOnyx.tsx +++ b/lib/withOnyx.tsx @@ -17,7 +17,6 @@ import type { OnyxCollection, OnyxEntry, OnyxKey, - OnyxValue, Selector, WithOnyxConnectOptions, WithOnyxInstance, @@ -33,10 +32,16 @@ type BaseMapping = { allowStaleData?: boolean; }; +/** + * Represents the base mapping options when an Onyx collection key is supplied. + */ type CollectionBaseMapping = { initialValue?: OnyxCollection; }; +/** + * Represents the base mapping options when an Onyx non-collection key is supplied. + */ type EntryBaseMapping = { initialValue?: OnyxEntry; }; @@ -121,6 +126,9 @@ type Mapping ); +/** + * Represents a superset of `Mapping` type with internal properties included. + */ type WithOnyxMapping = Mapping & { connectionID: number; previousKey?: OnyxKey; @@ -153,12 +161,21 @@ type OnyxPropCollectionMapping; }[CollectionKeyBase]; +/** + * Represents an Onyx mapping object that connects Onyx keys to component's props. + */ type MapOnyxToState = { [TOnyxProp in keyof TOnyxProps]: OnyxPropMapping | OnyxPropCollectionMapping; }; +/** + * Represents the `withOnyx` internal component props. + */ type WithOnyxProps = Omit & {forwardedRef?: ForwardedRef}; +/** + * Represents the `withOnyx` internal component state. + */ type WithOnyxState = TOnyxProps & { loading: boolean; }; @@ -180,19 +197,24 @@ function getDisplayName(component: React.ComponentType(state: WithOnyxState, onyxToStateMapping: MapOnyxToState) => - utils.pick(state, Object.keys(onyxToStateMapping)) as WithOnyxState; +function getOnyxDataFromState(state: WithOnyxState, onyxToStateMapping: MapOnyxToState) { + return utils.pick(state, Object.keys(onyxToStateMapping)); +} + +/** + * Utility function to return the keys properly typed of the `withOnyx` mapping object. + */ +function mapOnyxToStateKeys(mapOnyxToState: MapOnyxToState) { + return Object.keys(mapOnyxToState) as Array; +} export default function ( mapOnyxToState: MapOnyxToState, shouldDelayUpdates = false, -): React.ComponentType> { +): (component: React.ComponentType) => React.ComponentType> { // A list of keys that must be present in tempState before we can render the WrappedComponent const requiredKeysForInit = Object.keys( - utils.omit( - mapOnyxToState as Record[keyof TOnyxProps]>, - (entry: [string, MapOnyxToState[keyof TOnyxProps]]) => entry[1].initWithStoredValues === false, - ), + utils.omit(mapOnyxToState, ([, options]) => (options as MapOnyxToState[keyof TOnyxProps]).initWithStoredValues === false), ); return (WrappedComponent: React.ComponentType): React.ComponentType> => { @@ -221,11 +243,11 @@ export default function ( // disconnected. It is a key value store with the format {[mapping.key]: connectionID}. this.activeConnectionIDs = {}; - const cachedState = Object.keys(mapOnyxToState).reduce>((resultObj, propName) => { - const mapping = mapOnyxToState[propName as keyof TOnyxProps] as WithOnyxMapping; + const cachedState = mapOnyxToStateKeys(mapOnyxToState).reduce>((resultObj, propName) => { + const mapping = mapOnyxToState[propName]; const key = Str.result(mapping.key as GenericFunction, props) as OnyxKey; - let value = OnyxUtils.tryGetCachedValue(key, mapping as Partial>) as OnyxValue; + let value = OnyxUtils.tryGetCachedValue(key, mapping as Partial>); if (!value && mapping.initialValue) { value = mapping.initialValue; } @@ -243,7 +265,7 @@ export default function ( */ if (mapping.initWithStoredValues !== false && ((value !== undefined && !OnyxUtils.hasPendingMergeForKey(key)) || mapping.allowStaleData)) { // eslint-disable-next-line no-param-reassign - resultObj[propName as keyof TOnyxProps] = value as WithOnyxState[keyof TOnyxProps]; + resultObj[propName] = value as WithOnyxState[keyof TOnyxProps]; } return resultObj; @@ -262,15 +284,15 @@ export default function ( const onyxDataFromState = getOnyxDataFromState(this.state, mapOnyxToState); // Subscribe each of the state properties to the proper Onyx key - Object.keys(mapOnyxToState).forEach((propName) => { - const mapping = mapOnyxToState[propName as keyof TOnyxProps] as WithOnyxMapping; + mapOnyxToStateKeys(mapOnyxToState).forEach((propName) => { + const mapping = mapOnyxToState[propName]; - if (mappingPropertiesToIgnoreChangesTo.includes(propName)) { + if (mappingPropertiesToIgnoreChangesTo.some((prop) => prop === propName)) { return; } const key = Str.result(mapping.key as GenericFunction, {...this.props, ...onyxDataFromState}) as OnyxKey; - this.connectMappingToOnyx(mapping, propName as keyof TOnyxProps, key); + this.connectMappingToOnyx(mapping, propName, key); }); this.checkEvictableKeys(); @@ -284,11 +306,11 @@ export default function ( const onyxDataFromState = getOnyxDataFromState(this.state, mapOnyxToState); const prevOnyxDataFromState = getOnyxDataFromState(prevState, mapOnyxToState); - Object.keys(mapOnyxToState).forEach((propName) => { - const mapping = mapOnyxToState[propName as keyof TOnyxProps] as WithOnyxMapping; + mapOnyxToStateKeys(mapOnyxToState).forEach((propName) => { + const mapping = mapOnyxToState[propName] as WithOnyxMapping; // Some properties can be ignored because they aren't related to onyx keys and they will never change - if (mappingPropertiesToIgnoreChangesTo.includes(propName)) { + if (mappingPropertiesToIgnoreChangesTo.some((prop) => prop === propName)) { return; } @@ -306,7 +328,7 @@ export default function ( if (previousKey !== newKey) { Onyx.disconnect(this.activeConnectionIDs[previousKey], previousKey); delete this.activeConnectionIDs[previousKey]; - this.connectMappingToOnyx(mapping, propName as keyof TOnyxProps, newKey); + this.connectMappingToOnyx(mapping, propName, newKey); } }); @@ -315,8 +337,8 @@ export default function ( componentWillUnmount() { // Disconnect everything from Onyx - Object.keys(mapOnyxToState).forEach((propName) => { - const mapping = mapOnyxToState[propName as keyof TOnyxProps] as WithOnyxMapping; + mapOnyxToStateKeys(mapOnyxToState).forEach((propName) => { + const mapping = mapOnyxToState[propName]; const key = Str.result(mapping.key as GenericFunction, {...this.props, ...getOnyxDataFromState(this.state, mapOnyxToState)}) as OnyxKey; Onyx.disconnect(this.activeConnectionIDs[key], key); @@ -345,9 +367,6 @@ export default function ( * We however need to workaround this issue in the HOC. The addition of initialValue makes things even more complex, * since you cannot be really sure if the component has been updated before or after the initial hydration. Therefore if * initialValue is there, we just check if the update is different than that and then try to handle it as best as we can. - * - * @param {String} statePropertyName - * @param {*} val */ setWithOnyxState(statePropertyName: T, val: WithOnyxState[T]) { const prevValue = this.state[statePropertyName]; @@ -424,8 +443,8 @@ export default function ( // We will add this key to our list of recently accessed keys // if the canEvict function returns true. This is necessary criteria // we MUST use to specify if a key can be removed or not. - Object.keys(mapOnyxToState).forEach((propName) => { - const mapping = mapOnyxToState[propName as keyof TOnyxProps] as WithOnyxMapping; + mapOnyxToStateKeys(mapOnyxToState).forEach((propName) => { + const mapping = mapOnyxToState[propName] as WithOnyxMapping; if (mapping.canEvict === undefined) { return; @@ -457,7 +476,7 @@ export default function ( * @param key to connect to Onyx with */ connectMappingToOnyx(mapping: MapOnyxToState[keyof TOnyxProps], statePropertyName: keyof TOnyxProps, key: OnyxKey) { - const onyxMapping = mapOnyxToState[statePropertyName as keyof TOnyxProps] as WithOnyxMapping; + const onyxMapping = mapOnyxToState[statePropertyName] as WithOnyxMapping; // Remember what the previous key was so that key changes can be detected when data is being loaded from Onyx. This will allow // dependent keys to finish loading their data. @@ -506,7 +525,7 @@ export default function ( Date: Thu, 16 May 2024 09:41:12 +0100 Subject: [PATCH 07/14] Refactor Object.keys() to Object.entries() and improve types --- lib/withOnyx.tsx | 34 ++++++++++++---------------------- 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/lib/withOnyx.tsx b/lib/withOnyx.tsx index d34c33670..1a0bf47ff 100644 --- a/lib/withOnyx.tsx +++ b/lib/withOnyx.tsx @@ -182,7 +182,7 @@ type WithOnyxState = TOnyxProps & { // This is a list of keys that can exist on a `mapping`, but are not directly related to loading data from Onyx. When the keys of a mapping are looped over to check // if a key has changed, it's a good idea to skip looking at these properties since they would have unexpected results. -const mappingPropertiesToIgnoreChangesTo = ['initialValue', 'allowStaleData']; +const mappingPropertiesToIgnoreChangesTo: Array = ['initialValue', 'allowStaleData']; /** * Returns the display name of a component @@ -198,14 +198,14 @@ function getDisplayName(component: React.ComponentType(state: WithOnyxState, onyxToStateMapping: MapOnyxToState) { - return utils.pick(state, Object.keys(onyxToStateMapping)); + return utils.pick(state, Object.keys(onyxToStateMapping)) as Partial>; } /** - * Utility function to return the keys properly typed of the `withOnyx` mapping object. + * Utility function to return the properly typed entries of the `withOnyx` mapping object. */ -function mapOnyxToStateKeys(mapOnyxToState: MapOnyxToState) { - return Object.keys(mapOnyxToState) as Array; +function mapOnyxToStateEntries(mapOnyxToState: MapOnyxToState) { + return Object.entries(mapOnyxToState) as Array<[keyof TOnyxProps, WithOnyxMapping]>; } export default function ( @@ -243,9 +243,7 @@ export default function ( // disconnected. It is a key value store with the format {[mapping.key]: connectionID}. this.activeConnectionIDs = {}; - const cachedState = mapOnyxToStateKeys(mapOnyxToState).reduce>((resultObj, propName) => { - const mapping = mapOnyxToState[propName]; - + const cachedState = mapOnyxToStateEntries(mapOnyxToState).reduce>((resultObj, [propName, mapping]) => { const key = Str.result(mapping.key as GenericFunction, props) as OnyxKey; let value = OnyxUtils.tryGetCachedValue(key, mapping as Partial>); if (!value && mapping.initialValue) { @@ -284,10 +282,8 @@ export default function ( const onyxDataFromState = getOnyxDataFromState(this.state, mapOnyxToState); // Subscribe each of the state properties to the proper Onyx key - mapOnyxToStateKeys(mapOnyxToState).forEach((propName) => { - const mapping = mapOnyxToState[propName]; - - if (mappingPropertiesToIgnoreChangesTo.some((prop) => prop === propName)) { + mapOnyxToStateEntries(mapOnyxToState).forEach(([propName, mapping]) => { + if (mappingPropertiesToIgnoreChangesTo.includes(propName)) { return; } @@ -306,11 +302,9 @@ export default function ( const onyxDataFromState = getOnyxDataFromState(this.state, mapOnyxToState); const prevOnyxDataFromState = getOnyxDataFromState(prevState, mapOnyxToState); - mapOnyxToStateKeys(mapOnyxToState).forEach((propName) => { - const mapping = mapOnyxToState[propName] as WithOnyxMapping; - + mapOnyxToStateEntries(mapOnyxToState).forEach(([propName, mapping]) => { // Some properties can be ignored because they aren't related to onyx keys and they will never change - if (mappingPropertiesToIgnoreChangesTo.some((prop) => prop === propName)) { + if (mappingPropertiesToIgnoreChangesTo.includes(propName)) { return; } @@ -337,9 +331,7 @@ export default function ( componentWillUnmount() { // Disconnect everything from Onyx - mapOnyxToStateKeys(mapOnyxToState).forEach((propName) => { - const mapping = mapOnyxToState[propName]; - + mapOnyxToStateEntries(mapOnyxToState).forEach(([, mapping]) => { const key = Str.result(mapping.key as GenericFunction, {...this.props, ...getOnyxDataFromState(this.state, mapOnyxToState)}) as OnyxKey; Onyx.disconnect(this.activeConnectionIDs[key], key); }); @@ -443,9 +435,7 @@ export default function ( // We will add this key to our list of recently accessed keys // if the canEvict function returns true. This is necessary criteria // we MUST use to specify if a key can be removed or not. - mapOnyxToStateKeys(mapOnyxToState).forEach((propName) => { - const mapping = mapOnyxToState[propName] as WithOnyxMapping; - + mapOnyxToStateEntries(mapOnyxToState).forEach(([, mapping]) => { if (mapping.canEvict === undefined) { return; } From 68954bdb0306f1b072639fad0a47dbc4a00d9f5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Thu, 16 May 2024 09:48:36 +0100 Subject: [PATCH 08/14] Remove some assertions --- lib/withOnyx.tsx | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/lib/withOnyx.tsx b/lib/withOnyx.tsx index 1a0bf47ff..043dc3576 100644 --- a/lib/withOnyx.tsx +++ b/lib/withOnyx.tsx @@ -244,7 +244,7 @@ export default function ( this.activeConnectionIDs = {}; const cachedState = mapOnyxToStateEntries(mapOnyxToState).reduce>((resultObj, [propName, mapping]) => { - const key = Str.result(mapping.key as GenericFunction, props) as OnyxKey; + const key = Str.result(mapping.key as GenericFunction, props); let value = OnyxUtils.tryGetCachedValue(key, mapping as Partial>); if (!value && mapping.initialValue) { value = mapping.initialValue; @@ -287,7 +287,7 @@ export default function ( return; } - const key = Str.result(mapping.key as GenericFunction, {...this.props, ...onyxDataFromState}) as OnyxKey; + const key = Str.result(mapping.key as GenericFunction, {...this.props, ...onyxDataFromState}); this.connectMappingToOnyx(mapping, propName, key); }); @@ -315,10 +315,8 @@ export default function ( // (eg. if a user switches chats really quickly). In this case, it's much more stable to always look at the changes to prevProp and prevState to derive the key. // The second case cannot be used all the time because the onyx data doesn't change the first time that `componentDidUpdate()` runs after loading. In this case, // the `mapping.previousKey` must be used for the comparison or else this logic never detects that onyx data could have changed during the loading process. - const previousKey = ( - isFirstTimeUpdatingAfterLoading ? mapping.previousKey : Str.result(mapping.key as GenericFunction, {...prevProps, ...prevOnyxDataFromState}) - ) as OnyxKey; - const newKey = Str.result(mapping.key as GenericFunction, {...this.props, ...onyxDataFromState}) as OnyxKey; + const previousKey = isFirstTimeUpdatingAfterLoading ? mapping.previousKey : Str.result(mapping.key as GenericFunction, {...prevProps, ...prevOnyxDataFromState}); + const newKey = Str.result(mapping.key as GenericFunction, {...this.props, ...onyxDataFromState}); if (previousKey !== newKey) { Onyx.disconnect(this.activeConnectionIDs[previousKey], previousKey); delete this.activeConnectionIDs[previousKey]; @@ -332,16 +330,16 @@ export default function ( componentWillUnmount() { // Disconnect everything from Onyx mapOnyxToStateEntries(mapOnyxToState).forEach(([, mapping]) => { - const key = Str.result(mapping.key as GenericFunction, {...this.props, ...getOnyxDataFromState(this.state, mapOnyxToState)}) as OnyxKey; + const key = Str.result(mapping.key as GenericFunction, {...this.props, ...getOnyxDataFromState(this.state, mapOnyxToState)}); Onyx.disconnect(this.activeConnectionIDs[key], key); }); } - setStateProxy(modifier: Partial>) { + setStateProxy(modifier: WithOnyxState) { if (this.shouldDelayUpdates) { this.pendingSetStates.push(modifier); } else { - this.setState(modifier as WithOnyxState); + this.setState(modifier); } } @@ -441,7 +439,7 @@ export default function ( } const canEvict = Str.result(mapping.canEvict as GenericFunction, this.props) as boolean; - const key = Str.result(mapping.key as GenericFunction, this.props) as OnyxKey; + const key = Str.result(mapping.key as GenericFunction, this.props); if (!OnyxUtils.isSafeEvictionKey(key)) { throw new Error(`canEvict can't be used on key '${key}'. This key must explicitly be flagged as safe for removal by adding it to Onyx.init({safeEvictionKeys: []}).`); From 4aafd8a25f662d36ae2366159aa986afff541e88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Thu, 16 May 2024 16:54:45 +0100 Subject: [PATCH 09/14] Improve WithOnyxInstance types --- lib/OnyxUtils.ts | 28 ++++++++++++++-------------- lib/index.ts | 25 +++++++++++++------------ lib/types.ts | 16 ++-------------- lib/withOnyx.tsx | 17 ++++++++++++++--- 4 files changed, 43 insertions(+), 43 deletions(-) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index e06496851..d544eb519 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -3,32 +3,32 @@ import {deepEqual} from 'fast-equals'; import lodashClone from 'lodash/clone'; import type {ValueOf} from 'type-fest'; +import DevTools from './DevTools'; import * as Logger from './Logger'; +import type Onyx from './Onyx'; import cache from './OnyxCache'; -import * as Str from './Str'; import * as PerformanceUtils from './PerformanceUtils'; -import Storage from './storage'; -import utils from './utils'; +import * as Str from './Str'; import unstable_batchedUpdates from './batch'; -import DevTools from './DevTools'; +import Storage from './storage'; import type { - DeepRecord, - Mapping, CollectionKey, CollectionKeyBase, + DeepRecord, + DefaultConnectCallback, + DefaultConnectOptions, + KeyValueMapping, + Mapping, NullableKeyValueMapping, + OnyxCollection, + OnyxEntry, OnyxKey, OnyxValue, Selector, - WithOnyxInstanceState, - OnyxCollection, WithOnyxConnectOptions, - DefaultConnectOptions, - OnyxEntry, - KeyValueMapping, - DefaultConnectCallback, } from './types'; -import type Onyx from './Onyx'; +import utils from './utils'; +import type {WithOnyxState} from './withOnyx'; // Method constants const METHOD = { @@ -195,7 +195,7 @@ function batchUpdates(updates: () => void): Promise { function reduceCollectionWithSelector( collection: OnyxCollection, selector: Selector, - withOnyxInstanceState: WithOnyxInstanceState | undefined, + withOnyxInstanceState: WithOnyxState | undefined, ): Record { return Object.entries(collection ?? {}).reduce((finalCollection: Record, [key, item]) => { // eslint-disable-next-line no-param-reassign diff --git a/lib/index.ts b/lib/index.ts index e7e818893..63a727eec 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -1,25 +1,26 @@ +import type {ConnectOptions, OnyxUpdate} from './Onyx'; import Onyx from './Onyx'; -import type {OnyxUpdate, ConnectOptions} from './Onyx'; -import type {CustomTypeOptions, OnyxCollection, OnyxEntry, NullishDeep, KeyValueMapping, OnyxKey, Selector, WithOnyxInstanceState, OnyxValue} from './types'; -import type {UseOnyxResult, FetchStatus, ResultMetadata} from './useOnyx'; +import type {CustomTypeOptions, KeyValueMapping, NullishDeep, OnyxCollection, OnyxEntry, OnyxKey, OnyxValue, Selector} from './types'; +import type {FetchStatus, ResultMetadata, UseOnyxResult} from './useOnyx'; import useOnyx from './useOnyx'; +import type {WithOnyxState} from './withOnyx'; import withOnyx from './withOnyx'; export default Onyx; -export {withOnyx, useOnyx}; +export {useOnyx, withOnyx}; export type { + ConnectOptions, CustomTypeOptions, + FetchStatus, + KeyValueMapping, + NullishDeep, OnyxCollection, OnyxEntry, - OnyxUpdate, - ConnectOptions, - NullishDeep, - KeyValueMapping, OnyxKey, - Selector, - WithOnyxInstanceState, - UseOnyxResult, + OnyxUpdate, OnyxValue, - FetchStatus, ResultMetadata, + Selector, + UseOnyxResult, + WithOnyxState, }; diff --git a/lib/types.ts b/lib/types.ts index d5921cbe9..db0eaea77 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -1,7 +1,7 @@ -import type {Component} from 'react'; import type {Merge} from 'type-fest'; import type {BuiltIns} from 'type-fest/source/internal'; import type OnyxUtils from './OnyxUtils'; +import type {WithOnyxInstance, WithOnyxState} from './withOnyx'; /** * Utility type that excludes `null` from the type `TValue`. @@ -148,7 +148,7 @@ type NullableKeyValueMapping = { * The type `TKey` extends `OnyxKey` and it is the key used to access a value in `KeyValueMapping`. * `TReturnType` is the type of the returned value from the selector function. */ -type Selector = (value: OnyxEntry, state: WithOnyxInstanceState) => TReturnType; +type Selector = (value: OnyxEntry, state?: WithOnyxState) => TReturnType; /** * Represents a single Onyx entry, that can be either `TOnyxValue` or `null` / `undefined` if it doesn't exist. @@ -252,11 +252,6 @@ type NullishObjectDeep = { [KeyType in keyof ObjectType]?: NullishDeep | null; }; -/** - * Represents withOnyx's internal state, containing the Onyx props and a `loading` flag. - */ -type WithOnyxInstanceState = (TOnyxProps & {loading: boolean}) | undefined; - /** * Represents a mapping between Onyx collection keys and their respective values. * @@ -274,11 +269,6 @@ type Collection = { : never; }; -type WithOnyxInstance = Component> & { - setStateProxy: (cb: (state: Record>) => OnyxValue) => void; - setWithOnyxState: (statePropertyName: OnyxKey, value: OnyxValue) => void; -}; - /** Represents the base options used in `Onyx.connect()` method. */ type BaseConnectOptions = { initWithStoredValues?: boolean; @@ -432,6 +422,4 @@ export type { OnyxValue, Selector, WithOnyxConnectOptions, - WithOnyxInstance, - WithOnyxInstanceState, }; diff --git a/lib/withOnyx.tsx b/lib/withOnyx.tsx index 043dc3576..50d22f03d 100644 --- a/lib/withOnyx.tsx +++ b/lib/withOnyx.tsx @@ -14,12 +14,13 @@ import type { ExtractOnyxCollectionValue, GenericFunction, KeyValueMapping, + NullableKeyValueMapping, OnyxCollection, OnyxEntry, OnyxKey, + OnyxValue, Selector, WithOnyxConnectOptions, - WithOnyxInstance, } from './types'; import utils from './utils'; @@ -180,6 +181,14 @@ type WithOnyxState = TOnyxProps & { loading: boolean; }; +/** + * Represents the `withOnyx` internal component instance. + */ +type WithOnyxInstance = React.Component> & { + setStateProxy: (modifier: Record> | ((state: Record>) => OnyxValue)) => void; + setWithOnyxState: (statePropertyName: OnyxKey, value: OnyxValue) => void; +}; + // This is a list of keys that can exist on a `mapping`, but are not directly related to loading data from Onyx. When the keys of a mapping are looped over to check // if a key has changed, it's a good idea to skip looking at these properties since they would have unexpected results. const mappingPropertiesToIgnoreChangesTo: Array = ['initialValue', 'allowStaleData']; @@ -224,7 +233,7 @@ export default function ( // eslint-disable-next-line react/static-property-placement static displayName: string; - pendingSetStates: Array>> = []; + pendingSetStates: Array | ((state: WithOnyxState) => WithOnyxState | null)> = []; shouldDelayUpdates: boolean; @@ -335,7 +344,7 @@ export default function ( }); } - setStateProxy(modifier: WithOnyxState) { + setStateProxy(modifier: WithOnyxState | ((state: WithOnyxState) => WithOnyxState | null)) { if (this.shouldDelayUpdates) { this.pendingSetStates.push(modifier); } else { @@ -536,3 +545,5 @@ export default function ( }); }; } + +export type {WithOnyxState, WithOnyxInstance}; From a6f3e05e113984cbf76409dd3ca9761697bae0e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Thu, 16 May 2024 18:55:10 +0100 Subject: [PATCH 10/14] Moves react.d.ts inside lib and excludes it from dist --- {types => lib/types}/modules/react.d.ts | 0 lib/withOnyx.d.ts | 141 ------------------------ lib/withOnyx.tsx | 7 ++ package.json | 2 +- 4 files changed, 8 insertions(+), 142 deletions(-) rename {types => lib/types}/modules/react.d.ts (100%) delete mode 100644 lib/withOnyx.d.ts diff --git a/types/modules/react.d.ts b/lib/types/modules/react.d.ts similarity index 100% rename from types/modules/react.d.ts rename to lib/types/modules/react.d.ts diff --git a/lib/withOnyx.d.ts b/lib/withOnyx.d.ts deleted file mode 100644 index f665cea9f..000000000 --- a/lib/withOnyx.d.ts +++ /dev/null @@ -1,141 +0,0 @@ -import {IsEqual} from 'type-fest'; -import {CollectionKeyBase, ExtractOnyxCollectionValue, KeyValueMapping, OnyxCollection, OnyxEntry, OnyxKey, Selector} from './types'; - -/** - * Represents the base mapping options between an Onyx key and the component's prop. - */ -type BaseMapping = { - canEvict?: boolean | ((props: Omit) => boolean); - initWithStoredValues?: boolean; - allowStaleData?: boolean; -}; - -type CollectionBaseMapping = { - initialValue?: OnyxCollection; -}; - -type EntryBaseMapping = { - initialValue?: OnyxEntry; -}; - -/** - * Represents the string / function `key` mapping option between an Onyx key and the component's prop. - * - * If `key` is `string`, the type of the Onyx value that is associated with `key` must match with the type of the component's prop, - * otherwise an error will be thrown. - * - * If `key` is `function`, the return type of `key` function must be a valid Onyx key and the type of the Onyx value associated - * with `key` must match with the type of the component's prop, otherwise an error will be thrown. - * - * @example - * ```ts - * // Onyx prop with `string` key - * onyxProp: { - * key: ONYXKEYS.ACCOUNT, - * }, - * - * // Onyx prop with `function` key - * onyxProp: { - * key: ({reportId}) => ONYXKEYS.ACCOUNT, - * }, - * ``` - */ -type BaseMappingKey = IsEqual extends true - ? { - key: TOnyxKey | ((props: Omit & Partial) => TOnyxKey); - } - : never; - -/** - * Represents the string `key` and `selector` mapping options between an Onyx key and the component's prop. - * - * The function signature and return type of `selector` must match with the type of the component's prop, - * otherwise an error will be thrown. - * - * @example - * ```ts - * // Onyx prop with `string` key and selector - * onyxProp: { - * key: ONYXKEYS.ACCOUNT, - * selector: (value: Account | null): string => value?.id ?? '', - * }, - * ``` - */ -type BaseMappingStringKeyAndSelector = { - key: TOnyxKey; - selector: Selector; -}; - -/** - * Represents the function `key` and `selector` mapping options between an Onyx key and the component's prop. - * - * The function signature and return type of `selector` must match with the type of the component's prop, - * otherwise an error will be thrown. - * - * @example - * ```ts - * // Onyx prop with `function` key and selector - * onyxProp: { - * key: ({reportId}) => ONYXKEYS.ACCOUNT, - * selector: (value: Account | null) => value?.id ?? '', - * }, - * ``` - */ -type BaseMappingFunctionKeyAndSelector = { - key: (props: Omit & Partial) => TOnyxKey; - selector: Selector; -}; - -/** - * Represents the mapping options between an Onyx key and the component's prop with all its possibilities. - */ -type Mapping = BaseMapping & - EntryBaseMapping & - ( - | BaseMappingKey> - | BaseMappingStringKeyAndSelector - | BaseMappingFunctionKeyAndSelector - ); - -/** - * Represents the mapping options between an Onyx collection key without suffix and the component's prop with all its possibilities. - */ -type CollectionMapping = BaseMapping & - CollectionBaseMapping & - ( - | BaseMappingKey> - | BaseMappingStringKeyAndSelector, TOnyxKey> - | BaseMappingFunctionKeyAndSelector, TOnyxKey> - ); - -/** - * Represents an union type of all the possible Onyx key mappings. - * Each `OnyxPropMapping` will be associated with its respective Onyx key, ensuring different type-safety for each object. - */ -type OnyxPropMapping = { - [TOnyxKey in OnyxKey]: Mapping; -}[OnyxKey]; - -/** - * Represents an union type of all the possible Onyx collection keys without suffix mappings. - * Each `OnyxPropCollectionMapping` will be associated with its respective Onyx key, ensuring different type-safety for each object. - */ -type OnyxPropCollectionMapping = { - [TOnyxKey in CollectionKeyBase]: CollectionMapping; -}[CollectionKeyBase]; - -/** - * @deprecated Use `useOnyx` instead of `withOnyx` whenever possible. - * - * This is a higher order component that provides the ability to map a state property directly to - * something in Onyx (a key/value store). That way, as soon as data in Onyx changes, the state will be set and the view - * will automatically change to reflect the new data. - */ -declare function withOnyx( - mapping: { - [TOnyxProp in keyof TOnyxProps]: OnyxPropMapping | OnyxPropCollectionMapping; - }, - shouldDelayUpdates?: boolean, -): (component: React.ComponentType) => React.ComponentType>; - -export default withOnyx; diff --git a/lib/withOnyx.tsx b/lib/withOnyx.tsx index 50d22f03d..789bcd1e3 100644 --- a/lib/withOnyx.tsx +++ b/lib/withOnyx.tsx @@ -217,6 +217,13 @@ function mapOnyxToStateEntries(mapOnyxToState: MapO return Object.entries(mapOnyxToState) as Array<[keyof TOnyxProps, WithOnyxMapping]>; } +/** + * @deprecated Use `useOnyx` instead of `withOnyx` whenever possible. + * + * This is a higher order component that provides the ability to map a state property directly to + * something in Onyx (a key/value store). That way, as soon as data in Onyx changes, the state will be set and the view + * will automatically change to reflect the new data. + */ export default function ( mapOnyxToState: MapOnyxToState, shouldDelayUpdates = false, diff --git a/package.json b/package.json index 9e520e4ad..44ce969fc 100644 --- a/package.json +++ b/package.json @@ -31,7 +31,7 @@ "lint": "eslint .", "typecheck": "tsc --noEmit", "test": "jest", - "build": "tsc -p tsconfig.build.json && cp ./lib/*.d.ts ./dist", + "build": "tsc -p tsconfig.build.json", "build:watch": "nodemon --watch lib --ext js,json,ts,tsx --exec \"npm run build && npm pack\"", "prebuild:docs": "npm run build", "build:docs": "ts-node buildDocs.ts", From 3238b5dd9b014723ee3dc2a789ec34e3327f35b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Thu, 16 May 2024 19:09:20 +0100 Subject: [PATCH 11/14] Extract withOnyx types to a new file --- lib/OnyxUtils.ts | 2 +- lib/index.ts | 2 +- lib/types.ts | 2 +- lib/{withOnyx.tsx => withOnyx/index.tsx} | 192 +---------------------- lib/withOnyx/types.ts | 170 ++++++++++++++++++++ 5 files changed, 179 insertions(+), 189 deletions(-) rename lib/{withOnyx.tsx => withOnyx/index.tsx} (73%) create mode 100644 lib/withOnyx/types.ts diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index d544eb519..68054766e 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -28,7 +28,7 @@ import type { WithOnyxConnectOptions, } from './types'; import utils from './utils'; -import type {WithOnyxState} from './withOnyx'; +import type {WithOnyxState} from './withOnyx/types'; // Method constants const METHOD = { diff --git a/lib/index.ts b/lib/index.ts index 63a727eec..79bcf8567 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -3,8 +3,8 @@ import Onyx from './Onyx'; import type {CustomTypeOptions, KeyValueMapping, NullishDeep, OnyxCollection, OnyxEntry, OnyxKey, OnyxValue, Selector} from './types'; import type {FetchStatus, ResultMetadata, UseOnyxResult} from './useOnyx'; import useOnyx from './useOnyx'; -import type {WithOnyxState} from './withOnyx'; import withOnyx from './withOnyx'; +import type {WithOnyxState} from './withOnyx/types'; export default Onyx; export {useOnyx, withOnyx}; diff --git a/lib/types.ts b/lib/types.ts index db0eaea77..7ea33e52f 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -1,7 +1,7 @@ import type {Merge} from 'type-fest'; import type {BuiltIns} from 'type-fest/source/internal'; import type OnyxUtils from './OnyxUtils'; -import type {WithOnyxInstance, WithOnyxState} from './withOnyx'; +import type {WithOnyxInstance, WithOnyxState} from './withOnyx/types'; /** * Utility type that excludes `null` from the type `TValue`. diff --git a/lib/withOnyx.tsx b/lib/withOnyx/index.tsx similarity index 73% rename from lib/withOnyx.tsx rename to lib/withOnyx/index.tsx index 789bcd1e3..b6c860791 100644 --- a/lib/withOnyx.tsx +++ b/lib/withOnyx/index.tsx @@ -3,191 +3,13 @@ * something in Onyx (a key/value store). That way, as soon as data in Onyx changes, the state will be set and the view * will automatically change to reflect the new data. */ -import type {ForwardedRef} from 'react'; import React from 'react'; -import type {IsEqual} from 'type-fest'; -import Onyx from './Onyx'; -import OnyxUtils from './OnyxUtils'; -import * as Str from './Str'; -import type { - CollectionKeyBase, - ExtractOnyxCollectionValue, - GenericFunction, - KeyValueMapping, - NullableKeyValueMapping, - OnyxCollection, - OnyxEntry, - OnyxKey, - OnyxValue, - Selector, - WithOnyxConnectOptions, -} from './types'; -import utils from './utils'; - -/** - * Represents the base mapping options between an Onyx key and the component's prop. - */ -type BaseMapping = { - canEvict?: boolean | ((props: Omit) => boolean); - initWithStoredValues?: boolean; - allowStaleData?: boolean; -}; - -/** - * Represents the base mapping options when an Onyx collection key is supplied. - */ -type CollectionBaseMapping = { - initialValue?: OnyxCollection; -}; - -/** - * Represents the base mapping options when an Onyx non-collection key is supplied. - */ -type EntryBaseMapping = { - initialValue?: OnyxEntry; -}; - -/** - * Represents the string / function `key` mapping option between an Onyx key and the component's prop. - * - * If `key` is `string`, the type of the Onyx value that is associated with `key` must match with the type of the component's prop, - * otherwise an error will be thrown. - * - * If `key` is `function`, the return type of `key` function must be a valid Onyx key and the type of the Onyx value associated - * with `key` must match with the type of the component's prop, otherwise an error will be thrown. - * - * @example - * ```ts - * // Onyx prop with `string` key - * onyxProp: { - * key: ONYXKEYS.ACCOUNT, - * }, - * - * // Onyx prop with `function` key - * onyxProp: { - * key: ({reportId}) => ONYXKEYS.ACCOUNT, - * }, - * ``` - */ -type BaseMappingKey = IsEqual extends true - ? { - key: TOnyxKey | ((props: Omit & Partial) => TOnyxKey); - } - : never; - -/** - * Represents the string `key` and `selector` mapping options between an Onyx key and the component's prop. - * - * The function signature and return type of `selector` must match with the type of the component's prop, - * otherwise an error will be thrown. - * - * @example - * ```ts - * // Onyx prop with `string` key and selector - * onyxProp: { - * key: ONYXKEYS.ACCOUNT, - * selector: (value: Account | null): string => value?.id ?? '', - * }, - * ``` - */ -// eslint-disable-next-line @typescript-eslint/no-unused-vars -type BaseMappingStringKeyAndSelector = { - key: TOnyxKey; - selector: Selector; -}; - -/** - * Represents the function `key` and `selector` mapping options between an Onyx key and the component's prop. - * - * The function signature and return type of `selector` must match with the type of the component's prop, - * otherwise an error will be thrown. - * - * @example - * ```ts - * // Onyx prop with `function` key and selector - * onyxProp: { - * key: ({reportId}) => ONYXKEYS.ACCOUNT, - * selector: (value: Account | null) => value?.id ?? '', - * }, - * ``` - */ -type BaseMappingFunctionKeyAndSelector = { - key: (props: Omit & Partial) => TOnyxKey; - selector: Selector; -}; - -/** - * Represents the mapping options between an Onyx key and the component's prop with all its possibilities. - */ -type Mapping = BaseMapping & - EntryBaseMapping & - ( - | BaseMappingKey> - | BaseMappingStringKeyAndSelector - | BaseMappingFunctionKeyAndSelector - ); - -/** - * Represents a superset of `Mapping` type with internal properties included. - */ -type WithOnyxMapping = Mapping & { - connectionID: number; - previousKey?: OnyxKey; -}; - -/** - * Represents the mapping options between an Onyx collection key without suffix and the component's prop with all its possibilities. - */ -type CollectionMapping = BaseMapping & - CollectionBaseMapping & - ( - | BaseMappingKey> - | BaseMappingStringKeyAndSelector, TOnyxKey> - | BaseMappingFunctionKeyAndSelector, TOnyxKey> - ); - -/** - * Represents an union type of all the possible Onyx key mappings. - * Each `OnyxPropMapping` will be associated with its respective Onyx key, ensuring different type-safety for each object. - */ -type OnyxPropMapping = { - [TOnyxKey in OnyxKey]: Mapping; -}[OnyxKey]; - -/** - * Represents an union type of all the possible Onyx collection keys without suffix mappings. - * Each `OnyxPropCollectionMapping` will be associated with its respective Onyx key, ensuring different type-safety for each object. - */ -type OnyxPropCollectionMapping = { - [TOnyxKey in CollectionKeyBase]: CollectionMapping; -}[CollectionKeyBase]; - -/** - * Represents an Onyx mapping object that connects Onyx keys to component's props. - */ -type MapOnyxToState = { - [TOnyxProp in keyof TOnyxProps]: OnyxPropMapping | OnyxPropCollectionMapping; -}; - -/** - * Represents the `withOnyx` internal component props. - */ -type WithOnyxProps = Omit & {forwardedRef?: ForwardedRef}; - -/** - * Represents the `withOnyx` internal component state. - */ -type WithOnyxState = TOnyxProps & { - loading: boolean; -}; - -/** - * Represents the `withOnyx` internal component instance. - */ -type WithOnyxInstance = React.Component> & { - setStateProxy: (modifier: Record> | ((state: Record>) => OnyxValue)) => void; - setWithOnyxState: (statePropertyName: OnyxKey, value: OnyxValue) => void; -}; +import Onyx from '../Onyx'; +import OnyxUtils from '../OnyxUtils'; +import * as Str from '../Str'; +import type {GenericFunction, OnyxKey, WithOnyxConnectOptions} from '../types'; +import utils from '../utils'; +import type {MapOnyxToState, WithOnyxInstance, WithOnyxMapping, WithOnyxProps, WithOnyxState} from './types'; // This is a list of keys that can exist on a `mapping`, but are not directly related to loading data from Onyx. When the keys of a mapping are looped over to check // if a key has changed, it's a good idea to skip looking at these properties since they would have unexpected results. @@ -552,5 +374,3 @@ export default function ( }); }; } - -export type {WithOnyxState, WithOnyxInstance}; diff --git a/lib/withOnyx/types.ts b/lib/withOnyx/types.ts new file mode 100644 index 000000000..06145551a --- /dev/null +++ b/lib/withOnyx/types.ts @@ -0,0 +1,170 @@ +import type {ForwardedRef} from 'react'; +import type {IsEqual} from 'type-fest'; +import type {CollectionKeyBase, ExtractOnyxCollectionValue, KeyValueMapping, NullableKeyValueMapping, OnyxCollection, OnyxEntry, OnyxKey, OnyxValue, Selector} from '../types'; + +/** + * Represents the base mapping options between an Onyx key and the component's prop. + */ +type BaseMapping = { + canEvict?: boolean | ((props: Omit) => boolean); + initWithStoredValues?: boolean; + allowStaleData?: boolean; +}; + +/** + * Represents the base mapping options when an Onyx collection key is supplied. + */ +type CollectionBaseMapping = { + initialValue?: OnyxCollection; +}; + +/** + * Represents the base mapping options when an Onyx non-collection key is supplied. + */ +type EntryBaseMapping = { + initialValue?: OnyxEntry; +}; + +/** + * Represents the string / function `key` mapping option between an Onyx key and the component's prop. + * + * If `key` is `string`, the type of the Onyx value that is associated with `key` must match with the type of the component's prop, + * otherwise an error will be thrown. + * + * If `key` is `function`, the return type of `key` function must be a valid Onyx key and the type of the Onyx value associated + * with `key` must match with the type of the component's prop, otherwise an error will be thrown. + * + * @example + * ```ts + * // Onyx prop with `string` key + * onyxProp: { + * key: ONYXKEYS.ACCOUNT, + * }, + * + * // Onyx prop with `function` key + * onyxProp: { + * key: ({reportId}) => ONYXKEYS.ACCOUNT, + * }, + * ``` + */ +type BaseMappingKey = IsEqual extends true + ? { + key: TOnyxKey | ((props: Omit & Partial) => TOnyxKey); + } + : never; + +/** + * Represents the string `key` and `selector` mapping options between an Onyx key and the component's prop. + * + * The function signature and return type of `selector` must match with the type of the component's prop, + * otherwise an error will be thrown. + * + * @example + * ```ts + * // Onyx prop with `string` key and selector + * onyxProp: { + * key: ONYXKEYS.ACCOUNT, + * selector: (value: Account | null): string => value?.id ?? '', + * }, + * ``` + */ +// eslint-disable-next-line @typescript-eslint/no-unused-vars +type BaseMappingStringKeyAndSelector = { + key: TOnyxKey; + selector: Selector; +}; + +/** + * Represents the function `key` and `selector` mapping options between an Onyx key and the component's prop. + * + * The function signature and return type of `selector` must match with the type of the component's prop, + * otherwise an error will be thrown. + * + * @example + * ```ts + * // Onyx prop with `function` key and selector + * onyxProp: { + * key: ({reportId}) => ONYXKEYS.ACCOUNT, + * selector: (value: Account | null) => value?.id ?? '', + * }, + * ``` + */ +type BaseMappingFunctionKeyAndSelector = { + key: (props: Omit & Partial) => TOnyxKey; + selector: Selector; +}; + +/** + * Represents the mapping options between an Onyx key and the component's prop with all its possibilities. + */ +type Mapping = BaseMapping & + EntryBaseMapping & + ( + | BaseMappingKey> + | BaseMappingStringKeyAndSelector + | BaseMappingFunctionKeyAndSelector + ); + +/** + * Represents a superset of `Mapping` type with internal properties included. + */ +type WithOnyxMapping = Mapping & { + connectionID: number; + previousKey?: OnyxKey; +}; + +/** + * Represents the mapping options between an Onyx collection key without suffix and the component's prop with all its possibilities. + */ +type CollectionMapping = BaseMapping & + CollectionBaseMapping & + ( + | BaseMappingKey> + | BaseMappingStringKeyAndSelector, TOnyxKey> + | BaseMappingFunctionKeyAndSelector, TOnyxKey> + ); + +/** + * Represents an union type of all the possible Onyx key mappings. + * Each `OnyxPropMapping` will be associated with its respective Onyx key, ensuring different type-safety for each object. + */ +type OnyxPropMapping = { + [TOnyxKey in OnyxKey]: Mapping; +}[OnyxKey]; + +/** + * Represents an union type of all the possible Onyx collection keys without suffix mappings. + * Each `OnyxPropCollectionMapping` will be associated with its respective Onyx key, ensuring different type-safety for each object. + */ +type OnyxPropCollectionMapping = { + [TOnyxKey in CollectionKeyBase]: CollectionMapping; +}[CollectionKeyBase]; + +/** + * Represents an Onyx mapping object that connects Onyx keys to component's props. + */ +type MapOnyxToState = { + [TOnyxProp in keyof TOnyxProps]: OnyxPropMapping | OnyxPropCollectionMapping; +}; + +/** + * Represents the `withOnyx` internal component props. + */ +type WithOnyxProps = Omit & {forwardedRef?: ForwardedRef}; + +/** + * Represents the `withOnyx` internal component state. + */ +type WithOnyxState = TOnyxProps & { + loading: boolean; +}; + +/** + * Represents the `withOnyx` internal component instance. + */ +type WithOnyxInstance = React.Component> & { + setStateProxy: (modifier: Record> | ((state: Record>) => OnyxValue)) => void; + setWithOnyxState: (statePropertyName: OnyxKey, value: OnyxValue) => void; +}; + +export type {WithOnyxMapping, MapOnyxToState, WithOnyxProps, WithOnyxInstance, WithOnyxState}; From f59ea2ee777f6f9ceb107ce7b80d2511b4fd681e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Thu, 16 May 2024 19:09:25 +0100 Subject: [PATCH 12/14] Update docs --- API-INTERNAL.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/API-INTERNAL.md b/API-INTERNAL.md index aae82ccc7..db3e45f5f 100644 --- a/API-INTERNAL.md +++ b/API-INTERNAL.md @@ -115,7 +115,8 @@ whatever it is we attempted to do.

removeNullValues() ⇒

Removes a key from storage if the value is null. -Otherwise removes all nested null values in objects and returns the object

+Otherwise removes all nested null values in objects, +if shouldRemoveNestedNulls is true and returns the object.

prepareKeyValuePairsForStorage() ⇒

Storage expects array like: [["@MyApp_user", value_1], ["@MyApp_key", value_2]] @@ -367,7 +368,8 @@ Notifies subscribers and writes current value to cache ## removeNullValues() ⇒ Removes a key from storage if the value is null. -Otherwise removes all nested null values in objects and returns the object +Otherwise removes all nested null values in objects, +if shouldRemoveNestedNulls is true and returns the object. **Kind**: global function **Returns**: The value without null values and a boolean "wasRemoved", which indicates if the key got removed completely From bb3c09d49673372e6ce5679484ece1a688e0e432 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Thu, 16 May 2024 19:15:40 +0100 Subject: [PATCH 13/14] Type improvement --- lib/withOnyx/index.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/withOnyx/index.tsx b/lib/withOnyx/index.tsx index b6c860791..77f517055 100644 --- a/lib/withOnyx/index.tsx +++ b/lib/withOnyx/index.tsx @@ -335,7 +335,7 @@ export default function ( render() { // Remove any null values so that React replaces them with default props - const propsToPass = utils.omit(this.props as Omit, (entry) => entry[1] === null); + const propsToPass = utils.omit(this.props as Omit, ([, propValue]) => propValue === null); if (this.state.loading) { return null; @@ -343,7 +343,7 @@ export default function ( // Remove any internal state properties used by withOnyx // that should not be passed to a wrapped component - const stateToPass = utils.omit(this.state as WithOnyxState, (entry) => entry[0] === 'loading' || entry[1] === null); + const stateToPass = utils.omit(this.state as WithOnyxState, ([stateKey, stateValue]) => stateKey === 'loading' || stateValue === null); const stateToPassWithoutNestedNulls = utils.removeNestedNullValues(stateToPass); // Spreading props and state is necessary in an HOC where the data cannot be predicted From dbdb6753d21ac9b1a18dd95297728cb37d2de94a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Fri, 17 May 2024 14:08:35 +0100 Subject: [PATCH 14/14] Address review comment --- lib/withOnyx/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/withOnyx/index.tsx b/lib/withOnyx/index.tsx index 77f517055..3245bc8d4 100644 --- a/lib/withOnyx/index.tsx +++ b/lib/withOnyx/index.tsx @@ -276,7 +276,7 @@ export default function ( return; } - const canEvict = Str.result(mapping.canEvict as GenericFunction, this.props) as boolean; + const canEvict = !!Str.result(mapping.canEvict as GenericFunction, this.props); const key = Str.result(mapping.key as GenericFunction, this.props); if (!OnyxUtils.isSafeEvictionKey(key)) {