Skip to content

Commit

Permalink
Merge branch 'main' into feat/emojipicker-flashlist-migration
Browse files Browse the repository at this point in the history
  • Loading branch information
TMisiukiewicz committed Dec 21, 2023
2 parents 7c774f9 + 10dd12e commit 6b555d0
Show file tree
Hide file tree
Showing 229 changed files with 4,312 additions and 3,409 deletions.
10 changes: 10 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,21 @@ const restrictedImportPaths = [
importNames: ['TouchableOpacity', 'TouchableWithoutFeedback', 'TouchableNativeFeedback', 'TouchableHighlight'],
message: "Please use 'PressableWithFeedback' and/or 'PressableWithoutFeedback' from 'src/components/Pressable' instead.",
},
{
name: 'awesome-phonenumber',
importNames: ['parsePhoneNumber'],
message: "Please use '@libs/PhoneNumber' instead.",
},
{
name: 'react-native-safe-area-context',
importNames: ['useSafeAreaInsets', 'SafeAreaConsumer', 'SafeAreaInsetsContext'],
message: "Please use 'useSafeAreaInsets' from 'src/hooks/useSafeAreaInset' and/or 'SafeAreaConsumer' from 'src/components/SafeAreaConsumer' instead.",
},
{
name: 'react',
importNames: ['CSSProperties'],
message: "Please use 'ViewStyle', 'TextStyle', 'ImageStyle' from 'react-native' instead.",
},
];

const restrictedImportPatterns = [
Expand Down
3 changes: 3 additions & 0 deletions .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,9 @@ This is a checklist for PR authors. Please make sure to complete all tasks and c
- [ ] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like `Avatar` is modified, I verified that `Avatar` is working as expected in all cases)
- [ ] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
- [ ] If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
- [ ] If the PR modifies the form input styles:
- [ ] I verified that all the inputs inside a form are aligned with each other.
- [ ] I added `Design` label so the design team can review the changes.
- [ ] If a new page is added, I verified it's using the `ScrollView` component to make it scrollable when more elements are added to the page.
- [ ] If the `main` branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the `Test` steps.

Expand Down
4 changes: 2 additions & 2 deletions android/app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,8 @@ android {
minSdkVersion rootProject.ext.minSdkVersion
targetSdkVersion rootProject.ext.targetSdkVersion
multiDexEnabled rootProject.ext.multiDexEnabled
versionCode 1001041308
versionName "1.4.13-8"
versionCode 1001041504
versionName "1.4.15-4"
}

flavorDimensions "default"
Expand Down

This file was deleted.

Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
4 changes: 0 additions & 4 deletions android/app/src/adhoc/res/values/ic_launcher_background.xml

This file was deleted.

This file was deleted.

Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.

This file was deleted.

1 change: 0 additions & 1 deletion android/app/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
android:icon="@mipmap/ic_launcher"
android:label="@string/app_name"
android:resizeableActivity="false"
android:roundIcon="@mipmap/ic_launcher_round"
android:supportsRtl="false"
android:theme="@style/AppTheme"
tools:replace="android:supportsRtl">
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions android/app/src/main/res/mipmap-anydpi-v26/ic_launcher.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,5 @@
<adaptive-icon xmlns:android="http://schemas.android.com/apk/res/android">
<background android:drawable="@color/ic_launcher_background"/>
<foreground android:drawable="@drawable/ic_launcher_foreground"/>
<monochrome android:drawable="@drawable/ic_launcher_monochrome"/>
</adaptive-icon>

This file was deleted.

Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions contributingGuides/OFFLINE_UX.md
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ When the user is offline:
- `optimisticData` - always include this object when using the Pattern B
- `successData` - include this if the action is `update` or `delete`. You do not have to include this if the action is `add` (same data was already passed using the `optimisticData` object)
- `failureData` - always include this object. In case of `add` action, you will want to add some generic error which covers some unexpected failures which were not handled in the backend
- In the event that `successData` and `failureData` are the same, you can use a single object `finallyData` in place of both.

