Skip to content

Commit

Permalink
Merge pull request #5999 from Expensify/marcaaron-onyxUsagesInViews
Browse files Browse the repository at this point in the history
Clarify Onyx method usage in View
  • Loading branch information
marcaaron committed Oct 23, 2021
2 parents b9d7e2f + bf86d61 commit 2a2e791
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 8 deletions.
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,8 @@ This layer is solely responsible for:
- Reflecting exactly the data that is in persistent storage by using `withOnyx()` to bind to Onyx data.
- Taking user input and passing it to an action
**Note:** As a convention, the UI layer should never interact with device storage directly or call `Onyx.set()` or `Onyx.merge()`. Use an action! For example, check out this action that is signing in the user [here](https://github.com/Expensify/App/blob/919c890cc391ad38b670ca1b266c114c8b3c3285/src/pages/signin/PasswordForm.js#L78-L78). That action will then call `Onyx.merge()` to [set default data and a loading state, then make an API request, and set the response with another `Onyx.merge()`](https://github.com/Expensify/App/blob/919c890cc391ad38b670ca1b266c114c8b3c3285/src/libs/actions/Session.js#L228-L247). Keeping our `Onyx.merge()` out of the view layer and in actions helps organize things as all interactions with device storage and API handling happen in the same place.
## Directory structure
Almost all the code is located in the `src` folder, inside it there's some organization, we chose to name directories that are
created to house a collection of items in plural form and using camelCase (eg: pages, libs, etc), the main ones we have for now are:
Expand Down Expand Up @@ -252,6 +254,7 @@ This application is built with the following principles.
- When data needs to be written to or read from the server, this is done through Actions only.
- Public action methods should never return anything (not data or a promise). This is done to ensure that action methods can be called in parallel with no dependency on other methods (see discussion above).
- Actions should favor using `Onyx.merge()` over `Onyx.set()` so that other values in an object aren't completely overwritten.
- Views should not call `Onyx.merge()` or `Onyx.set()` directly and should call an action instead.
- In general, the operations that happen inside an action should be done in parallel and not in sequence (eg. don't use the promise of one Onyx method to trigger a second Onyx method). Onyx is built so that every operation is done in parallel and it doesn't matter what order they finish in. XHRs on the other hand need to be handled in sequence with promise chains in order to access and act upon the response.
- If an Action needs to access data stored on disk, use a local variable and `Onyx.connect()`
- Data should be optimistically stored on disk whenever possible without waiting for a server response. Example of creating a new optimistic comment:
Expand Down
5 changes: 5 additions & 0 deletions src/libs/actions/Session.js
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,10 @@ function cleanupSession() {
Timers.clearAll();
}

function clearAccountMessages() {
Onyx.merge(ONYXKEYS.ACCOUNT, {error: '', success: ''});
}

export {
continueSessionFromECom,
fetchAccountDetails,
Expand All @@ -382,4 +386,5 @@ export {
resetPassword,
clearSignInData,
cleanupSession,
clearAccountMessages,
};
5 changes: 5 additions & 0 deletions src/libs/actions/User.js
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,10 @@ function setShouldUseSecureStaging(shouldUseSecureStaging) {
Onyx.merge(ONYXKEYS.USER, {shouldUseSecureStaging});
}

function clearUserErrorMessage() {
Onyx.merge(ONYXKEYS.USER, {error: ''});
}

export {
changePassword,
getBetas,
Expand All @@ -307,4 +311,5 @@ export {
subscribeToUserEvents,
setPreferredSkinTone,
setShouldUseSecureStaging,
clearUserErrorMessage,
};
6 changes: 3 additions & 3 deletions src/pages/settings/AddSecondaryLoginPage.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React, {Component} from 'react';
import Onyx, {withOnyx} from 'react-native-onyx';
import {withOnyx} from 'react-native-onyx';
import PropTypes from 'prop-types';
import {View, ScrollView} from 'react-native';
import _ from 'underscore';
Expand All @@ -9,7 +9,7 @@ import Navigation from '../../libs/Navigation/Navigation';
import ScreenWrapper from '../../components/ScreenWrapper';
import Text from '../../components/Text';
import styles from '../../styles/styles';
import {setSecondaryLogin} from '../../libs/actions/User';
import {clearUserErrorMessage, setSecondaryLogin} from '../../libs/actions/User';
import ONYXKEYS from '../../ONYXKEYS';
import Button from '../../components/Button';
import ROUTES from '../../ROUTES';
Expand Down Expand Up @@ -59,7 +59,7 @@ class AddSecondaryLoginPage extends Component {
}

componentWillUnmount() {
Onyx.merge(ONYXKEYS.USER, {error: ''});
clearUserErrorMessage();
}

onSecondaryLoginChange(login) {
Expand Down
5 changes: 3 additions & 2 deletions src/pages/settings/PasswordPage.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React, {Component} from 'react';
import {View, ScrollView} from 'react-native';
import Onyx, {withOnyx} from 'react-native-onyx';
import {withOnyx} from 'react-native-onyx';
import PropTypes from 'prop-types';
import {isEmpty} from 'underscore';

Expand All @@ -20,6 +20,7 @@ import KeyboardAvoidingView from '../../components/KeyboardAvoidingView';
import FixedFooter from '../../components/FixedFooter';
import ExpensiTextInput from '../../components/ExpensiTextInput';
import InlineErrorText from '../../components/InlineErrorText';
import {clearAccountMessages} from '../../libs/actions/Session';

const propTypes = {
/* Onyx Props */
Expand Down Expand Up @@ -59,7 +60,7 @@ class PasswordPage extends Component {
}

componentWillUnmount() {
Onyx.merge(ONYXKEYS.ACCOUNT, {error: '', success: ''});
clearAccountMessages();
}

onBlurNewPassword() {
Expand Down
6 changes: 3 additions & 3 deletions src/pages/signin/LoginForm.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import React from 'react';
import {View} from 'react-native';
import Onyx, {withOnyx} from 'react-native-onyx';
import {withOnyx} from 'react-native-onyx';
import PropTypes from 'prop-types';
import _ from 'underscore';
import Str from 'expensify-common/lib/str';
import styles from '../../styles/styles';
import Button from '../../components/Button';
import Text from '../../components/Text';
import {fetchAccountDetails} from '../../libs/actions/Session';
import {clearAccountMessages, fetchAccountDetails} from '../../libs/actions/Session';
import ONYXKEYS from '../../ONYXKEYS';
import withWindowDimensions, {windowDimensionsPropTypes} from '../../components/withWindowDimensions';
import compose from '../../libs/compose';
Expand Down Expand Up @@ -66,7 +66,7 @@ class LoginForm extends React.Component {
});

if (this.props.account.error) {
Onyx.merge(ONYXKEYS.ACCOUNT, {error: ''});
clearAccountMessages();
}
}

Expand Down

0 comments on commit 2a2e791

Please sign in to comment.