Skip to content

Commit

Permalink
Merge branch 'Expensify:main' into krishna2323/issue/35377
Browse files Browse the repository at this point in the history
  • Loading branch information
Krishna2323 authored Mar 19, 2024
2 parents 86e996f + 4976bdb commit 4272054
Show file tree
Hide file tree
Showing 853 changed files with 22,384 additions and 18,771 deletions.
8 changes: 7 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
const restrictedImportPaths = [
{
name: 'react-native',
importNames: ['useWindowDimensions', 'StatusBar', 'TouchableOpacity', 'TouchableWithoutFeedback', 'TouchableNativeFeedback', 'TouchableHighlight', 'Pressable', 'Text'],
importNames: ['useWindowDimensions', 'StatusBar', 'TouchableOpacity', 'TouchableWithoutFeedback', 'TouchableNativeFeedback', 'TouchableHighlight', 'Pressable', 'Text', 'ScrollView'],
message: [
'',
"For 'useWindowDimensions', please use 'src/hooks/useWindowDimensions' instead.",
"For 'TouchableOpacity', 'TouchableWithoutFeedback', 'TouchableNativeFeedback', 'TouchableHighlight', 'Pressable', please use 'PressableWithFeedback' and/or 'PressableWithoutFeedback' from 'src/components/Pressable' instead.",
"For 'StatusBar', please use 'src/libs/StatusBar' instead.",
"For 'Text', please use '@components/Text' instead.",
"For 'ScrollView', please use '@components/ScrollView' instead.",
].join('\n'),
},
{
Expand Down Expand Up @@ -50,6 +51,10 @@ const restrictedImportPaths = [
name: '@styles/theme/illustrations',
message: 'Do not import theme illustrations directly. Please use the `useThemeIllustrations` hook instead.',
},
{
name: 'date-fns/locale',
message: "Do not import 'date-fns/locale' directly. Please use the submodule import instead, like 'date-fns/locale/en-GB'.",
},
];

const restrictedImportPatterns = [
Expand Down Expand Up @@ -193,6 +198,7 @@ module.exports = {
{
selector: ['parameter', 'method'],
format: ['camelCase', 'PascalCase'],
leadingUnderscore: 'allow',
},
],
'@typescript-eslint/ban-types': [
Expand Down
6 changes: 3 additions & 3 deletions .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ This is a checklist for PR authors. Please make sure to complete all tasks and c
- [ ] I verified any copy / text shown in the product is localized by adding it to `src/languages/*` files and using the [translation method](https://github.com/Expensify/App/blob/4bd99402cebdf4d7394e0d1f260879ea238197eb/src/components/withLocalize.js#L60)
- [ ] If any non-english text was added/modified, I verified the translation was requested/reviewed in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:
- [ ] I verified all numbers, amounts, dates and phone numbers shown in the product are using the [localization methods](https://github.com/Expensify/App/blob/4bd99402cebdf4d7394e0d1f260879ea238197eb/src/components/withLocalize.js#L60-L68)
- [ ] I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the `Waiting for Copy` label for a copy review on the original GH to get the correct copy.
- [ ] I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
- [ ] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
- [ ] I verified the JSDocs style guidelines (in [`STYLE.md`](https://github.com/Expensify/App/blob/main/contributingGuides/STYLE.md#jsdocs)) were followed
- [ ] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
Expand All @@ -103,9 +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:
- [ ] If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or 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.
- [ ] I added `Design` label and/or tagged `@Expensify/design` 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
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,11 @@ const run = () => {
// Graphite doesn't accept metrics name with space, we replace spaces with "-"
const formattedName = current.name.split(' ').join('-');

const renderDurationString = `${GRAPHITE_PATH}.PR-${prNumber}.${formattedName}.renderDuration ${current.meanDuration} ${timestamp}`;
const renderCountString = `${GRAPHITE_PATH}.PR-${prNumber}.${formattedName}.renderCount ${current.meanCount} ${timestamp}`;
const renderDurationString = `${GRAPHITE_PATH}.${formattedName}.renderDuration ${current.meanDuration} ${timestamp}`;
const renderCountString = `${GRAPHITE_PATH}.${formattedName}.renderCount ${current.meanCount} ${timestamp}`;
const renderPRNumberString = `${GRAPHITE_PATH}.${formattedName}.prNumber ${prNumber} ${timestamp}`;

return `${renderDurationString}\n${renderCountString}`;
return `${renderDurationString}\n${renderCountString}\n${renderPRNumberString}`;
})
.join('\n');

Expand Down
7 changes: 4 additions & 3 deletions .github/actions/javascript/getGraphiteString/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,11 @@ const run = () => {
// Graphite doesn't accept metrics name with space, we replace spaces with "-"
const formattedName = current.name.split(' ').join('-');

const renderDurationString = `${GRAPHITE_PATH}.PR-${prNumber}.${formattedName}.renderDuration ${current.meanDuration} ${timestamp}`;
const renderCountString = `${GRAPHITE_PATH}.PR-${prNumber}.${formattedName}.renderCount ${current.meanCount} ${timestamp}`;
const renderDurationString = `${GRAPHITE_PATH}.${formattedName}.renderDuration ${current.meanDuration} ${timestamp}`;
const renderCountString = `${GRAPHITE_PATH}.${formattedName}.renderCount ${current.meanCount} ${timestamp}`;
const renderPRNumberString = `${GRAPHITE_PATH}.${formattedName}.prNumber ${prNumber} ${timestamp}`;

return `${renderDurationString}\n${renderCountString}`;
return `${renderDurationString}\n${renderCountString}\n${renderPRNumberString}`;
})
.join('\n');

Expand Down
6 changes: 1 addition & 5 deletions .github/workflows/e2ePerformanceTests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,6 @@ on:
type: string
required: true

concurrency:
group: "${{ github.ref }}-e2e"
cancel-in-progress: true

jobs:
buildBaseline:
runs-on: ubuntu-latest-xl
Expand Down Expand Up @@ -183,7 +179,7 @@ jobs:
run: npm run e2e-test-runner-build

- name: Copy e2e code into zip folder
run: cp tests/e2e/dist/index.js zip/testRunner.js
run: cp tests/e2e/dist/index.js zip/testRunner.ts

- name: Zip everything in the zip directory up
run: zip -qr App.zip ./zip
Expand Down
5 changes: 1 addition & 4 deletions .github/workflows/reassurePerformanceTests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,6 @@ jobs:
uses: ./.github/actions/javascript/getGraphiteString

- name: Send graphite data
env:
GRAPHITE_SERVER: ${{ vars.GRAPHITE_SERVER }}
GRAPHITE_PORT: ${{ vars.GRAPHITE_PORT }}
# run only when merged to main
if: github.event.pull_request.merged == true && github.event.pull_request.base.ref == 'main'
run: echo -e "${{ steps.saveGraphiteString.outputs.GRAPHITE_STRING }}" | nc -q0 "$GRAPHITE_SERVER" "$GRAPHITE_PORT"
run: echo -e "${{ steps.saveGraphiteString.outputs.GRAPHITE_STRING }}" | nc -q0 stats.expensify.com 3003
121 changes: 97 additions & 24 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ These instructions should get you set up ready to work on New Expensify 🙌
1. Install `nvm` then `node` & `npm`: `brew install nvm && nvm install`
2. Install `watchman`: `brew install watchman`
3. Install dependencies: `npm install`
4. Install `mkcert`: `brew install mkcert` followed by `npm run setup-https`. If you are not using macOS, follow the instructions [here](https://github.com/FiloSottile/mkcert?tab=readme-ov-file#installation).
4. Install `mkcert`: `brew install mkcert` followed by `npm run setup-https`. If you are not using macOS, follow the instructions [here](https://github.com/FiloSottile/mkcert?tab=readme-ov-file#installation).
5. Create a host entry in your local hosts file, `/etc/hosts` for dev.new.expensify.com pointing to localhost:
```
127.0.0.1 dev.new.expensify.com
Expand Down Expand Up @@ -86,7 +86,7 @@ If you want to run the app on an actual physical iOS device, please follow the i
1. If you are having issues with **_Getting Started_**, please reference [React Native's Documentation](https://reactnative.dev/docs/environment-setup)
2. If you are running into CORS errors like (in the browser dev console)
```sh
Access to fetch at 'https://www.expensify.com/api?command=BeginSignIn' from origin 'http://localhost:8080' has been blocked by CORS policy
Access to fetch at 'https://www.expensify.com/api/BeginSignIn' from origin 'http://localhost:8080' has been blocked by CORS policy
```
You probably have a misconfigured `.env` file - remove it (`rm .env`) and try again

Expand All @@ -113,7 +113,7 @@ variables referenced here get updated since your local `.env` file is ignored.
see [PERFORMANCE.md](contributingGuides/PERFORMANCE.md#performance-metrics-opt-in-on-local-release-builds) for more information
- `ONYX_METRICS` (optional) - Set this to `true` to capture even more performance metrics and see them in Flipper
see [React-Native-Onyx#benchmarks](https://github.com/Expensify/react-native-onyx#benchmarks) for more information
- `E2E_TESTING` (optional) - This needs to be set to `true` when running the e2e tests for performance regression testing.
- `E2E_TESTING` (optional) - This needs to be set to `true` when running the e2e tests for performance regression testing.
This happens usually automatically, read [this](tests/e2e/README.md) for more information

----
Expand All @@ -127,7 +127,7 @@ You create this certificate by following the instructions in [`Configuring HTTPS
#### Pre-requisite for Android flow
1. Open any emulator using Android Studio
2. Use `adb push "$(mkcert -CAROOT)/rootCA.pem" /storage/emulated/0/Download/` to push certificate to install in Download folder.
3. Install the certificate as CA certificate from the settings. On the Android emulator, this option can be found in Settings > Security > Encryption & Credentials > Install a certificate > CA certificate.
3. Install the certificate as CA certificate from the settings. On the Android emulator, this option can be found in Settings > Security > Encryption & Credentials > Install a certificate > CA certificate.
4. Close the emulator.

Note - If you want to run app on `https://127.0.0.1:8082`, then just install the certificate and use `adb reverse tcp:8082 tcp:8082` on every startup.
Expand Down Expand Up @@ -187,6 +187,79 @@ Our React Native Android app now uses the `Hermes` JS engine which requires your
To make it easier to test things in web, we expose the Onyx object to the window, so you can easily do `Onyx.set('bla', 1)`.
----
# Release Profiler
Often, performance issue debugging occurs in debug builds, which can introduce errors from elements such as JS Garbage Collection, Hermes debug markers, or LLDB pauses.
`react-native-release-profiler` facilitates profiling within release builds for accurate local problem-solving and broad performance analysis in production to spot regressions or collect extensive device data. Therefore, we will utilize the production build version
### Getting Started with Source Maps
To accurately profile your application, generating source maps for Android and iOS is crucial. Here's how to enable them:
1. Enable source maps on Android
Ensure the following is set in your app's `android/app/build.gradle` file.
```jsx
project.ext.react = [
enableHermes: true,
hermesFlagsRelease: ["-O", "-output-source-map"], // <-- here, plus whichever flag was required to set this away from default
]
```
2. Enable source maps on IOS
Within Xcode head to the build phase - `Bundle React Native code and images`.
```jsx
export SOURCEMAP_FILE="$(pwd)/../main.jsbundle.map" // <-- here;
export NODE_BINARY=node
../node_modules/react-native/scripts/react-native-xcode.sh
```
3. Install the necessary packages and CocoaPods dependencies:
```jsx
npm i && npm run pod-install
```
7. Depending on the platform you are targeting, run your Android/iOS app in production mode.
8. Upon completion, the generated source map can be found at:
Android: `android/app/build/generated/sourcemaps/react/productionRelease/index.android.bundle.map`
IOS: `main.jsbundle.map`
### Recording a Trace:
1. Ensure you have generated the source map as outlined above.
2. Launch the app in production mode.
2. Navigate to the feature you wish to profile.
3. Initiate the profiling session by tapping with four fingers to open the menu and selecting **`Use Profiling`**.
4. Close the menu and interact with the app.
5. After completing your interactions, tap with four fingers again and select to stop profiling.
6. You will be presented with a **`Share`** option to export the trace, which includes a trace file (`Profile<app version>.cpuprofile`) and build info (`AppInfo<app version>.json`).
Build info:
```jsx
{
appVersion: "1.0.0",
environment: "production",
platform: "IOS",
totalMemory: "3GB",
usedMemory: "300MB"
}
```
### How to symbolicate trace record:
1. You have two files: `AppInfo<app version>.json` and `Profile<app version>.cpuprofile`
2. Place the `Profile<app version>.cpuprofile` file at the root of your project.
3. If you have already generated a source map from the steps above for this branch, you can skip to the next step. Otherwise, obtain the app version from `AppInfo<app version>.json` switch to that branch and generate the source map as described.
`IMPORTANT:` You should generate the source map from the same branch as the trace was recorded.
4. Use the following commands to symbolicate the trace for Android and iOS, respectively:
Android: `npm run symbolicate-release:android`
IOS: `npm run symbolicate-release:ios`
5. A new file named `Profile_trace_for_<app version>-converted.json` will appear in your project's root folder.
6. Open this file in your tool of choice:
- SpeedScope ([https://www.speedscope.app](https://www.speedscope.app/))
- Perfetto UI (https://ui.perfetto.dev/)
- Google Chrome's Tracing UI (chrome://tracing)
---
# App Structure and Conventions
Expand Down Expand Up @@ -409,8 +482,8 @@ Updated rules for managing members across all types of chats in New Expensify.
- Members can't leave or be removed from the #announce room
- Admins can't leave or be removed from #admins
- Domain members can't leave or be removed from their domain chat
- Report submitters can't leave or be removed from their reports
- Report managers can't leave or be removed from their reports
- Report submitters can't leave or be removed from their reports
- Report managers can't leave or be removed from their reports
- Group owners cannot be removed from their groups - they need to transfer ownership first
- **Excepting the above, admins can remove anyone. For example:**
- Group admins can remove other group admins, as well as group members
Expand All @@ -421,17 +494,17 @@ Updated rules for managing members across all types of chats in New Expensify.
1. ### DM
| | Member
| :---: | :---:
| **Invite** | ❌
| **Remove** | ❌
| **Leave** | ❌
| :---: | :---:
| **Invite** | ❌
| **Remove** | ❌
| **Leave** | ❌
| **Can be removed** | ❌
- DM always has two participants. None of the participant can leave or be removed from the DM. Also no additional member can be invited to the chat.
2. ### Workspace
1. #### Workspace
| | Creator | Member(Employee/User) | Admin | Auditor?
| :---: | :---: | :---: | :---: | :---:
| :---: | :---: | :---: | :---: | :---:
| **Invite** | ✅ | ❌ | ✅ | ❌
| **Remove** | ✅ | ❌ | ✅ | ❌
| **Leave** | ❌ | ✅ | ❌ | ✅
Expand All @@ -445,7 +518,7 @@ Updated rules for managing members across all types of chats in New Expensify.
2. #### Workspace #announce room
| | Member(Employee/User) | Admin | Auditor?
| :---: | :---: | :---: | :---:
| :---: | :---: | :---: | :---:
| **Invite** | ❌ | ❌ | ❌
| **Remove** | ❌ | ❌ | ❌
| **Leave** | ❌ | ❌ | ❌
Expand All @@ -455,14 +528,14 @@ Updated rules for managing members across all types of chats in New Expensify.
3. #### Workspace #admin room
| | Admin |
| :---: | :---:
| **Invite** | ❌
| **Remove** | ❌
| **Leave** | ❌
| :---: | :---:
| **Invite** | ❌
| **Remove** | ❌
| **Leave** | ❌
| **Can be removed** | ❌
- Admins can't leave or be removed from #admins
4. #### Workspace rooms
| | Creator | Member | Guest(outside of the workspace)
| :---: | :---: | :---: | :---:
Expand All @@ -475,10 +548,10 @@ Updated rules for managing members across all types of chats in New Expensify.
- Guests are not able to remove anyone from the room
4. #### Workspace chats
| | Admin | Member(default) | Member(invited)
| | Admin | Member(default) | Member(invited)
| :---: | :---: | :---: | :---:
| **Invite** | ✅ | ✅ | ❌
| **Remove** | ✅ | ✅ | ❌
| **Remove** | ✅ | ✅ | ❌
| **Leave** | ❌ | ❌ | ✅
| **Can be removed** | ❌ | ❌ | ✅
Expand All @@ -490,16 +563,16 @@ Updated rules for managing members across all types of chats in New Expensify.
3. ### Domain chat
| | Member
| :---: | :---:
| **Remove** | ❌
| **Leave** | ❌
| **Can be removed** | ❌
| :---: | :---:
| **Remove** | ❌
| **Leave** | ❌
| **Can be removed** | ❌
- Domain members can't leave or be removed from their domain chat
4. ### Reports
| | Submitter | Manager
| :---: | :---: | :---:
| :---: | :---: | :---:
| **Remove** | ❌ | ❌
| **Leave** | ❌ | ❌
| **Can be removed** | ❌ | ❌
Expand Down
File renamed without changes.
Loading

0 comments on commit 4272054

Please sign in to comment.