diff --git a/.gitignore b/.gitignore index 41f9bece4..4ee70c35e 100644 --- a/.gitignore +++ b/.gitignore @@ -11,6 +11,7 @@ dist/ # IDEs .idea +.vscode # Decrypted private key we do not want to commit .github/OSBotify-private-key.asc diff --git a/lib/withOnyx.js b/lib/withOnyx.js index 976152848..58e2a87e6 100644 --- a/lib/withOnyx.js +++ b/lib/withOnyx.js @@ -20,6 +20,15 @@ function getDisplayName(component) { 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} + */ +const getOnyxDataFromState = (state, onyxToStateMapping) => _.pick(state, _.keys(onyxToStateMapping)); + export default function (mapOnyxToState, shouldDelayUpdates = false) { // A list of keys that must be present in tempState before we can render the WrappedComponent const requiredKeysForInit = _.chain(mapOnyxToState) @@ -92,16 +101,20 @@ export default function (mapOnyxToState, shouldDelayUpdates = false) { this.checkEvictableKeys(); } - componentDidUpdate(prevProps) { + componentDidUpdate() { + // When the state is passed to the key functions with Str.result(), omit anything + // from state that was not part of the mapped keys. + const onyxDataFromState = getOnyxDataFromState(this.state, mapOnyxToState); + // If any of the mappings use data from the props, then when the props change, all the // connections need to be reconnected with the new props - _.each(mapOnyxToState, (mapping, propertyName) => { - const previousKey = Str.result(mapping.key, prevProps); - const newKey = Str.result(mapping.key, this.props); + _.each(mapOnyxToState, (mapping, propName) => { + const previousKey = mapping.previousKey; + const newKey = Str.result(mapping.key, {...this.props, ...onyxDataFromState}); if (previousKey !== newKey) { Onyx.disconnect(this.activeConnectionIDs[previousKey], previousKey); delete this.activeConnectionIDs[previousKey]; - this.connectMappingToOnyx(mapping, propertyName); + this.connectMappingToOnyx(mapping, propName); } }); this.checkEvictableKeys(); @@ -238,7 +251,12 @@ export default function (mapOnyxToState, shouldDelayUpdates = false) { * component */ connectMappingToOnyx(mapping, statePropertyName) { - const key = Str.result(mapping.key, this.props); + const key = Str.result(mapping.key, {...this.props, ...getOnyxDataFromState(this.state, mapOnyxToState)}); + + // Remember the previous key so that if it ever changes, the component will reconnect to Onyx + // in componentDidUpdate + // eslint-disable-next-line no-param-reassign + mapOnyxToState[statePropertyName].previousKey = key; // eslint-disable-next-line rulesdir/prefer-onyx-connect-in-libs this.activeConnectionIDs[key] = Onyx.connect({ diff --git a/tests/unit/withOnyxTest.js b/tests/unit/withOnyxTest.js index 1f36cfe09..c6ee21406 100644 --- a/tests/unit/withOnyxTest.js +++ b/tests/unit/withOnyxTest.js @@ -5,14 +5,16 @@ import Onyx, {withOnyx} from '../../lib'; import ViewWithText from '../components/ViewWithText'; import ViewWithCollections from '../components/ViewWithCollections'; import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; -import compose from '../../lib/compose'; import ViewWithObject from '../components/ViewWithObject'; const ONYX_KEYS = { TEST_KEY: 'test', COLLECTION: { TEST_KEY: 'test_', - RELATED_KEY: 'related_', + STATIC: 'static_', + DEPENDS_ON_STATIC: 'dependsOnStatic_', + DEPENDS_ON_DEPENDS_ON_STATIC: 'dependsOnDependsOnStatic_', + DEPENDS_ON_DEPENDS_ON_DEPENDS_ON_STATIC: 'dependsOnDependsOnDependsOnStatic_', }, SIMPLE_KEY: 'simple', SIMPLE_KEY_2: 'simple2', @@ -243,30 +245,65 @@ describe('withOnyxTest', () => { }); it('should pass a prop from one connected component to another', () => { - const collectionItemID = 1; const onRender = jest.fn(); const markReadyForHydration = jest.fn(); - Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, {test_1: {id: 1}}); - Onyx.mergeCollection(ONYX_KEYS.COLLECTION.RELATED_KEY, {related_1: 'Test'}); + + // Given three collections with multiple items in each + Onyx.mergeCollection(ONYX_KEYS.COLLECTION.STATIC, { + static_1: {name: 'Static 1', id: 1}, + static_2: {name: 'Static 2', id: 2}, + }); + + // And one collection will depends on data being loaded from the static collection + Onyx.mergeCollection(ONYX_KEYS.COLLECTION.DEPENDS_ON_STATIC, { + dependsOnStatic_1: {name: 'dependsOnStatic 1', id: 3}, + dependsOnStatic_2: {name: 'dependsOnStatic 2', id: 4}, + }); + + // And one collection will depend on the data being loaded from the collection that depends on the static collection (multiple nested dependencies) + Onyx.mergeCollection(ONYX_KEYS.COLLECTION.DEPENDS_ON_DEPENDS_ON_STATIC, { + dependsOnDependsOnStatic_3: {name: 'dependsOnDependsOnStatic 1', id: 5}, + dependsOnDependsOnStatic_4: {name: 'dependsOnDependsOnStatic 2', id: 6}, + }); + + // And another collection with one more layer of dependency just to prove it works + Onyx.mergeCollection(ONYX_KEYS.COLLECTION.DEPENDS_ON_DEPENDS_ON_DEPENDS_ON_STATIC, { + dependsOnDependsOnDependsOnStatic_5: {name: 'dependsOnDependsOnDependsOnStatic 1'}, + dependsOnDependsOnDependsOnStatic_6: {name: 'dependsOnDependsOnDependsOnStatic 2'}, + }); + + // When a component is rendered using withOnyx and several nested dependencies on the keys return waitForPromisesToResolve() .then(() => { - const TestComponentWithOnyx = compose( - withOnyx({ - testObject: { - key: `${ONYX_KEYS.COLLECTION.TEST_KEY}${collectionItemID}`, - }, - }), - withOnyx({ - testThing: { - key: ({testObject}) => `${ONYX_KEYS.COLLECTION.RELATED_KEY}${testObject.id}`, - }, - }), - )(ViewWithCollections); + const TestComponentWithOnyx = withOnyx({ + staticObject: { + key: `${ONYX_KEYS.COLLECTION.STATIC}1`, + }, + dependentObject: { + key: ({staticObject}) => `${ONYX_KEYS.COLLECTION.DEPENDS_ON_STATIC}${(staticObject && staticObject.id) || 0}`, + }, + multiDependentObject: { + key: ({dependentObject}) => `${ONYX_KEYS.COLLECTION.DEPENDS_ON_DEPENDS_ON_STATIC}${(dependentObject && dependentObject.id) || 0}`, + }, + extremeMultiDependentObject: { + key: ({multiDependentObject}) => `${ONYX_KEYS.COLLECTION.DEPENDS_ON_DEPENDS_ON_DEPENDS_ON_STATIC}${(multiDependentObject && multiDependentObject.id) || 0}`, + }, + })(ViewWithCollections); render(); + return waitForPromisesToResolve(); }) + + // Then all of the data gets properly loaded into the component as expected with the nested dependencies resolved .then(() => { expect(onRender).toHaveBeenLastCalledWith({ - collections: {}, markReadyForHydration, onRender, testObject: {id: 1}, testThing: 'Test', + markReadyForHydration, + onRender, + collections: {}, + testObject: {isDefaultProp: true}, + staticObject: {name: 'Static 1', id: 1}, + dependentObject: {name: 'dependsOnStatic 1', id: 3}, + multiDependentObject: {name: 'dependsOnDependsOnStatic 1', id: 5}, + extremeMultiDependentObject: {name: 'dependsOnDependsOnDependsOnStatic 1'}, }); }); });