Skip to content

Commit

Permalink
fix merge conflicts
Browse files Browse the repository at this point in the history
  • Loading branch information
akinwale committed Jun 5, 2023
2 parents 3ec8704 + f8a0a34 commit 2defd1d
Show file tree
Hide file tree
Showing 93 changed files with 940 additions and 501 deletions.
1 change: 1 addition & 0 deletions .github/actionlint.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@
self-hosted-runner:
labels:
- ubuntu-20.04-64core
- macos-12-xl
2 changes: 1 addition & 1 deletion .github/workflows/platformDeploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ jobs:
name: Build and deploy iOS
needs: validateActor
if: ${{ fromJSON(needs.validateActor.outputs.IS_DEPLOYER) }}
runs-on: macos-12
runs-on: macos-12-xl
steps:
# This action checks-out the repository, so the workflow can access it.
- uses: actions/checkout@93ea575cb5d8a053eaa0ac8fa3b40d7e05a33cc8
Expand Down
10 changes: 10 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,16 @@ jobs:

- name: Jest tests
run: npx jest --silent --shard=${{ fromJSON(matrix.chunk) }}/${{ strategy.job-total }} --max-workers ${{ steps.cpu-cores.outputs.count }}

storybookTests:
if: ${{ github.actor != 'OSBotify' || github.event_name == 'workflow_call' }}
runs-on: ubuntu-latest
name: Storybook tests
steps:
- uses: actions/checkout@885641592076c27bfb56c028cd5612cdad63e16d
- uses: Expensify/App/.github/actions/composite/setupNode@main
- name: Storybook run
run: npm run storybook -- --smoke-test --ci

shellTests:
if: ${{ github.actor != 'OSBotify' || github.event_name == 'workflow_call' }}
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/testBuild.yml
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ jobs:
if: ${{ fromJSON(needs.validateActor.outputs.READY_TO_BUILD) }}
env:
PULL_REQUEST_NUMBER: ${{ github.event.number || github.event.inputs.PULL_REQUEST_NUMBER }}
runs-on: macos-12
runs-on: macos-12-xl
steps:
# This action checks-out the repository, so the workflow can access it.
- uses: actions/checkout@93ea575cb5d8a053eaa0ac8fa3b40d7e05a33cc8
Expand Down
2 changes: 1 addition & 1 deletion .storybook/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const path = require('path');
const dotenv = require('dotenv');
const _ = require('underscore');
const custom = require('../config/webpack/webpack.common')({
envFile: '../.env.production',
envFile: '.env.production',
});

const env = dotenv.config({path: path.resolve(__dirname, '../.env.staging')}).parsed;
Expand Down
4 changes: 2 additions & 2 deletions android/app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,8 @@ android {
minSdkVersion rootProject.ext.minSdkVersion
targetSdkVersion rootProject.ext.targetSdkVersion
multiDexEnabled rootProject.ext.multiDexEnabled
versionCode 1001032200
versionName "1.3.22-0"
versionCode 1001032400
versionName "1.3.24-0"
}

