Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[No QA] Clarify Onyx method usage in View #5999

Merged
merged 3 commits into from
Oct 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,21 +64,21 @@ You can use any IDE or code editing tool for developing on any platform. Use you
**Note:** Expensify engineers that will be testing with the API in your local dev environment please refer to [these additional instructions](https://stackoverflow.com/c/expensify/questions/7699/7700).

## Environment variables
Creating an `.env` file is not necessary. We advise external contributors against it. It can lead to errors when
Creating an `.env` file is not necessary. We advise external contributors against it. It can lead to errors when
variables referenced here get updated since your local `.env` file is ignored.

- `EXPENSIFY_URL_CASH` - The root URL used for the website
- `EXPENSIFY_URL_SECURE` - The URL used to hit the Expensify secure API
- `EXPENSIFY_URL_COM` - The URL used to hit the Expensify API
- `EXPENSIFY_PARTNER_NAME` - Constant used for the app when authenticating.
- `EXPENSIFY_PARTNER_PASSWORD` - Another constant used for the app when authenticating. (This is OK to be public)
- `EXPENSIFY_PARTNER_PASSWORD` - Another constant used for the app when authenticating. (This is OK to be public)
- `PUSHER_APP_KEY` - Key used to authenticate with Pusher.com
- `SECURE_NGROK_URL` - Secure URL used for `ngrok` when testing
- `NGROK_URL` - URL used for `ngrok` when testing
- `USE_NGROK` - Flag to turn `ngrok` testing on or off
- `USE_WDYR` - Flag to turn [`Why Did You Render`](https://github.com/welldone-software/why-did-you-render) testing on or off
- `USE_WEB_PROXY`⚠️- Used in web/desktop development, it starts a server along the local development server to proxy
requests to the backend. External contributors should set this to `true` otherwise they'll have CORS errors.
requests to the backend. External contributors should set this to `true` otherwise they'll have CORS errors.
If you don't want to start the proxy server set this explicitly to `false`
- `CAPTURE_METRICS` (optional) - Set this to `true` to capture performance metrics and see them in Flipper
see [PERFORMANCE.md](PERFORMANCE.md#performance-metrics-opt-in-on-local-release-builds) for more information
Expand Down 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 @@ -250,6 +252,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