**Handling errors:**
- The [OfflineWithFeedback component](https://github.com/Expensify/App/blob/main/src/components/OfflineWithFeedback.js) already handles showing errors too, as long as you pass the error field in the [errors prop](https://github.com/Expensify/App/blob/128ea378f2e1418140325c02f0b894ee60a8e53f/src/components/OfflineWithFeedback.js#L29-L31)
Expand Down
3 changes: 3 additions & 0 deletions contributingGuides/REVIEWER_CHECKLIST.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@
- [ ] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like `Avatar` is modified, I verified that `Avatar` is working as expected in all cases)
- [ ] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
- [ ] If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
- [ ] If the PR modifies the form input styles:
- [ ] I verified that all the inputs inside a form are aligned with each other.
- [ ] I added `Design` label so the design team can review the changes.
- [ ] If a new page is added, I verified it's using the `ScrollView` component to make it scrollable when more elements are added to the page.
- [ ] If the `main` branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the `Test` steps.
- [ ] I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.
Expand Down
123 changes: 120 additions & 3 deletions contributingGuides/TS_STYLE.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
- [1.17 `.tsx`](#tsx)
- [1.18 No inline prop types](#no-inline-prop-types)
- [1.19 Satisfies operator](#satisfies-operator)
- [1.20 Hooks instead of HOCs](#hooks-instead-of-hocs)
- [1.21 `compose` usage](#compose-usage)
- [Exception to Rules](#exception-to-rules)
- [Communication Items](#communication-items)
- [Migration Guidelines](#migration-guidelines)
Expand Down Expand Up @@ -124,7 +126,7 @@ type Foo = {

<a name="d-ts-extension"></a><a name="1.2"></a>

- [1.2](#d-ts-extension) **`d.ts` Extension**: Do not use `d.ts` file extension even when a file contains only type declarations. Only exceptions are `src/types/global.d.ts` and `src/types/modules/*.d.ts` files in which third party packages can be modified using module augmentation. Refer to the [Communication Items](#communication-items) section to learn more about module augmentation.
- [1.2](#d-ts-extension) **`d.ts` Extension**: Do not use `d.ts` file extension even when a file contains only type declarations. Only exceptions are `src/types/global.d.ts` and `src/types/modules/*.d.ts` files in which third party packages and JavaScript's built-in modules (e.g. `window` object) can be modified using module augmentation. Refer to the [Communication Items](#communication-items) section to learn more about module augmentation.
> Why? Type errors in `d.ts` files are not checked by TypeScript [^1].
Expand Down Expand Up @@ -509,6 +511,102 @@ type Foo = {
} satisfies Record<string, ViewStyle>;
```

<a name="hooks-instead-of-hocs"></a><a name="1.20"></a>

- [1.20](#hooks-instead-of-hocs) **Hooks instead of HOCs**: Replace HOCs usage with Hooks whenever possible.

> Why? Hooks are easier to use (can be used inside the function component), and don't need nesting or `compose` when exporting the component. It also allows us to remove `compose` completely in some components since it has been bringing up some issues with TypeScript. Read the [`compose` usage](#compose-usage) section for further information about the TypeScript issues with `compose`.
> Note: Because Onyx doesn't provide a hook yet, in a component that accesses Onyx data with `withOnyx` HOC, please make sure that you don't use other HOCs (if applicable) to avoid HOC nesting.
```tsx
// BAD
type ComponentOnyxProps = {
session: OnyxEntry<Session>;
};

type ComponentProps = WindowDimensionsProps &
WithLocalizeProps &
ComponentOnyxProps & {
someProp: string;
};

function Component({windowWidth, windowHeight, translate, session, someProp}: ComponentProps) {
// component's code
}

export default compose(
withWindowDimensions,
withLocalize,
withOnyx<ComponentProps, ComponentOnyxProps>({
session: {
key: ONYXKEYS.SESSION,
},
}),
)(Component);

// GOOD
type ComponentOnyxProps = {
session: OnyxEntry<Session>;
};

type ComponentProps = ComponentOnyxProps & {
someProp: string;
};

function Component({session, someProp}: ComponentProps) {
const {windowWidth, windowHeight} = useWindowDimensions();
const {translate} = useLocalize();
// component's code
}

// There is no hook alternative for withOnyx yet.
export default withOnyx<ComponentProps, ComponentOnyxProps>({
session: {
key: ONYXKEYS.SESSION,
},
})(Component);
```

<a name="compose-usage"></a><a name="1.21"></a>

- [1.21](#compose-usage) **`compose` usage**: Avoid the usage of `compose` function to compose HOCs in TypeScript files. Use nesting instead.

> Why? `compose` function doesn't work well with TypeScript when dealing with several HOCs being used in a component, many times resulting in wrong types and errors. Instead, nesting can be used to allow a seamless use of multiple HOCs and result in a correct return type of the compoment. Also, you can use [hooks instead of HOCs](#hooks-instead-of-hocs) whenever possible to minimize or even remove the need of HOCs in the component.
```ts
// BAD
export default compose(
withCurrentUserPersonalDetails,
withReportOrNotFound(),
withOnyx<ComponentProps, ComponentOnyxProps>({
session: {
key: ONYXKEYS.SESSION,
},
}),
)(Component);

// GOOD
export default withCurrentUserPersonalDetails(
withReportOrNotFound()(
withOnyx<ComponentProps, ComponentOnyxProps>({
session: {
key: ONYXKEYS.SESSION,
},
})(Component),
),
);

// GOOD - alternative to HOC nesting
const ComponentWithOnyx = withOnyx<ComponentProps, ComponentOnyxProps>({
session: {
key: ONYXKEYS.SESSION,
},
})(Component);
const ComponentWithReportOrNotFound = withReportOrNotFound()(ComponentWithOnyx);
export default withCurrentUserPersonalDetails(ComponentWithReportOrNotFound);
```

## Exception to Rules

Most of the rules are enforced in ESLint or checked by TypeScript. If you think your particular situation warrants an exception, post the context in the `#expensify-open-source` Slack channel with your message prefixed with `TS EXCEPTION:`. The internal engineer assigned to the PR should be the one that approves each exception, however all discussion regarding granting exceptions should happen in the public channel instead of the GitHub PR page so that the TS migration team can access them easily.
Expand All @@ -521,7 +619,7 @@ This rule will apply until the migration is done. After the migration, discussio

> Comment in the `#expensify-open-source` Slack channel if any of the following situations are encountered. Each comment should be prefixed with `TS ATTENTION:`. Internal engineers will access each situation and prescribe solutions to each case. Internal engineers should refer to general solutions to each situation that follows each list item.
- I think types definitions in a third party library is incomplete or incorrect
- I think types definitions in a third party library or JavaScript's built-in module are incomplete or incorrect

When the library indeed contains incorrect or missing type definitions and it cannot be updated, use module augmentation to correct them. All module augmentation code should be contained in `/src/types/modules/*.d.ts`, each library as a separate file.

Expand All @@ -540,7 +638,7 @@ declare module "external-library-name" {

> This section contains instructions that are applicable during the migration.
- 🚨 DO NOT write new code in TypeScript yet. The only time you write TypeScript code is when the file you're editing has already been migrated to TypeScript by the migration team. This guideline will be updated once it's time for new code to be written in TypeScript. If you're doing a major overhaul or refactoring of particular features or utilities of App and you believe it might be beneficial to migrate relevant code to TypeScript as part of the refactoring, please ask in the #expensify-open-source channel about it (and prefix your message with `TS ATTENTION:`).
- 🚨 DO NOT write new code in TypeScript yet. The only time you write TypeScript code is when the file you're editing has already been migrated to TypeScript by the migration team, or when you need to add new files under `src/libs`, `src/hooks`, `src/styles`, and `src/languages` directories. This guideline will be updated once it's time for new code to be written in TypeScript. If you're doing a major overhaul or refactoring of particular features or utilities of App and you believe it might be beneficial to migrate relevant code to TypeScript as part of the refactoring, please ask in the #expensify-open-source channel about it (and prefix your message with `TS ATTENTION:`).

- If you're migrating a module that doesn't have a default implementation (i.e. `index.ts`, e.g. `getPlatform`), convert `index.website.js` to `index.ts`. Without `index.ts`, TypeScript cannot get type information where the module is imported.

Expand Down Expand Up @@ -579,6 +677,25 @@ object?.foo ?? 'bar';
const y: number = 123; // TS error: Unused '@ts-expect-error' directive.
```

- The TS issue I'm working on is blocked by another TS issue because of type errors. What should I do?

In order to proceed with the migration faster, we are now allowing the use of `@ts-expect-error` annotation to temporally suppress those errors and help you unblock your issues. The only requirements is that you MUST add the annotation with a comment explaining that it must be removed when the blocking issue is migrated, e.g.:

```tsx
return (
<MenuItem
// @ts-expect-error TODO: Remove this once MenuItem (https://github.com/Expensify/App/issues/25144) is migrated to TypeScript.
wrapperStyle={styles.mr3}
key={text}
icon={icon}
title={text}
onPress={onPress}
/>
);
```

**You will also need to reference the blocking issue in your PR.** You can find all the TS issues [here](https://github.com/orgs/Expensify/projects/46).

## Learning Resources

### Quickest way to learn TypeScript
Expand Down
3 changes: 2 additions & 1 deletion docs/TEMPLATE.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ What options does a user have then interacting with this feature?
What elements of this feature are pay-walled vs. free?
-->

# FAQ
{% include faq-begin.md %}
<!--
This section covers the useful but not as vital information, it should capture commonly queried elements which do not organically form part of the About or How-to sections.
Expand All @@ -32,6 +32,7 @@ This section covers the useful but not as vital information, it should capture c
- Is there any general troubleshooting for this feature?
- Note: troubleshooting should generally go in the FAQ, but if there is extensive troubleshooting, such as with integrations, that will be housed in a separate page, stored with and linked from the main page for that feature.
-->
{% include faq-end.md %}

# Deep Dive
<!--
Expand Down
6 changes: 3 additions & 3 deletions docs/_data/_routes.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ platforms:

- href: billing-and-subscriptions
title: Billing & Subscriptions
icon: /assets/images/money-wings.svg
icon: /assets/images/subscription-annual.svg
description: Here is where you can review Expensify's billing and subscription options, plan types, and payment methods.

- href: expense-and-report-features
Expand Down Expand Up @@ -71,7 +71,7 @@ platforms:

- href: send-payments
title: Send Payments
icon: /assets/images/money-wings.svg
icon: /assets/images/send-money.svg
description: Uncover step-by-step guidance on sending direct reimbursements to employees, paying an invoice to a vendor, and utilizing third-party payment options.

- href: workspace-and-domain-settings
Expand Down Expand Up @@ -105,7 +105,7 @@ platforms:

- href: billing-and-plan-types
title: Billing & Plan Types
icon: /assets/images/money-wings.svg
icon: /assets/images/subscription-annual.svg
description: Here is where you can review Expensify's billing and subscription options, plan types, and payment methods.

- href: expensify-card
Expand Down
4 changes: 4 additions & 0 deletions docs/_includes/faq-begin.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{::options parse_block_html="true" /}
<details>
{::options parse_block_html="false" /}
<summary>FAQ</summary> {: #faq}
1 change: 1 addition & 0 deletions docs/_includes/faq-end.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
</details>
29 changes: 23 additions & 6 deletions docs/_sass/_main.scss
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ blockquote {

article,
aside,
details,
figcaption,
figure,
footer,
Expand All @@ -55,8 +54,7 @@ hgroup,
main,
menu,
nav,
section,
summary {
section {
display: block;
}

Expand Down Expand Up @@ -99,7 +97,8 @@ h2,
h3,
h4,
h5,
h6 {
h6,
summary {
color: $color-text;
font-weight: bold;
padding-bottom: 12px;
Expand All @@ -113,7 +112,25 @@ h6 {
margin-top: 20px;
}

h1 {
#faq::marker {
font-size: 1.5em;
}

details summary {
cursor: pointer;
user-select: none;
}

details > summary {
list-style-image: url("/assets/images/arrow-right.svg");
}

details[open] > summary {
list-style-image: url("/assets/images/down.svg");
}

h1,
summary {
font-family: "ExpensifyNewKansas", "Helvetica Neue", "Helvetica", Arial, sans-serif;
font-weight: 500;
font-size: larger;
Expand Down Expand Up @@ -398,7 +415,7 @@ button {
flex-wrap: wrap;
}

h1 {
h1, summary {
font-size: 1.5em;
padding: 20px 0 12px 0;
}
Expand Down
Loading

0 comments on commit 6b555d0

Please sign in to comment.