splits {
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
60 changes: 47 additions & 13 deletions contributingGuides/STYLE.md
Original file line number Diff line number Diff line change
Expand Up @@ -219,20 +219,24 @@ const {firstName, lastName} = state;
...

// Bad
const UserInfo = ({name, email}) => (
<View>
<Text>Name: {name}</Text>
<Text>Email: {email}</Text>
</View>
);
function UserInfo({name, email}) {
return (
<View>
<Text>Name: {name}</Text>
<Text>Email: {email}</Text>
</View>
);
}

// Good
const UserInfo = props => (
<View>
<Text>Name: {props.name}</Text>
<Text>Email: {props.email}</Text>
</View>
);
function UserInfo(props) {
return (
<View>
<Text>Name: {props.name}</Text>
<Text>Email: {props.email}</Text>
</View>
);
}
```
## Named vs Default Exports in ES6 - When to use what?
Expand Down Expand Up @@ -480,7 +484,7 @@ When writing a function component you must ALWAYS add a `displayName` property a
```javascript

const Avatar = (props) => {...};
function Avatar(props) {...};

Avatar.propTypes = propTypes;
Avatar.defaultProps = defaultProps;
Expand Down Expand Up @@ -535,6 +539,36 @@ There are several ways to use and declare refs and we prefer the [callback metho
We love React and learning about all the new features that are regularly being added to the API. However, we try to keep our organization's usage of React limited to the most stable set of features that React offers. We do this mainly for **consistency** and so our engineers don't have to spend extra time trying to figure out how everything is working. That said, if you aren't sure if we have adopted something please ask us first.
# React Hooks: Frequently Asked Questions
## Are Hooks a Replacement for HOCs or Render Props?
In most cases, a custom hook is a better pattern to use than an HOC or Render Prop. They are easier to create, understand, use and document. However, there might still be a case for a HOC e.g. if you have a component that abstracts some conditional rendering logic.
## Should I wrap all my inline functions with `useCallback()` or move them out of the component if they have no dependencies?
The answer depends on whether you need a stable reference for the function. If there are no dependencies, you could move the function out of the component. If there are dependencies, you could use `useCallback()` to ensure the reference updates only when the dependencies change. However, it's important to note that using `useCallback()` may have a performance penalty, although the trade-off is still debated. You might choose to do nothing at all if there is no obvious performance downside to declaring a function inline. It's recommended to follow the guidance in the [React documentation](https://react.dev/reference/react/useCallback#should-you-add-usecallback-everywhere) and add the optimization only if necessary. If it's not obvious why such an optimization (i.e. `useCallback()` or `useMemo()`) would be used, leave a code comment explaining the reasoning to aid reviewers and future contributors.
## Why does `useState()` sometimes get initialized with a function?
React saves the initial state once and ignores it on the next renders. However, if you pass the result of a function to `useState()` or call a function directly e.g. `useState(doExpensiveThings())` it will *still run on every render*. This can hurt performance depending on what work the function is doing. As an optimization, we can pass an initializer function instead of a value e.g. `useState(doExpensiveThings)` or `useState(() => doExpensiveThings())`.
## Is there an equivalent to `componentDidUpdate()` when using hooks?
The short answer is no. A longer answer is that sometimes we need to check not only that a dependency has changed, but how it has changed in order to run a side effect. For example, a prop had a value of an empty string on a previous render, but now is non-empty. The generally accepted practice is to store the "previous" value in a `ref` so the comparison can be made in a `useEffect()` call.
## Are `useCallback()` and `useMemo()` basically the same thing?
No! It is easy to confuse `useCallback()` with a memoization helper like `_.memoize()` or `useMemo()` but they are really not the same at all. [`useCallback()` will return a cached function _definition_](https://react.dev/reference/react/useCallback) and will not save us any computational cost of running that function. So, if you are wrapping something in a `useCallback()` and then calling it in the render then it is better to use `useMemo()` to cache the actual **result** of calling that function and use it directly in the render.
## What is the `exhaustive-deps` lint rule? Can I ignore it?
A `useEffect()` that does not include referenced props or state in its dependency array is [usually a mistake](https://legacy.reactjs.org/docs/hooks-faq.html#is-it-safe-to-omit-functions-from-the-list-of-dependencies) as often we want effects to re-run when those dependencies change. However, there are some cases where we might actually only want to re-run the effect when only some of those dependencies change. We determined the best practice here should be to allow disabling the “next line” with a comment `//eslint-disable-next-line react-hooks/exhaustive-deps` and an additional comment explanation so the next developer can understand why the rule was not used.

## Should I declare my components with arrow functions (`const`) or the `function` keyword?

There are pros and cons of each, but ultimately we have standardized on using the `function` keyword to align things more with modern React conventions. There are also some minor cognitive overhead benefits in that you don't need to think about adding and removing brackets when encountering an implicit return. The `function` syntax also has the benefit of being able to be hoisted where arrow functions do not.
# Onyx Best Practices
[Onyx Documentation](https://github.com/expensify/react-native-onyx)
Expand Down
4 changes: 2 additions & 2 deletions ios/NewExpensify/Info.plist
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
<key>CFBundlePackageType</key>
<string>APPL</string>
<key>CFBundleShortVersionString</key>
<string>1.3.22</string>
<string>1.3.24</string>
<key>CFBundleSignature</key>
<string>????</string>
<key>CFBundleURLTypes</key>
Expand All @@ -30,7 +30,7 @@
</dict>
</array>
<key>CFBundleVersion</key>
<string>1.3.22.0</string>
<string>1.3.24.0</string>
<key>ITSAppUsesNonExemptEncryption</key>
<false/>
<key>LSApplicationQueriesSchemes</key>
Expand Down
4 changes: 2 additions & 2 deletions ios/NewExpensifyTests/Info.plist
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@
<key>CFBundlePackageType</key>
<string>BNDL</string>
<key>CFBundleShortVersionString</key>
<string>1.3.22</string>
<string>1.3.24</string>
<key>CFBundleSignature</key>
<string>????</string>
<key>CFBundleVersion</key>
<string>1.3.22.0</string>
<string>1.3.24.0</string>
</dict>
</plist>
2 changes: 1 addition & 1 deletion ios/Podfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1135,4 +1135,4 @@ SPEC CHECKSUMS:

PODFILE CHECKSUM: 4ed1c7b099741c82e2b0411b95f6468e72be6c76

COCOAPODS: 1.12.1
COCOAPODS: 1.12.0
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "new.expensify",
"version": "1.3.22-0",
"version": "1.3.24-0",
"author": "Expensify, Inc.",
"homepage": "https://new.expensify.com",
"description": "New Expensify is the next generation of Expensify: a reimagination of payments based atop a foundation of chat.",
Expand Down
7 changes: 7 additions & 0 deletions src/CONST.js
Original file line number Diff line number Diff line change
Expand Up @@ -796,6 +796,7 @@ const CONST = {
EMAIL_ADDRESS: 'email-address',
ASCII_CAPABLE: 'ascii-capable',
URL: 'url',
DEFAULT: 'default',
},

ATTACHMENT_MESSAGE_TEXT: '[Attachment]',
Expand Down Expand Up @@ -2443,6 +2444,12 @@ const CONST = {
FLAG_SEVERITY_HARASSMENT: 'harassment',
FLAG_SEVERITY_ASSAULT: 'assault',
},
QR: {
DEFAULT_LOGO_SIZE_RATIO: 0.25,
DEFAULT_LOGO_MARGIN_RATIO: 0.02,
EXPENSIFY_LOGO_SIZE_RATIO: 0.22,
EXPENSIFY_LOGO_MARGIN_RATIO: 0.03,
},
};

export default CONST;
6 changes: 3 additions & 3 deletions src/ONYXKEYS.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ export default {
// Contains all the personalDetails the user has access to
PERSONAL_DETAILS: 'personalDetails',

// Contains all the personalDetails the user has access to, keyed by accountID
PERSONAL_DETAILS_LIST: 'personalDetailsList',

// Contains all the private personal details of the user
PRIVATE_PERSONAL_DETAILS: 'private_personalDetails',

Expand Down Expand Up @@ -76,9 +79,6 @@ export default {
BETAS: 'betas',

// NVP keys
// Contains the user's payPalMe address
NVP_PAYPAL_ME_ADDRESS: 'nvp_paypalMeAddress',

// Contains the user's payPalMe data
PAYPAL: 'paypal',

Expand Down
3 changes: 2 additions & 1 deletion src/components/AttachmentView.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ const propTypes = {
/** Function for handle on press */
onPress: PropTypes.func,

/** Handles scale changed event in PDF component */
/** Handles scale changed event */
onScaleChanged: PropTypes.func,

/** Notify parent that the UI should be modified to accommodate keyboard */
Expand Down Expand Up @@ -111,6 +111,7 @@ const AttachmentView = (props) => {
if (isImage || (props.file && Str.isImage(props.file.name))) {
const children = (
<ImageView
onScaleChanged={props.onScaleChanged}
url={props.source}
isAuthTokenRequired={isImage && props.isAuthTokenRequired}
/>
Expand Down
28 changes: 18 additions & 10 deletions src/components/Button/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,14 @@ const propTypes = {
/** The fill color to pass into the icon. */
iconFill: PropTypes.string,

/** Any additional styles to pass to the icon container. */
/** Any additional styles to pass to the left icon container. */
// eslint-disable-next-line react/forbid-prop-types
iconStyles: PropTypes.arrayOf(PropTypes.object),

/** Any additional styles to pass to the right icon container. */
// eslint-disable-next-line react/forbid-prop-types
iconRightStyles: PropTypes.arrayOf(PropTypes.object),

/** Small sized button */
small: PropTypes.bool,

Expand Down Expand Up @@ -122,6 +126,7 @@ const defaultProps = {
iconRight: Expensicons.ArrowRight,
iconFill: themeColors.textLight,
iconStyles: [],
iconRightStyles: [],
isLoading: false,
isDisabled: false,
small: false,
Expand Down Expand Up @@ -214,24 +219,27 @@ class Button extends Component {
</Text>
);

if (this.props.icon) {
if (this.props.icon || this.props.shouldShowRightIcon) {
return (
<View style={[styles.justifyContentBetween, styles.flexRow]}>
<View style={[styles.alignItemsCenter, styles.flexRow, styles.flexShrink1]}>
<View style={[styles.mr1, ...this.props.iconStyles]}>
<Icon
src={this.props.icon}
fill={this.props.iconFill}
small={this.props.small}
/>
</View>
{this.props.icon && (
<View style={[styles.mr1, ...this.props.iconStyles]}>
<Icon
src={this.props.icon}
fill={this.props.iconFill}
small={this.props.small}
/>
</View>
)}
{textComponent}
</View>
{this.props.shouldShowRightIcon && (
<View style={styles.justifyContentCenter}>
<View style={[styles.justifyContentCenter, styles.ml1, ...this.props.iconRightStyles]}>
<Icon
src={this.props.iconRight}
fill={this.props.iconFill}
small={this.props.small}
/>
</View>
)}
Expand Down
2 changes: 1 addition & 1 deletion src/components/CalendarPicker/calendarPickerPropTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import CONST from '../../CONST';

const propTypes = {
/** An initial value of date string */
value: PropTypes.string,
value: PropTypes.oneOfType([PropTypes.string, PropTypes.instanceOf(Date)]),

/** A minimum date (oldest) allowed to select */
minDate: PropTypes.objectOf(Date),
Expand Down
Loading

0 comments on commit 2defd1d

Please sign in to comment.