diff --git a/.eslintignore b/.eslintignore index d3358a02fe4b..5338df11c520 100644 --- a/.eslintignore +++ b/.eslintignore @@ -1,5 +1,11 @@ -**/node_modules/* -**/dist/* -android/**/build/* -.github/actions/**/index.js" +!.storybook +!.github +.github/actions/**/index.js +*.config.js +**/.eslintrc.js +**/node_modules/** +**/dist/** +android/**/build/** docs/vendor/** +docs/assets/** +web/gtm.js diff --git a/.eslintrc.js b/.eslintrc.js index 4df9493b2e8c..135252825dcf 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -4,9 +4,9 @@ const restrictedImportPaths = [ 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 'useWindowDimensions', please use '@src/hooks/useWindowDimensions' instead.", + "For 'TouchableOpacity', 'TouchableWithoutFeedback', 'TouchableNativeFeedback', 'TouchableHighlight', 'Pressable', please use 'PressableWithFeedback' and/or 'PressableWithoutFeedback' from '@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'), @@ -14,7 +14,7 @@ const restrictedImportPaths = [ { name: 'react-native-gesture-handler', importNames: ['TouchableOpacity', 'TouchableWithoutFeedback', 'TouchableNativeFeedback', 'TouchableHighlight'], - message: "Please use 'PressableWithFeedback' and/or 'PressableWithoutFeedback' from 'src/components/Pressable' instead.", + message: "Please use 'PressableWithFeedback' and/or 'PressableWithoutFeedback' from '@components/Pressable' instead.", }, { name: 'awesome-phonenumber', @@ -24,7 +24,7 @@ const restrictedImportPaths = [ { 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.", + message: "Please use 'useSafeAreaInsets' from '@src/hooks/useSafeAreaInset' and/or 'SafeAreaConsumer' from '@components/SafeAreaConsumer' instead.", }, { name: 'react', @@ -34,18 +34,18 @@ const restrictedImportPaths = [ { name: '@styles/index', importNames: ['default', 'defaultStyles'], - message: 'Do not import styles directly. Please use the `useThemeStyles` hook or `withThemeStyles` HOC instead.', + message: 'Do not import styles directly. Please use the `useThemeStyles` hook instead.', }, { name: '@styles/utils', importNames: ['default', 'DefaultStyleUtils'], - message: 'Do not import StyleUtils directly. Please use the `useStyleUtils` hook or `withStyleUtils` HOC instead.', + message: 'Do not import StyleUtils directly. Please use the `useStyleUtils` hook instead.', }, { name: '@styles/theme', importNames: ['default', 'defaultTheme'], - message: 'Do not import themes directly. Please use the `useTheme` hook or `withTheme` HOC instead.', + message: 'Do not import themes directly. Please use the `useTheme` hook instead.', }, { name: '@styles/theme/illustrations', @@ -60,15 +60,15 @@ const restrictedImportPaths = [ const restrictedImportPatterns = [ { group: ['**/assets/animations/**/*.json'], - message: "Do not import animations directly. Please use the 'src/components/LottieAnimations' import instead.", + message: "Do not import animations directly. Please use the '@components/LottieAnimations' import instead.", }, { group: ['@styles/theme/themes/**'], - message: 'Do not import themes directly. Please use the `useTheme` hook or `withTheme` HOC instead.', + message: 'Do not import themes directly. Please use the `useTheme` hook instead.', }, { group: ['@styles/utils/**', '!@styles/utils/FontUtils', '!@styles/utils/types'], - message: 'Do not import style util functions directly. Please use the `useStyleUtils` hook or `withStyleUtils` HOC instead.', + message: 'Do not import style util functions directly. Please use the `useStyleUtils` hook instead.', }, { group: ['@styles/theme/illustrations/themes/**'], @@ -77,203 +77,181 @@ const restrictedImportPatterns = [ ]; module.exports = { - extends: ['expensify', 'plugin:storybook/recommended', 'plugin:react-native-a11y/basic', 'plugin:@dword-design/import-alias/recommended', 'prettier'], - plugins: ['react-native-a11y'], - parser: 'babel-eslint', - ignorePatterns: ['!.*', 'src/vendor', '.github/actions/**/index.js', 'desktop/dist/*.js', 'dist/*.js', 'node_modules/.bin/**', 'node_modules/.cache/**', '.git/**'], + extends: [ + 'expensify', + 'airbnb-typescript', + 'plugin:storybook/recommended', + 'plugin:react-native-a11y/basic', + 'plugin:@dword-design/import-alias/recommended', + 'plugin:@typescript-eslint/recommended-type-checked', + 'plugin:@typescript-eslint/stylistic-type-checked', + 'plugin:you-dont-need-lodash-underscore/all', + 'prettier', + ], + plugins: ['@typescript-eslint', 'jsdoc', 'you-dont-need-lodash-underscore', 'react-native-a11y', 'react', 'testing-library'], + parser: '@typescript-eslint/parser', + parserOptions: { + project: './tsconfig.json', + }, env: { jest: true, }, globals: { __DEV__: 'readonly', }, - overrides: [ - { - files: ['*.js', '*.jsx', '*.ts', '*.tsx'], - plugins: ['react'], - rules: { - 'prefer-regex-literals': 'off', - 'rulesdir/no-multiple-onyx-in-file': 'off', - 'react-native-a11y/has-accessibility-hint': ['off'], - 'react/jsx-no-constructed-context-values': 'error', - 'react-native-a11y/has-valid-accessibility-descriptors': [ - 'error', - { - touchables: ['PressableWithoutFeedback', 'PressableWithFeedback'], - }, - ], - '@dword-design/import-alias/prefer-alias': [ - 'warn', - { - alias: { - '@assets': './assets', - '@components': './src/components', - '@hooks': './src/hooks', - // This is needed up here, if not @libs/actions would take the priority - '@userActions': './src/libs/actions', - '@libs': './src/libs', - '@navigation': './src/libs/Navigation', - '@pages': './src/pages', - '@styles': './src/styles', - // This path is provide alias for files like `ONYXKEYS` and `CONST`. - '@src': './src', - '@desktop': './desktop', - '@github': './.github', - }, - }, - ], + rules: { + '@typescript-eslint/no-unsafe-call': 'off', + '@typescript-eslint/no-unsafe-member-access': 'off', + '@typescript-eslint/no-unsafe-assignment': 'off', + + // TypeScript specific rules + '@typescript-eslint/prefer-enum-initializers': 'error', + '@typescript-eslint/no-var-requires': 'off', + '@typescript-eslint/no-non-null-assertion': 'error', + '@typescript-eslint/switch-exhaustiveness-check': 'error', + '@typescript-eslint/consistent-type-definitions': ['error', 'type'], + '@typescript-eslint/no-floating-promises': 'off', + '@typescript-eslint/no-import-type-side-effects': 'error', + '@typescript-eslint/array-type': ['error', {default: 'array-simple'}], + '@typescript-eslint/naming-convention': [ + 'error', + { + selector: ['variable', 'property'], + format: ['camelCase', 'UPPER_CASE', 'PascalCase'], }, - }, - // This helps disable the `prefer-alias` rule to be enabled for specific directories - { - files: ['tests/**/*.js', 'tests/**/*.ts', 'tests/**/*.jsx', 'assets/**/*.js', '.storybook/**/*.js'], - rules: {'@dword-design/import-alias/prefer-alias': ['off']}, - }, - { - files: ['*.js', '*.jsx'], - settings: { - 'import/resolver': { - node: { - extensions: ['.js', '.website.js', '.desktop.js', '.native.js', '.ios.js', '.android.js', '.config.js', '.ts', '.tsx'], - }, + { + selector: 'function', + format: ['camelCase', 'PascalCase'], + }, + { + selector: ['typeLike', 'enumMember'], + format: ['PascalCase'], + }, + { + selector: ['parameter', 'method'], + format: ['camelCase', 'PascalCase'], + leadingUnderscore: 'allow', + }, + ], + '@typescript-eslint/ban-types': [ + 'error', + { + types: { + object: "Use 'Record' instead.", }, + extendDefaults: true, }, - rules: { - 'import/extensions': [ - 'error', - 'ignorePackages', - { - js: 'never', - jsx: 'never', - ts: 'never', - tsx: 'never', - }, - ], - 'no-restricted-imports': [ - 'error', - { - paths: restrictedImportPaths, - patterns: restrictedImportPatterns, - }, - ], - curly: 'error', - 'react/display-name': 'error', + ], + '@typescript-eslint/consistent-type-imports': [ + 'error', + { + prefer: 'type-imports', + fixStyle: 'separate-type-imports', }, - }, - { - files: ['*.ts', '*.tsx'], - extends: [ - 'airbnb-typescript', - 'plugin:@typescript-eslint/recommended-type-checked', - 'plugin:@typescript-eslint/stylistic-type-checked', - 'plugin:you-dont-need-lodash-underscore/all', - 'prettier', - ], - plugins: ['@typescript-eslint', 'jsdoc', 'you-dont-need-lodash-underscore'], - parser: '@typescript-eslint/parser', - parserOptions: { - project: './tsconfig.json', + ], + '@typescript-eslint/consistent-type-exports': [ + 'error', + { + fixMixedExportsWithInlineTypeSpecifier: false, }, - rules: { - // TODO: Remove the following rules after TypeScript migration is complete. - '@typescript-eslint/no-unsafe-argument': 'off', - '@typescript-eslint/no-unsafe-call': 'off', - '@typescript-eslint/no-unsafe-member-access': 'off', - '@typescript-eslint/no-unsafe-assignment': 'off', + ], + '@typescript-eslint/no-use-before-define': ['error', {functions: false}], - '@typescript-eslint/naming-convention': [ - 'error', - { - selector: ['variable', 'property'], - format: ['camelCase', 'UPPER_CASE', 'PascalCase'], - }, - { - selector: 'function', - format: ['camelCase', 'PascalCase'], - }, - { - selector: ['typeLike', 'enumMember'], - format: ['PascalCase'], - }, - { - selector: ['parameter', 'method'], - format: ['camelCase', 'PascalCase'], - leadingUnderscore: 'allow', - }, - ], - '@typescript-eslint/ban-types': [ - 'error', - { - types: { - object: "Use 'Record' instead.", - }, - extendDefaults: true, - }, - ], - '@typescript-eslint/array-type': ['error', {default: 'array-simple'}], - '@typescript-eslint/prefer-enum-initializers': 'error', - '@typescript-eslint/no-var-requires': 'off', - '@typescript-eslint/no-non-null-assertion': 'error', - '@typescript-eslint/switch-exhaustiveness-check': 'error', - '@typescript-eslint/consistent-type-definitions': ['error', 'type'], - '@typescript-eslint/no-floating-promises': 'off', - '@typescript-eslint/consistent-type-imports': [ - 'error', - { - prefer: 'type-imports', - fixStyle: 'separate-type-imports', - }, - ], - '@typescript-eslint/no-import-type-side-effects': 'error', - '@typescript-eslint/consistent-type-exports': [ - 'error', - { - fixMixedExportsWithInlineTypeSpecifier: false, - }, - ], - 'import/consistent-type-specifier-style': ['error', 'prefer-top-level'], - 'es/no-nullish-coalescing-operators': 'off', - 'es/no-optional-chaining': 'off', - 'valid-jsdoc': 'off', - 'jsdoc/no-types': 'error', - 'rulesdir/no-default-props': 'error', - 'import/no-extraneous-dependencies': 'off', - 'rulesdir/prefer-underscore-method': 'off', - 'rulesdir/prefer-import-module-contents': 'off', - 'react/require-default-props': 'off', - 'react/prop-types': 'off', - 'no-restricted-syntax': [ - 'error', - { - selector: 'TSEnumDeclaration', - message: "Please don't declare enums, use union types instead.", - }, - ], - 'no-restricted-properties': [ - 'error', - { - object: 'Image', - property: 'getSize', - message: 'Usage of Image.getImage is restricted. Please use the `react-native-image-size`.', - }, - ], - 'no-restricted-imports': [ - 'error', - { - paths: restrictedImportPaths, - patterns: restrictedImportPatterns, - }, - ], - curly: 'error', - 'you-dont-need-lodash-underscore/throttle': 'off', + // ESLint core rules + 'es/no-nullish-coalescing-operators': 'off', + 'es/no-optional-chaining': 'off', + + // Import specific rules + 'import/consistent-type-specifier-style': ['error', 'prefer-top-level'], + 'import/no-extraneous-dependencies': 'off', + + // Rulesdir specific rules + 'rulesdir/no-default-props': 'error', + 'rulesdir/no-multiple-onyx-in-file': 'off', + 'rulesdir/prefer-underscore-method': 'off', + 'rulesdir/prefer-import-module-contents': 'off', + + // React and React Native specific rules + 'react-native-a11y/has-accessibility-hint': ['off'], + 'react/require-default-props': 'off', + 'react/prop-types': 'off', + 'react/jsx-no-constructed-context-values': 'error', + 'react-native-a11y/has-valid-accessibility-descriptors': [ + 'error', + { + touchables: ['PressableWithoutFeedback', 'PressableWithFeedback'], + }, + ], + + // Disallow usage of certain functions and imports + 'no-restricted-syntax': [ + 'error', + { + selector: 'TSEnumDeclaration', + message: "Please don't declare enums, use union types instead.", + }, + ], + 'no-restricted-properties': [ + 'error', + { + object: 'Image', + property: 'getSize', + message: 'Usage of Image.getImage is restricted. Please use the `react-native-image-size`.', + }, + ], + 'no-restricted-imports': [ + 'error', + { + paths: restrictedImportPaths, + patterns: restrictedImportPatterns, + }, + ], + + // Other rules + curly: 'error', + 'you-dont-need-lodash-underscore/throttle': 'off', + 'prefer-regex-literals': 'off', + 'valid-jsdoc': 'off', + 'jsdoc/no-types': 'error', + '@dword-design/import-alias/prefer-alias': [ + 'warn', + { + alias: { + '@assets': './assets', + '@components': './src/components', + '@hooks': './src/hooks', + // This is needed up here, if not @libs/actions would take the priority + '@userActions': './src/libs/actions', + '@libs': './src/libs', + '@navigation': './src/libs/Navigation', + '@pages': './src/pages', + '@styles': './src/styles', + // This path is provide alias for files like `ONYXKEYS` and `CONST`. + '@src': './src', + '@desktop': './desktop', + '@github': './.github', + }, + }, + ], + }, + + // Remove once no JS files are left + overrides: [ + { + files: ['*.js', '*.jsx'], + rules: { + '@typescript-eslint/prefer-nullish-coalescing': 'off', + '@typescript-eslint/no-unsafe-return': 'off', + '@typescript-eslint/unbound-method': 'off', + 'jsdoc/no-types': 'off', + 'react/jsx-filename-extension': 'off', + 'rulesdir/no-default-props': 'off', }, }, { - files: ['workflow_tests/**/*.{js,jsx,ts,tsx}', 'tests/**/*.{js,jsx,ts,tsx}', '.github/**/*.{js,jsx,ts,tsx}'], + files: ['en.ts', 'es.ts'], rules: { - '@lwc/lwc/no-async-await': 'off', - 'no-await-in-loop': 'off', - 'no-restricted-syntax': ['error', 'ForInStatement', 'LabeledStatement', 'WithStatement'], + 'rulesdir/use-periods-for-error-messages': 'error', }, }, ], diff --git a/.github/.eslintrc.js b/.github/.eslintrc.js index e769944cd1a9..d6d39822b737 100644 --- a/.github/.eslintrc.js +++ b/.github/.eslintrc.js @@ -1,6 +1,10 @@ -// For all these Node.js scripts, we do not want to disable `console` statements module.exports = { rules: { + // For all these Node.js scripts, we do not want to disable `console` statements 'no-console': 'off', + + '@lwc/lwc/no-async-await': 'off', + 'no-await-in-loop': 'off', + 'no-restricted-syntax': ['error', 'ForInStatement', 'LabeledStatement', 'WithStatement'], }, }; diff --git a/.github/actions/javascript/awaitStagingDeploys/awaitStagingDeploys.ts b/.github/actions/javascript/awaitStagingDeploys/awaitStagingDeploys.ts index 7de78e257dc4..96bb17a14354 100644 --- a/.github/actions/javascript/awaitStagingDeploys/awaitStagingDeploys.ts +++ b/.github/actions/javascript/awaitStagingDeploys/awaitStagingDeploys.ts @@ -8,18 +8,10 @@ import {promiseDoWhile} from '@github/libs/promiseWhile'; type CurrentStagingDeploys = Awaited>['data']['workflow_runs']; function run() { - console.info('[awaitStagingDeploys] run()'); - console.info('[awaitStagingDeploys] getStringInput', getStringInput); - console.info('[awaitStagingDeploys] GitHubUtils', GitHubUtils); - console.info('[awaitStagingDeploys] promiseDoWhile', promiseDoWhile); - const tag = getStringInput('TAG', {required: false}); - console.info('[awaitStagingDeploys] run() tag', tag); let currentStagingDeploys: CurrentStagingDeploys = []; - console.info('[awaitStagingDeploys] run() _.throttle', lodashThrottle); - const throttleFunc = () => Promise.all([ // These are active deploys @@ -41,24 +33,20 @@ function run() { }), ]) .then((responses) => { - console.info('[awaitStagingDeploys] listWorkflowRuns responses', responses); const workflowRuns = responses[0].data.workflow_runs; if (!tag && typeof responses[1] === 'object') { workflowRuns.push(...responses[1].data.workflow_runs); } - console.info('[awaitStagingDeploys] workflowRuns', workflowRuns); return workflowRuns; }) .then((workflowRuns) => (currentStagingDeploys = workflowRuns.filter((workflowRun) => workflowRun.status !== 'completed'))) .then(() => { - console.info('[awaitStagingDeploys] currentStagingDeploys', currentStagingDeploys); console.log( !currentStagingDeploys.length ? 'No current staging deploys found' : `Found ${currentStagingDeploys.length} staging deploy${currentStagingDeploys.length > 1 ? 's' : ''} still running...`, ); }); - console.info('[awaitStagingDeploys] run() throttleFunc', throttleFunc); return promiseDoWhile( () => !!currentStagingDeploys.length, diff --git a/.github/actions/javascript/awaitStagingDeploys/index.js b/.github/actions/javascript/awaitStagingDeploys/index.js index d84c6df1a0d3..0e0168fdb7ae 100644 --- a/.github/actions/javascript/awaitStagingDeploys/index.js +++ b/.github/actions/javascript/awaitStagingDeploys/index.js @@ -12131,14 +12131,8 @@ const CONST_1 = __importDefault(__nccwpck_require__(9873)); const GithubUtils_1 = __importDefault(__nccwpck_require__(9296)); const promiseWhile_1 = __nccwpck_require__(9438); function run() { - console.info('[awaitStagingDeploys] run()'); - console.info('[awaitStagingDeploys] getStringInput', ActionUtils_1.getStringInput); - console.info('[awaitStagingDeploys] GitHubUtils', GithubUtils_1.default); - console.info('[awaitStagingDeploys] promiseDoWhile', promiseWhile_1.promiseDoWhile); const tag = (0, ActionUtils_1.getStringInput)('TAG', { required: false }); - console.info('[awaitStagingDeploys] run() tag', tag); let currentStagingDeploys = []; - console.info('[awaitStagingDeploys] run() _.throttle', throttle_1.default); const throttleFunc = () => Promise.all([ // These are active deploys GithubUtils_1.default.octokit.actions.listWorkflowRuns({ @@ -12158,22 +12152,18 @@ function run() { }), ]) .then((responses) => { - console.info('[awaitStagingDeploys] listWorkflowRuns responses', responses); const workflowRuns = responses[0].data.workflow_runs; if (!tag && typeof responses[1] === 'object') { workflowRuns.push(...responses[1].data.workflow_runs); } - console.info('[awaitStagingDeploys] workflowRuns', workflowRuns); return workflowRuns; }) .then((workflowRuns) => (currentStagingDeploys = workflowRuns.filter((workflowRun) => workflowRun.status !== 'completed'))) .then(() => { - console.info('[awaitStagingDeploys] currentStagingDeploys', currentStagingDeploys); console.log(!currentStagingDeploys.length ? 'No current staging deploys found' : `Found ${currentStagingDeploys.length} staging deploy${currentStagingDeploys.length > 1 ? 's' : ''} still running...`); }); - console.info('[awaitStagingDeploys] run() throttleFunc', throttleFunc); return (0, promiseWhile_1.promiseDoWhile)(() => !!currentStagingDeploys.length, (0, throttle_1.default)(throttleFunc, // Poll every 60 seconds instead of every 10 seconds CONST_1.default.POLL_RATE * 6)); @@ -12729,7 +12719,6 @@ exports.promiseDoWhile = exports.promiseWhile = void 0; * Simulates a while loop where the condition is determined by the result of a Promise. */ function promiseWhile(condition, action) { - console.info('[promiseWhile] promiseWhile()'); return new Promise((resolve, reject) => { const loop = function () { if (!condition()) { @@ -12737,12 +12726,16 @@ function promiseWhile(condition, action) { } else { const actionResult = action?.(); - console.info('[promiseWhile] promiseWhile() actionResult', actionResult); if (!actionResult) { resolve(); return; } - Promise.resolve(actionResult).then(loop).catch(reject); + Promise.resolve(actionResult) + .then(() => { + // Set a timeout to delay the next loop iteration + setTimeout(loop, 1000); // 1000 ms delay + }) + .catch(reject); } }; loop(); @@ -12753,11 +12746,8 @@ exports.promiseWhile = promiseWhile; * Simulates a do-while loop where the condition is determined by the result of a Promise. */ function promiseDoWhile(condition, action) { - console.info('[promiseWhile] promiseDoWhile()'); return new Promise((resolve, reject) => { - console.info('[promiseWhile] promiseDoWhile() condition', condition); const actionResult = action?.(); - console.info('[promiseWhile] promiseDoWhile() actionResult', actionResult); if (!actionResult) { resolve(); return; diff --git a/.github/actions/javascript/bumpVersion/bumpVersion.ts b/.github/actions/javascript/bumpVersion/bumpVersion.ts index ed4828367cf2..eba79c7c9edb 100644 --- a/.github/actions/javascript/bumpVersion/bumpVersion.ts +++ b/.github/actions/javascript/bumpVersion/bumpVersion.ts @@ -1,6 +1,7 @@ import * as core from '@actions/core'; import {exec as originalExec} from 'child_process'; import fs from 'fs'; +import type {PackageJson} from 'type-fest'; import {promisify} from 'util'; import {generateAndroidVersionCode, updateAndroidVersion, updateiOSVersion} from '@github/libs/nativeVersionUpdater'; import * as versionUpdater from '@github/libs/versionUpdater'; @@ -19,7 +20,7 @@ function updateNativeVersions(version: string) { .then(() => { console.log('Successfully updated Android!'); }) - .catch((err) => { + .catch((err: string | Error) => { console.error('Error updating Android'); core.setFailed(err); }); @@ -47,8 +48,12 @@ if (!semanticVersionLevel || !Object.keys(versionUpdater.SEMANTIC_VERSION_LEVELS console.log(`Invalid input for 'SEMVER_LEVEL': ${semanticVersionLevel}`, `Defaulting to: ${semanticVersionLevel}`); } -const {version: previousVersion} = JSON.parse(fs.readFileSync('./package.json').toString()); -const newVersion = versionUpdater.incrementVersion(previousVersion, semanticVersionLevel); +const {version: previousVersion}: PackageJson = JSON.parse(fs.readFileSync('./package.json').toString()); +if (!previousVersion) { + core.setFailed('Error: Could not read package.json'); +} + +const newVersion = versionUpdater.incrementVersion(previousVersion ?? '', semanticVersionLevel); console.log(`Previous version: ${previousVersion}`, `New version: ${newVersion}`); updateNativeVersions(newVersion); diff --git a/.github/actions/javascript/bumpVersion/index.js b/.github/actions/javascript/bumpVersion/index.js index d4a085fc9ddf..e1a5cf13a8d9 100644 --- a/.github/actions/javascript/bumpVersion/index.js +++ b/.github/actions/javascript/bumpVersion/index.js @@ -3478,7 +3478,10 @@ if (!semanticVersionLevel || !Object.keys(versionUpdater.SEMANTIC_VERSION_LEVELS console.log(`Invalid input for 'SEMVER_LEVEL': ${semanticVersionLevel}`, `Defaulting to: ${semanticVersionLevel}`); } const { version: previousVersion } = JSON.parse(fs_1.default.readFileSync('./package.json').toString()); -const newVersion = versionUpdater.incrementVersion(previousVersion, semanticVersionLevel); +if (!previousVersion) { + core.setFailed('Error: Could not read package.json'); +} +const newVersion = versionUpdater.incrementVersion(previousVersion ?? '', semanticVersionLevel); console.log(`Previous version: ${previousVersion}`, `New version: ${newVersion}`); updateNativeVersions(newVersion); console.log(`Setting npm version to ${newVersion}`); diff --git a/.github/actions/javascript/checkDeployBlockers/checkDeployBlockers.ts b/.github/actions/javascript/checkDeployBlockers/checkDeployBlockers.ts index bf94b136ce43..d51d68796070 100644 --- a/.github/actions/javascript/checkDeployBlockers/checkDeployBlockers.ts +++ b/.github/actions/javascript/checkDeployBlockers/checkDeployBlockers.ts @@ -60,7 +60,7 @@ const run = function (): Promise { core.setOutput('HAS_DEPLOY_BLOCKERS', false); } }) - .catch((error) => { + .catch((error: string | Error) => { console.error('A problem occurred while trying to communicate with the GitHub API', error); core.setFailed(error); }); diff --git a/.github/actions/javascript/getGraphiteString/getGraphiteString.ts b/.github/actions/javascript/getGraphiteString/getGraphiteString.ts index 57a941105f90..c486fdbd39f3 100644 --- a/.github/actions/javascript/getGraphiteString/getGraphiteString.ts +++ b/.github/actions/javascript/getGraphiteString/getGraphiteString.ts @@ -28,7 +28,7 @@ const run = () => { // Extract timestamp, Graphite accepts timestamp in seconds if (current.metadata?.creationDate) { - timestamp = Math.floor(new Date(current.metadata.creationDate).getTime() / 1000); + timestamp = Math.floor(new Date(current.metadata.creationDate as string).getTime() / 1000); } if (current.name && current.meanDuration && current.meanCount && timestamp) { diff --git a/.github/actions/javascript/getPreviousVersion/getPreviousVersion.ts b/.github/actions/javascript/getPreviousVersion/getPreviousVersion.ts index dc1e99d1e3b8..262b603124fa 100644 --- a/.github/actions/javascript/getPreviousVersion/getPreviousVersion.ts +++ b/.github/actions/javascript/getPreviousVersion/getPreviousVersion.ts @@ -1,5 +1,6 @@ import * as core from '@actions/core'; import {readFileSync} from 'fs'; +import type {PackageJson} from 'type-fest'; import * as versionUpdater from '@github/libs/versionUpdater'; const semverLevel = core.getInput('SEMVER_LEVEL', {required: true}); @@ -7,6 +8,10 @@ if (!semverLevel || !Object.values(versionUpdater.SEMANTIC_VERSION_LEVEL core.setFailed(`'Error: Invalid input for 'SEMVER_LEVEL': ${semverLevel}`); } -const {version: currentVersion} = JSON.parse(readFileSync('./package.json', 'utf8')); -const previousVersion = versionUpdater.getPreviousVersion(currentVersion, semverLevel); +const {version: currentVersion}: PackageJson = JSON.parse(readFileSync('./package.json', 'utf8')); +if (!currentVersion) { + core.setFailed('Error: Could not read package.json'); +} + +const previousVersion = versionUpdater.getPreviousVersion(currentVersion ?? '', semverLevel); core.setOutput('PREVIOUS_VERSION', previousVersion); diff --git a/.github/actions/javascript/getPreviousVersion/index.js b/.github/actions/javascript/getPreviousVersion/index.js index f372f0fdaf99..8eac2f62f03e 100644 --- a/.github/actions/javascript/getPreviousVersion/index.js +++ b/.github/actions/javascript/getPreviousVersion/index.js @@ -2728,7 +2728,10 @@ if (!semverLevel || !Object.values(versionUpdater.SEMANTIC_VERSION_LEVELS).inclu core.setFailed(`'Error: Invalid input for 'SEMVER_LEVEL': ${semverLevel}`); } const { version: currentVersion } = JSON.parse((0, fs_1.readFileSync)('./package.json', 'utf8')); -const previousVersion = versionUpdater.getPreviousVersion(currentVersion, semverLevel); +if (!currentVersion) { + core.setFailed('Error: Could not read package.json'); +} +const previousVersion = versionUpdater.getPreviousVersion(currentVersion ?? '', semverLevel); core.setOutput('PREVIOUS_VERSION', previousVersion); diff --git a/.github/actions/javascript/reviewerChecklist/reviewerChecklist.ts b/.github/actions/javascript/reviewerChecklist/reviewerChecklist.ts index aabc6b33086a..f57ef6c36a04 100644 --- a/.github/actions/javascript/reviewerChecklist/reviewerChecklist.ts +++ b/.github/actions/javascript/reviewerChecklist/reviewerChecklist.ts @@ -90,7 +90,7 @@ function checkIssueForCompletedChecklist(numberOfChecklistItems: number) { getNumberOfItemsFromReviewerChecklist() .then(checkIssueForCompletedChecklist) - .catch((err) => { + .catch((err: string | Error) => { console.error(err); core.setFailed(err); }); diff --git a/.github/libs/promiseWhile.ts b/.github/libs/promiseWhile.ts index 01c061096d64..8bedceb894fd 100644 --- a/.github/libs/promiseWhile.ts +++ b/.github/libs/promiseWhile.ts @@ -4,22 +4,24 @@ import type {DebouncedFunc} from 'lodash'; * Simulates a while loop where the condition is determined by the result of a Promise. */ function promiseWhile(condition: () => boolean, action: (() => Promise) | DebouncedFunc<() => Promise> | undefined): Promise { - console.info('[promiseWhile] promiseWhile()'); - return new Promise((resolve, reject) => { const loop = function () { if (!condition()) { resolve(); } else { const actionResult = action?.(); - console.info('[promiseWhile] promiseWhile() actionResult', actionResult); if (!actionResult) { resolve(); return; } - Promise.resolve(actionResult).then(loop).catch(reject); + Promise.resolve(actionResult) + .then(() => { + // Set a timeout to delay the next loop iteration + setTimeout(loop, 1000); // 1000 ms delay + }) + .catch(reject); } }; loop(); @@ -30,12 +32,9 @@ function promiseWhile(condition: () => boolean, action: (() => Promise) | * Simulates a do-while loop where the condition is determined by the result of a Promise. */ function promiseDoWhile(condition: () => boolean, action: (() => Promise) | DebouncedFunc<() => Promise> | undefined): Promise { - console.info('[promiseWhile] promiseDoWhile()'); - return new Promise((resolve, reject) => { - console.info('[promiseWhile] promiseDoWhile() condition', condition); const actionResult = action?.(); - console.info('[promiseWhile] promiseDoWhile() actionResult', actionResult); + if (!actionResult) { resolve(); return; diff --git a/.github/libs/sanitizeStringForJSONParse.ts b/.github/libs/sanitizeStringForJSONParse.ts index 3fbdaa8661f8..ddb7549b0186 100644 --- a/.github/libs/sanitizeStringForJSONParse.ts +++ b/.github/libs/sanitizeStringForJSONParse.ts @@ -16,7 +16,7 @@ const replacer = (str: string): string => * Solution partly taken from SO user Gabriel Rodríguez Flores 🙇 * https://stackoverflow.com/questions/52789718/how-to-remove-special-characters-before-json-parse-while-file-reading */ -const sanitizeStringForJSONParse = (inputString: string): string => { +const sanitizeStringForJSONParse = (inputString: string | number | boolean | null | undefined): string => { if (typeof inputString !== 'string') { throw new TypeError('Input must me of type String'); } diff --git a/.github/scripts/detectRedirectCycle.ts b/.github/scripts/detectRedirectCycle.ts index 5aa0d1daf342..6da0ecba158c 100644 --- a/.github/scripts/detectRedirectCycle.ts +++ b/.github/scripts/detectRedirectCycle.ts @@ -52,7 +52,7 @@ function detectCycle(): boolean { fs.createReadStream(`${process.cwd()}/docs/redirects.csv`) .pipe(parser) - .on('data', (row) => { + .on('data', (row: [string, string]) => { // Create a directed graph of sourceURL -> targetURL addEdge(row[0], row[1]); }) diff --git a/.github/workflows/e2ePerformanceTests.yml b/.github/workflows/e2ePerformanceTests.yml index 10723d5efa04..7e7d55ac5d2e 100644 --- a/.github/workflows/e2ePerformanceTests.yml +++ b/.github/workflows/e2ePerformanceTests.yml @@ -15,6 +15,10 @@ on: type: string required: true +concurrency: + group: ${{ github.ref == 'refs/heads/main' && format('{0}-{1}', github.ref, github.sha) || github.ref }}-e2e + cancel-in-progress: true + jobs: buildBaseline: runs-on: ubuntu-latest-xl diff --git a/.github/workflows/failureNotifier.yml b/.github/workflows/failureNotifier.yml index 9eb5bc6eb409..39dfbe8e84a7 100644 --- a/.github/workflows/failureNotifier.yml +++ b/.github/workflows/failureNotifier.yml @@ -27,19 +27,49 @@ jobs: }); return jobsData.data; + - name: Fetch Previous Workflow Run + id: previous-workflow-run + uses: actions/github-script@v7 + with: + script: | + const runId = ${{ github.event.workflow_run.id }}; + const allRuns = await github.rest.actions.listWorkflowRuns({ + owner: context.repo.owner, + repo: context.repo.repo, + workflow_id: 'preDeploy.yml', + }); + const filteredRuns = allRuns.data.workflow_runs.filter(run => run.actor.login !== 'OSBotify' && run.status !== 'cancelled'); + const currentIndex = filteredRuns.findIndex(run => run.id === runId); + const previousRun = filteredRuns[currentIndex + 1]; + return previousRun; + + - name: Fetch Previous Workflow Run Jobs + id: previous-workflow-jobs + uses: actions/github-script@v7 + with: + script: | + const previousRun = ${{ steps.previous-workflow-run.outputs.result }}; + const runId = previousRun.id; + const jobsData = await github.rest.actions.listJobsForWorkflowRun({ + owner: context.repo.owner, + repo: context.repo.repo, + run_id: runId, + }); + return jobsData.data; + - name: Process Each Failed Job uses: actions/github-script@v7 with: script: | const jobs = ${{ steps.fetch-workflow-jobs.outputs.result }}; - + const previousRun = ${{ steps.previous-workflow-run.outputs.result }}; + const previousRunJobs = ${{ steps.previous-workflow-jobs.outputs.result }}; const headCommit = "${{ github.event.workflow_run.head_commit.id }}"; const prData = await github.rest.repos.listPullRequestsAssociatedWithCommit({ owner: context.repo.owner, repo: context.repo.repo, commit_sha: headCommit, }); - const pr = prData.data[0]; const prLink = pr.html_url; const prAuthor = pr.user.login; @@ -50,14 +80,8 @@ jobs: if (jobs.jobs[i].conclusion == 'failure') { const jobName = jobs.jobs[i].name; const jobLink = jobs.jobs[i].html_url; - const issues = await github.rest.issues.listForRepo({ - owner: context.repo.owner, - repo: context.repo.repo, - labels: failureLabel, - state: 'open' - }); - const existingIssue = issues.data.find(issue => issue.title.includes(jobName)); - if (!existingIssue) { + const previousJob = previousRunJobs.jobs.find(job => job.name === jobName); + if (previousJob?.conclusion === 'success') { const annotations = await github.rest.checks.listAnnotations({ owner: context.repo.owner, repo: context.repo.repo, diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 50e886942c98..da7757fcbfa8 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -8,7 +8,7 @@ on: paths: ['**.js', '**.ts', '**.tsx', '**.json', '**.mjs', '**.cjs', 'config/.editorconfig', '.watchmanconfig', '.imgbotconfig'] concurrency: - group: "${{ github.ref }}-lint" + group: ${{ github.ref == 'refs/heads/main' && format('{0}-{1}', github.ref, github.sha) || github.ref }}-lint cancel-in-progress: true jobs: diff --git a/.github/workflows/platformDeploy.yml b/.github/workflows/platformDeploy.yml index bb850e6eda10..bb24a1be5e5c 100644 --- a/.github/workflows/platformDeploy.yml +++ b/.github/workflows/platformDeploy.yml @@ -87,12 +87,11 @@ jobs: MYAPP_UPLOAD_STORE_PASSWORD: ${{ secrets.MYAPP_UPLOAD_STORE_PASSWORD }} MYAPP_UPLOAD_KEY_PASSWORD: ${{ secrets.MYAPP_UPLOAD_KEY_PASSWORD }} - # Note: Android production deploys are temporarily disabled until https://github.com/Expensify/App/issues/40108 is resolved - # - name: Run Fastlane production - # if: ${{ fromJSON(env.SHOULD_DEPLOY_PRODUCTION) }} - # run: bundle exec fastlane android production - # env: - # VERSION: ${{ env.VERSION_CODE }} + - name: Run Fastlane production + if: ${{ fromJSON(env.SHOULD_DEPLOY_PRODUCTION) }} + run: bundle exec fastlane android production + env: + VERSION: ${{ env.VERSION_CODE }} - name: Archive Android sourcemaps uses: actions/upload-artifact@v3 @@ -353,6 +352,16 @@ jobs: env: CF_API_KEY: ${{ secrets.CLOUDFLARE_TOKEN }} + # Build a version of iOS and Android HybridApp if we are deploying to staging + hybridApp: + runs-on: ubuntu-latest + needs: validateActor + if: ${{ fromJSON(needs.validateActor.outputs.IS_DEPLOYER) }} + steps: + - name: 'Deploy HybridApp' + if: ${{ !fromJSON(env.SHOULD_DEPLOY_PRODUCTION) }} + uses: Expensify/Mobile-Deploy/.github/workflows/deploy.yml@main + postSlackMessageOnFailure: name: Post a Slack message when any platform fails to build or deploy runs-on: ubuntu-latest diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 71b4bc3d8fc3..d6b346cb3995 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -8,7 +8,7 @@ on: paths: ['**.js', '**.ts', '**.tsx', '**.sh', 'package.json', 'package-lock.json'] concurrency: - group: "${{ github.ref }}-jest" + group: ${{ github.ref == 'refs/heads/main' && format('{0}-{1}', github.ref, github.sha) || github.ref }}-jest cancel-in-progress: true jobs: @@ -41,7 +41,7 @@ jobs: key: ${{ runner.os }}-jest - name: Jest tests - run: npx jest --silent --shard=${{ fromJSON(matrix.chunk) }}/${{ strategy.job-total }} --max-workers ${{ steps.cpu-cores.outputs.count }} + run: NODE_OPTIONS="$NODE_OPTIONS --experimental-vm-modules" npx jest --silent --shard=${{ fromJSON(matrix.chunk) }}/${{ strategy.job-total }} --max-workers ${{ steps.cpu-cores.outputs.count }} storybookTests: if: ${{ github.actor != 'OSBotify' && github.actor != 'imgbot[bot]' || github.event_name == 'workflow_call' }} diff --git a/.github/workflows/typecheck.yml b/.github/workflows/typecheck.yml index f20939f9df0a..476b01f87b07 100644 --- a/.github/workflows/typecheck.yml +++ b/.github/workflows/typecheck.yml @@ -7,6 +7,10 @@ on: branches-ignore: [staging, production] paths: ['**.js', '**.ts', '**.tsx', 'package.json', 'package-lock.json', 'tsconfig.json'] +concurrency: + group: ${{ github.ref == 'refs/heads/main' && format('{0}-{1}', github.ref, github.sha) || github.ref }}-typecheck + cancel-in-progress: true + jobs: typecheck: if: ${{ github.actor != 'OSBotify' || github.event_name == 'workflow_call' }} @@ -30,7 +34,7 @@ jobs: # - git diff is used to see the files that were added on this branch # - gh pr view is used to list files touched by this PR. Git diff may give false positives if the branch isn't up-to-date with main # - wc counts the words in the result of the intersection - count_new_js=$(comm -1 -2 <(git diff --name-only --diff-filter=A origin/main HEAD -- 'src/*.js' '__mocks__/*.js' '.storybook/*.js' 'assets/*.js' 'config/*.js' 'desktop/*.js' 'jest/*.js' 'scripts/*.js' 'tests/*.js' 'web/*.js' 'workflow_tests/*.js' '.github/libs/*.js' '.github/scripts/*.js') <(gh pr view ${{ github.event.pull_request.number }} --json files | jq -r '.files | map(.path) | .[]') | wc -l) + count_new_js=$(comm -1 -2 <(git diff --name-only --diff-filter=A origin/main HEAD -- 'src/*.js' '__mocks__/*.js' '.storybook/*.js' 'assets/*.js' 'config/*.js' 'desktop/*.js' 'jest/*.js' 'scripts/*.js' 'tests/*.js' 'workflow_tests/*.js' '.github/libs/*.js' '.github/scripts/*.js') <(gh pr view ${{ github.event.pull_request.number }} --json files | jq -r '.files | map(.path) | .[]') | wc -l) if [ "$count_new_js" -gt "0" ]; then echo "ERROR: Found new JavaScript files in the project; use TypeScript instead." exit 1 diff --git a/.nvmrc b/.nvmrc index d5a159609d09..62d44807d084 100644 --- a/.nvmrc +++ b/.nvmrc @@ -1 +1 @@ -20.10.0 +20.13.0 diff --git a/.storybook/preview.tsx b/.storybook/preview.tsx index ec8e17dda4cf..7527857eeda7 100644 --- a/.storybook/preview.tsx +++ b/.storybook/preview.tsx @@ -16,7 +16,7 @@ import './fonts.css'; Onyx.init({ keys: ONYXKEYS, initialKeyStates: { - [ONYXKEYS.NETWORK]: {isOffline: false}, + [ONYXKEYS.NETWORK]: {isOffline: false, isBackendReachable: true}, }, }); diff --git a/.storybook/public/favicon.svg b/.storybook/public/favicon.svg index 6bc34f89282e..726791b58cfb 100644 --- a/.storybook/public/favicon.svg +++ b/.storybook/public/favicon.svg @@ -1,11 +1 @@ - - - - - - - - - - - + \ No newline at end of file diff --git a/README.md b/README.md index 8adadfc9cbe6..6544e0e95486 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,7 @@ #### Table of Contents * [Local Development](#local-development) -* [Testing on browsers on simulators and emulators](#testing-on-browsers-on-simulators-and-emulators) +* [Testing on browsers in simulators and emulators](#testing-on-browsers-in-simulators-and-emulators) * [Running The Tests](#running-the-tests) * [Debugging](#debugging) * [App Structure and Conventions](#app-structure-and-conventions) @@ -60,6 +60,7 @@ If you're using another operating system, you will need to ensure `mkcert` is in For an M1 Mac, read this [SO](https://stackoverflow.com/questions/64901180/how-to-run-cocoapods-on-apple-silicon-m1) for installing cocoapods. * If you haven't already, install Xcode tools and make sure to install the optional "iOS Platform" package as well. This installation may take awhile. + * After installation, check in System Settings that there's no update for Xcode. Otherwise, you may encounter issues later that don't explain that you solve them by updating Xcode. * Install project gems, including cocoapods, using bundler to ensure everyone uses the same versions. In the project root, run: `bundle install` * If you get the error `Could not find 'bundler'`, install the bundler gem first: `gem install bundler` and try again. * If you are using MacOS and get the error `Gem::FilePermissionError` when trying to install the bundler gem, you're likely using system Ruby, which requires administrator permission to modify. To get around this, install another version of Ruby with a version manager like [rbenv](https://github.com/rbenv/rbenv#installation). @@ -662,7 +663,38 @@ Sometimes it might be beneficial to generate a local production version instead In order to generate a production web build, run `npm run build`, this will generate a production javascript build in the `dist/` folder. #### Local production build of the MacOS desktop app -In order to compile a production desktop build, run `npm run desktop-build`, this will generate a production app in the `dist/Mac` folder named `Chat.app`. +The commands used to compile a production or staging desktop build are `npm run desktop-build` and `npm run desktop-build-staging`, respectively. These will product an app in the `dist/Mac` folder named NewExpensify.dmg that you can install like a normal app. + +HOWEVER, by default those commands will try to notarize the build (signing it as Expensify) and publish it to the S3 bucket where it's hosted for users. In most cases you won't actually need or want to do that for your local testing. To get around that and disable those behaviors for your local build, apply the following diff: + +```diff +diff --git a/config/electronBuilder.config.js b/config/electronBuilder.config.js +index e4ed685f65..4c7c1b3667 100644 +--- a/config/electronBuilder.config.js ++++ b/config/electronBuilder.config.js +@@ -42,9 +42,6 @@ module.exports = { + entitlements: 'desktop/entitlements.mac.plist', + entitlementsInherit: 'desktop/entitlements.mac.plist', + type: 'distribution', +- notarize: { +- teamId: '368M544MTT', +- }, + }, + dmg: { + title: 'New Expensify', +diff --git a/scripts/build-desktop.sh b/scripts/build-desktop.sh +index 791f59d733..526306eec1 100755 +--- a/scripts/build-desktop.sh ++++ b/scripts/build-desktop.sh +@@ -35,4 +35,4 @@ npx webpack --config config/webpack/webpack.desktop.ts --env file=$ENV_FILE + title "Building Desktop App Archive Using Electron" + info "" + shift 1 +-npx electron-builder --config config/electronBuilder.config.js --publish always "$@" ++npx electron-builder --config config/electronBuilder.config.js --publish never "$@" +``` + +There may be some cases where you need to test a signed and published build, such as when testing the update flows. Instructions on setting that up can be found in [Testing Electron Auto-Update](https://github.com/Expensify/App/blob/main/desktop/README.md#testing-electron-auto-update). Good luck 🙃 #### Local production build the iOS app In order to compile a production iOS build, run `npm run ios-build`, this will generate a `Chat.ipa` in the root directory of this project. diff --git a/__mocks__/@react-navigation/native/index.ts b/__mocks__/@react-navigation/native/index.ts index 9a6680ba0b6e..0b7dda4621ad 100644 --- a/__mocks__/@react-navigation/native/index.ts +++ b/__mocks__/@react-navigation/native/index.ts @@ -3,9 +3,7 @@ import {useIsFocused as realUseIsFocused, useTheme as realUseTheme} from '@react // We only want these mocked for storybook, not jest const useIsFocused: typeof realUseIsFocused = process.env.NODE_ENV === 'test' ? realUseIsFocused : () => true; -// @ts-expect-error as we're mocking this function -const useTheme: typeof realUseTheme = process.env.NODE_ENV === 'test' ? realUseTheme : () => ({}); +const useTheme = process.env.NODE_ENV === 'test' ? realUseTheme : () => ({}); export * from '@react-navigation/core'; -export * from '@react-navigation/native'; export {useIsFocused, useTheme}; diff --git a/__mocks__/react-native-permissions.ts b/__mocks__/react-native-permissions.ts index 67b7db830d94..d98b7f32a611 100644 --- a/__mocks__/react-native-permissions.ts +++ b/__mocks__/react-native-permissions.ts @@ -35,30 +35,30 @@ const requestNotifications: jest.Mock = jest.fn((options: Record notificationOptions.includes(option)) - .reduce((acc: NotificationSettings, option: string) => ({...acc, [option]: true}), { - lockScreen: true, - notificationCenter: true, - }), + .reduce( + (acc: NotificationSettings, option: string) => { + acc[option] = true; + return acc; + }, + { + lockScreen: true, + notificationCenter: true, + }, + ), })); const checkMultiple: jest.Mock = jest.fn((permissions: string[]) => - permissions.reduce( - (acc: ResultsCollection, permission: string) => ({ - ...acc, - [permission]: RESULTS.GRANTED, - }), - {}, - ), + permissions.reduce((acc: ResultsCollection, permission: string) => { + acc[permission] = RESULTS.GRANTED; + return acc; + }, {}), ); const requestMultiple: jest.Mock = jest.fn((permissions: string[]) => - permissions.reduce( - (acc: ResultsCollection, permission: string) => ({ - ...acc, - [permission]: RESULTS.GRANTED, - }), - {}, - ), + permissions.reduce((acc: ResultsCollection, permission: string) => { + acc[permission] = RESULTS.GRANTED; + return acc; + }, {}), ); export { diff --git a/android/app/build.gradle b/android/app/build.gradle index fa05b8058a7c..96c4fa0818e1 100644 --- a/android/app/build.gradle +++ b/android/app/build.gradle @@ -2,12 +2,21 @@ apply plugin: "com.android.application" apply plugin: "org.jetbrains.kotlin.android" apply plugin: "com.facebook.react" apply plugin: "com.google.firebase.firebase-perf" +apply plugin: "fullstory" apply from: project(':react-native-config').projectDir.getPath() + "/dotenv.gradle" /** * This is the configuration block to customize your React Native Android app. * By default you don't need to apply any configuration, just uncomment the lines you need. */ + +/* Fullstory settings */ +fullstory { + org 'o-1WN56P-na1' + enabledVariants 'all' + logcatLevel 'debug' +} + react { /* Folders */ // The root of your project, i.e. where "package.json" lives. Default is '..' @@ -98,8 +107,8 @@ android { minSdkVersion rootProject.ext.minSdkVersion targetSdkVersion rootProject.ext.targetSdkVersion multiDexEnabled rootProject.ext.multiDexEnabled - versionCode 1001046402 - versionName "1.4.64-2" + versionCode 1001047900 + versionName "1.4.79-0" // Supported language variants must be declared here to avoid from being removed during the compilation. // This also helps us to not include unnecessary language variants in the APK. resConfigs "en", "es" @@ -162,7 +171,7 @@ android { signingConfig null // buildTypes take precedence over productFlavors when it comes to the signing configuration, // thus we need to manually set the signing config, so that the e2e uses the debug config again. - // In other words, the signingConfig setting above will be ignored when we build the flavor in release mode. + // In other words, the signingConfig setting above will be ignored when we build the flavor in release mode. productFlavors.all { flavor -> // All release builds should be signed with the release config ... flavor.signingConfig signingConfigs.release diff --git a/android/build.gradle b/android/build.gradle index 10600480d8bb..52c998998ba0 100644 --- a/android/build.gradle +++ b/android/build.gradle @@ -20,6 +20,7 @@ buildscript { repositories { google() mavenCentral() + maven {url "https://maven.fullstory.com"} } dependencies { classpath("com.android.tools.build:gradle") @@ -27,6 +28,9 @@ buildscript { classpath("com.google.gms:google-services:4.3.4") classpath("com.google.firebase:firebase-crashlytics-gradle:2.7.1") classpath("com.google.firebase:perf-plugin:1.4.1") + // Fullstory integration + classpath ("com.fullstory:gradle-plugin-local:1.47.0") + // NOTE: Do not place your application dependencies here; they belong // in the individual module build.gradle files classpath("org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlinVersion") @@ -70,7 +74,7 @@ allprojects { // 'mapbox' is the fixed username for Mapbox's Maven repository. username = 'mapbox' - // The value for password is read from the 'MAPBOX_DOWNLOADS_TOKEN' gradle property. + // The value for password is read from the 'MAPBOX_DOWNLOADS_TOKEN' gradle property. // Run "npm run setup-mapbox-sdk" to set this property in «USER_HOME»/.gradle/gradle.properties // Example gradle.properties entry: diff --git a/assets/animations/Plane.lottie b/assets/animations/Plane.lottie new file mode 100644 index 000000000000..5244cb7bea10 Binary files /dev/null and b/assets/animations/Plane.lottie differ diff --git a/assets/emojis/index.ts b/assets/emojis/index.ts index 02328001674e..d230e8eec2be 100644 --- a/assets/emojis/index.ts +++ b/assets/emojis/index.ts @@ -1,7 +1,5 @@ import type {Locale} from '@src/types/onyx'; import emojis from './common'; -import enEmojis from './en'; -import esEmojis from './es'; import type {Emoji, EmojisList} from './types'; type EmojiTable = Record; @@ -30,10 +28,22 @@ const emojiCodeTableWithSkinTones = emojis.reduce((prev, cur) => { }, {}); const localeEmojis: LocaleEmojis = { - en: enEmojis, - es: esEmojis, + en: undefined, + es: undefined, +}; + +const importEmojiLocale = (locale: Locale) => { + const normalizedLocale = locale.toLowerCase().split('-')[0] as Locale; + if (!localeEmojis[normalizedLocale]) { + const emojiImportPromise = normalizedLocale === 'en' ? import('./en') : import('./es'); + return emojiImportPromise.then((esEmojiModule) => { + // it is needed because in jest test the modules are imported in double nested default object + localeEmojis[normalizedLocale] = esEmojiModule.default.default ? (esEmojiModule.default.default as unknown as EmojisList) : esEmojiModule.default; + }); + } + return Promise.resolve(); }; export default emojis; -export {emojiNameTable, emojiCodeTableWithSkinTones, localeEmojis}; +export {emojiNameTable, emojiCodeTableWithSkinTones, localeEmojis, importEmojiLocale}; export {skinTones, categoryFrequentlyUsed} from './common'; diff --git a/assets/images/all.svg b/assets/images/all.svg new file mode 100644 index 000000000000..f6d9f46fc92e --- /dev/null +++ b/assets/images/all.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/assets/images/arrow-down-long.svg b/assets/images/arrow-down-long.svg new file mode 100644 index 000000000000..cbf6e7e5ad2f --- /dev/null +++ b/assets/images/arrow-down-long.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/assets/images/arrow-right.svg b/assets/images/arrow-right.svg index df13c75ca414..649582544847 100644 --- a/assets/images/arrow-right.svg +++ b/assets/images/arrow-right.svg @@ -1 +1 @@ - \ No newline at end of file + \ No newline at end of file diff --git a/assets/images/arrow-up-long.svg b/assets/images/arrow-up-long.svg new file mode 100644 index 000000000000..13d7a0c2d67e --- /dev/null +++ b/assets/images/arrow-up-long.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/assets/images/avatars/fallback-avatar.svg b/assets/images/avatars/fallback-avatar.svg index 69293d72aed9..4a7fecf967db 100644 --- a/assets/images/avatars/fallback-avatar.svg +++ b/assets/images/avatars/fallback-avatar.svg @@ -1,10 +1 @@ - - - - - - - + \ No newline at end of file diff --git a/assets/images/avatars/group/default-avatar_1.svg b/assets/images/avatars/group/default-avatar_1.svg index 5d97c5bf855b..1edcaa33a8aa 100644 --- a/assets/images/avatars/group/default-avatar_1.svg +++ b/assets/images/avatars/group/default-avatar_1.svg @@ -1,7 +1 @@ - - - - - - - + \ No newline at end of file diff --git a/assets/images/avatars/group/default-avatar_10.svg b/assets/images/avatars/group/default-avatar_10.svg index 12c9dd76ae31..62e818cb3e45 100644 --- a/assets/images/avatars/group/default-avatar_10.svg +++ b/assets/images/avatars/group/default-avatar_10.svg @@ -1,7 +1 @@ - - - - - - - + \ No newline at end of file diff --git a/assets/images/avatars/group/default-avatar_11.svg b/assets/images/avatars/group/default-avatar_11.svg index 97f17f30f3a7..2f976b05519d 100644 --- a/assets/images/avatars/group/default-avatar_11.svg +++ b/assets/images/avatars/group/default-avatar_11.svg @@ -1,7 +1 @@ - - - - - - - + \ No newline at end of file diff --git a/assets/images/avatars/group/default-avatar_12.svg b/assets/images/avatars/group/default-avatar_12.svg index f917fb136582..c29992aa1793 100644 --- a/assets/images/avatars/group/default-avatar_12.svg +++ b/assets/images/avatars/group/default-avatar_12.svg @@ -1,7 +1 @@ - - - - - - - + \ No newline at end of file diff --git a/assets/images/avatars/group/default-avatar_13.svg b/assets/images/avatars/group/default-avatar_13.svg index 9e59fb9123a5..5f6b69f01fe3 100644 --- a/assets/images/avatars/group/default-avatar_13.svg +++ b/assets/images/avatars/group/default-avatar_13.svg @@ -1,7 +1 @@ - - - - - - - + \ No newline at end of file diff --git a/assets/images/avatars/group/default-avatar_14.svg b/assets/images/avatars/group/default-avatar_14.svg index ca071e488416..27096ffd77d7 100644 --- a/assets/images/avatars/group/default-avatar_14.svg +++ b/assets/images/avatars/group/default-avatar_14.svg @@ -1,7 +1 @@ - - - - - - - + \ No newline at end of file diff --git a/assets/images/avatars/group/default-avatar_15.svg b/assets/images/avatars/group/default-avatar_15.svg index f227cc0717be..7cae7b1e6562 100644 --- a/assets/images/avatars/group/default-avatar_15.svg +++ b/assets/images/avatars/group/default-avatar_15.svg @@ -1,7 +1 @@ - - - - - - - + \ No newline at end of file diff --git a/assets/images/avatars/group/default-avatar_16.svg b/assets/images/avatars/group/default-avatar_16.svg index efbb85f0b13d..1c02725ba669 100644 --- a/assets/images/avatars/group/default-avatar_16.svg +++ b/assets/images/avatars/group/default-avatar_16.svg @@ -1,7 +1 @@ - - - - - - - + \ No newline at end of file diff --git a/assets/images/avatars/group/default-avatar_17.svg b/assets/images/avatars/group/default-avatar_17.svg index 25c015c595ca..58a5014fae68 100644 --- a/assets/images/avatars/group/default-avatar_17.svg +++ b/assets/images/avatars/group/default-avatar_17.svg @@ -1,7 +1 @@ - - - - - - - + \ No newline at end of file diff --git a/assets/images/avatars/group/default-avatar_18.svg b/assets/images/avatars/group/default-avatar_18.svg index a58ee6e66eff..43eeffb3db8d 100644 --- a/assets/images/avatars/group/default-avatar_18.svg +++ b/assets/images/avatars/group/default-avatar_18.svg @@ -1,7 +1 @@ - - - - - - - + \ No newline at end of file diff --git a/assets/images/avatars/group/default-avatar_2.svg b/assets/images/avatars/group/default-avatar_2.svg index ff1cc3e6dd2d..f67a49d28cd2 100644 --- a/assets/images/avatars/group/default-avatar_2.svg +++ b/assets/images/avatars/group/default-avatar_2.svg @@ -1,7 +1 @@ - - - - - - - + \ No newline at end of file diff --git a/assets/images/avatars/group/default-avatar_3.svg b/assets/images/avatars/group/default-avatar_3.svg index dde31b5d02a0..471d3a348b4a 100644 --- a/assets/images/avatars/group/default-avatar_3.svg +++ b/assets/images/avatars/group/default-avatar_3.svg @@ -1,7 +1 @@ - - - - - - - + \ No newline at end of file diff --git a/assets/images/avatars/group/default-avatar_4.svg b/assets/images/avatars/group/default-avatar_4.svg index f6d02801bc6b..46e22d28b6df 100644 --- a/assets/images/avatars/group/default-avatar_4.svg +++ b/assets/images/avatars/group/default-avatar_4.svg @@ -1,7 +1 @@ - - - - - - - + \ No newline at end of file diff --git a/assets/images/avatars/group/default-avatar_5.svg b/assets/images/avatars/group/default-avatar_5.svg index fdabd36e2058..a81471170e23 100644 --- a/assets/images/avatars/group/default-avatar_5.svg +++ b/assets/images/avatars/group/default-avatar_5.svg @@ -1,7 +1 @@ - - - - - - - + \ No newline at end of file diff --git a/assets/images/avatars/group/default-avatar_6.svg b/assets/images/avatars/group/default-avatar_6.svg index 6f1c6b80eda6..71da5e5631f3 100644 --- a/assets/images/avatars/group/default-avatar_6.svg +++ b/assets/images/avatars/group/default-avatar_6.svg @@ -1,7 +1 @@ - - - - - - - + \ No newline at end of file diff --git a/assets/images/avatars/group/default-avatar_7.svg b/assets/images/avatars/group/default-avatar_7.svg index 62d9a8b76bb8..080426ca0454 100644 --- a/assets/images/avatars/group/default-avatar_7.svg +++ b/assets/images/avatars/group/default-avatar_7.svg @@ -1,7 +1 @@ - - - - - - - + \ No newline at end of file diff --git a/assets/images/avatars/group/default-avatar_8.svg b/assets/images/avatars/group/default-avatar_8.svg index 206b10c2322b..b6b2d98579eb 100644 --- a/assets/images/avatars/group/default-avatar_8.svg +++ b/assets/images/avatars/group/default-avatar_8.svg @@ -1,7 +1 @@ - - - - - - - + \ No newline at end of file diff --git a/assets/images/avatars/group/default-avatar_9.svg b/assets/images/avatars/group/default-avatar_9.svg index ffbe02ce57e8..14885d4c401c 100644 --- a/assets/images/avatars/group/default-avatar_9.svg +++ b/assets/images/avatars/group/default-avatar_9.svg @@ -1,7 +1 @@ - - - - - - - + \ No newline at end of file diff --git a/assets/images/back-left.svg b/assets/images/back-left.svg index 51164100ff59..2c709401916f 100644 --- a/assets/images/back-left.svg +++ b/assets/images/back-left.svg @@ -1 +1 @@ - \ No newline at end of file + \ No newline at end of file diff --git a/assets/images/check-circle.svg b/assets/images/check-circle.svg new file mode 100644 index 000000000000..c13b83cbf281 --- /dev/null +++ b/assets/images/check-circle.svg @@ -0,0 +1,13 @@ + + + + + + \ No newline at end of file diff --git a/assets/images/checkmark-circle.svg b/assets/images/checkmark-circle.svg new file mode 100644 index 000000000000..3497548bc1bc --- /dev/null +++ b/assets/images/checkmark-circle.svg @@ -0,0 +1,3 @@ + + + diff --git a/assets/images/coins.svg b/assets/images/coins.svg new file mode 100644 index 000000000000..164fa84388f5 --- /dev/null +++ b/assets/images/coins.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/assets/images/comment-bubbles.svg b/assets/images/comment-bubbles.svg new file mode 100644 index 000000000000..1277b8958c94 --- /dev/null +++ b/assets/images/comment-bubbles.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/assets/images/connection-complete.svg b/assets/images/connection-complete.svg new file mode 100644 index 000000000000..d864d9a33626 --- /dev/null +++ b/assets/images/connection-complete.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/assets/images/credit-card-hourglass.svg b/assets/images/credit-card-hourglass.svg new file mode 100644 index 000000000000..28ffe766b597 --- /dev/null +++ b/assets/images/credit-card-hourglass.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/assets/images/crosshair.svg b/assets/images/crosshair.svg new file mode 100644 index 000000000000..357faab49178 --- /dev/null +++ b/assets/images/crosshair.svg @@ -0,0 +1,23 @@ + + + + + + + + + + + + + + + + diff --git a/assets/images/document-plus.svg b/assets/images/document-plus.svg index cce2e3027cea..729bc98d4f8a 100644 --- a/assets/images/document-plus.svg +++ b/assets/images/document-plus.svg @@ -1,5 +1 @@ - - - - - + \ No newline at end of file diff --git a/assets/images/document-slash.svg b/assets/images/document-slash.svg index ebb183142e40..e8a0ff20702e 100644 --- a/assets/images/document-slash.svg +++ b/assets/images/document-slash.svg @@ -1 +1 @@ - + \ No newline at end of file diff --git a/assets/images/integrationicons/qbo-icon-square.svg b/assets/images/integrationicons/qbo-icon-square.svg index a8ce3468ffbf..e297b597f980 100644 --- a/assets/images/integrationicons/qbo-icon-square.svg +++ b/assets/images/integrationicons/qbo-icon-square.svg @@ -1,14 +1 @@ - - - - - - - + \ No newline at end of file diff --git a/assets/images/integrationicons/xero-icon-square.svg b/assets/images/integrationicons/xero-icon-square.svg index 94b79bb3533d..43774919c92c 100644 --- a/assets/images/integrationicons/xero-icon-square.svg +++ b/assets/images/integrationicons/xero-icon-square.svg @@ -1,32 +1 @@ - - - - - - - - - - - + \ No newline at end of file diff --git a/assets/images/invoice-generic.svg b/assets/images/invoice-generic.svg new file mode 100644 index 000000000000..251918c4cff4 --- /dev/null +++ b/assets/images/invoice-generic.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/assets/images/money-waving.svg b/assets/images/money-waving.svg index 5242e31092a0..e68744d595be 100644 --- a/assets/images/money-waving.svg +++ b/assets/images/money-waving.svg @@ -1,81 +1 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + \ No newline at end of file diff --git a/assets/images/new-expensify-adhoc.svg b/assets/images/new-expensify-adhoc.svg index b3dd92fbbaae..8da6331c8c94 100644 --- a/assets/images/new-expensify-adhoc.svg +++ b/assets/images/new-expensify-adhoc.svg @@ -1,31 +1 @@ - - - - - - - - - - - - - - - + \ No newline at end of file diff --git a/assets/images/new-expensify-dev.svg b/assets/images/new-expensify-dev.svg index 316da6b5aa4d..fcb371f586b6 100644 --- a/assets/images/new-expensify-dev.svg +++ b/assets/images/new-expensify-dev.svg @@ -1,27 +1 @@ - - - - - - - - - - - - - - - + \ No newline at end of file diff --git a/assets/images/new-expensify-stg.svg b/assets/images/new-expensify-stg.svg index 1a1994c7a9fd..d536257fc880 100644 --- a/assets/images/new-expensify-stg.svg +++ b/assets/images/new-expensify-stg.svg @@ -1,35 +1 @@ - - - - - - - - - - - - - - - - - - - - - + \ No newline at end of file diff --git a/assets/images/play.svg b/assets/images/play.svg index cb781459e44e..98a8c00520fc 100644 --- a/assets/images/play.svg +++ b/assets/images/play.svg @@ -1 +1 @@ - \ No newline at end of file + \ No newline at end of file diff --git a/assets/images/qrcode.svg b/assets/images/qrcode.svg index 42c49c958246..47d61d7dd47c 100644 --- a/assets/images/qrcode.svg +++ b/assets/images/qrcode.svg @@ -1,14 +1 @@ - - - - - - - - - - - - + \ No newline at end of file diff --git a/assets/images/receipt-plus.svg b/assets/images/receipt-plus.svg new file mode 100644 index 000000000000..ca4d96b3dfa5 --- /dev/null +++ b/assets/images/receipt-plus.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/assets/images/receipt-scan.svg b/assets/images/receipt-scan.svg new file mode 100644 index 000000000000..f7c164c948c8 --- /dev/null +++ b/assets/images/receipt-scan.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/assets/images/simple-illustrations/simple-illustration__abacus.svg b/assets/images/simple-illustrations/simple-illustration__abacus.svg index df94ab653982..6dac0e9009b1 100644 --- a/assets/images/simple-illustrations/simple-illustration__abacus.svg +++ b/assets/images/simple-illustrations/simple-illustration__abacus.svg @@ -1,43 +1 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + \ No newline at end of file diff --git a/assets/images/simple-illustrations/simple-illustration__accounting.svg b/assets/images/simple-illustrations/simple-illustration__accounting.svg index f7634141e966..3213b4f93856 100644 --- a/assets/images/simple-illustrations/simple-illustration__accounting.svg +++ b/assets/images/simple-illustrations/simple-illustration__accounting.svg @@ -1,32 +1 @@ - + \ No newline at end of file diff --git a/assets/images/simple-illustrations/simple-illustration__alert.svg b/assets/images/simple-illustrations/simple-illustration__alert.svg new file mode 100644 index 000000000000..cbf70b7655a7 --- /dev/null +++ b/assets/images/simple-illustrations/simple-illustration__alert.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/assets/images/simple-illustrations/simple-illustration__binoculars.svg b/assets/images/simple-illustrations/simple-illustration__binoculars.svg index 381be8988873..5abacd359464 100644 --- a/assets/images/simple-illustrations/simple-illustration__binoculars.svg +++ b/assets/images/simple-illustrations/simple-illustration__binoculars.svg @@ -1,50 +1 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + \ No newline at end of file diff --git a/assets/images/simple-illustrations/simple-illustration__car-ice.svg b/assets/images/simple-illustrations/simple-illustration__car-ice.svg index ba2b79bca6aa..9da1b844c101 100644 --- a/assets/images/simple-illustrations/simple-illustration__car-ice.svg +++ b/assets/images/simple-illustrations/simple-illustration__car-ice.svg @@ -1,53 +1 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + \ No newline at end of file diff --git a/assets/images/simple-illustrations/simple-illustration__car.svg b/assets/images/simple-illustrations/simple-illustration__car.svg index 2d420be6c3a9..9da1b844c101 100644 --- a/assets/images/simple-illustrations/simple-illustration__car.svg +++ b/assets/images/simple-illustrations/simple-illustration__car.svg @@ -1,25 +1 @@ - + \ No newline at end of file diff --git a/assets/images/simple-illustrations/simple-illustration__checkmarkcircle.svg b/assets/images/simple-illustrations/simple-illustration__checkmarkcircle.svg new file mode 100644 index 000000000000..a96a7e5dc0af --- /dev/null +++ b/assets/images/simple-illustrations/simple-illustration__checkmarkcircle.svg @@ -0,0 +1,21 @@ + + + + + + + + + \ No newline at end of file diff --git a/assets/images/simple-illustrations/simple-illustration__coins.svg b/assets/images/simple-illustrations/simple-illustration__coins.svg index 5350886402c6..5caa1c0635d5 100644 --- a/assets/images/simple-illustrations/simple-illustration__coins.svg +++ b/assets/images/simple-illustrations/simple-illustration__coins.svg @@ -1,26 +1 @@ - + \ No newline at end of file diff --git a/assets/images/simple-illustrations/simple-illustration__company-card.svg b/assets/images/simple-illustrations/simple-illustration__company-card.svg index 4121bbeeb205..1f4e43dbc047 100644 --- a/assets/images/simple-illustrations/simple-illustration__company-card.svg +++ b/assets/images/simple-illustrations/simple-illustration__company-card.svg @@ -1,38 +1 @@ - - - - - - - - - - - - - - - - - - - - - - + \ No newline at end of file diff --git a/assets/images/simple-illustrations/simple-illustration__lightbulb.svg b/assets/images/simple-illustrations/simple-illustration__lightbulb.svg index 1dc359764147..62a9cb0c3b76 100644 --- a/assets/images/simple-illustrations/simple-illustration__lightbulb.svg +++ b/assets/images/simple-illustrations/simple-illustration__lightbulb.svg @@ -1,33 +1 @@ - - - - - - - - - - - - - - - - - - - + \ No newline at end of file diff --git a/assets/images/simple-illustrations/simple-illustration__pencil.svg b/assets/images/simple-illustrations/simple-illustration__pencil.svg index 8d9f06991612..d3eaf8771021 100644 --- a/assets/images/simple-illustrations/simple-illustration__pencil.svg +++ b/assets/images/simple-illustrations/simple-illustration__pencil.svg @@ -1,20 +1 @@ - + \ No newline at end of file diff --git a/assets/images/simple-illustrations/simple-illustration__piggybank.svg b/assets/images/simple-illustrations/simple-illustration__piggybank.svg index a9cf2b02c5dc..ab1f73113f18 100644 --- a/assets/images/simple-illustrations/simple-illustration__piggybank.svg +++ b/assets/images/simple-illustrations/simple-illustration__piggybank.svg @@ -1,50 +1 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + \ No newline at end of file diff --git a/assets/images/simple-illustrations/simple-illustration__receiptupload.svg b/assets/images/simple-illustrations/simple-illustration__receiptupload.svg index b8fe5101715f..efff624f481f 100644 --- a/assets/images/simple-illustrations/simple-illustration__receiptupload.svg +++ b/assets/images/simple-illustrations/simple-illustration__receiptupload.svg @@ -1,22 +1 @@ - - - - - - - - - - - + \ No newline at end of file diff --git a/assets/images/simple-illustrations/simple-illustration__sendmoney.svg b/assets/images/simple-illustrations/simple-illustration__sendmoney.svg new file mode 100644 index 000000000000..80393e3c30cf --- /dev/null +++ b/assets/images/simple-illustrations/simple-illustration__sendmoney.svg @@ -0,0 +1,72 @@ + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/assets/images/simple-illustrations/simple-illustration__splitbill.svg b/assets/images/simple-illustrations/simple-illustration__splitbill.svg index dfed7535ee90..1390a7cf9205 100644 --- a/assets/images/simple-illustrations/simple-illustration__splitbill.svg +++ b/assets/images/simple-illustrations/simple-illustration__splitbill.svg @@ -1,55 +1 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + \ No newline at end of file diff --git a/assets/images/simple-illustrations/simple-illustration__tag.svg b/assets/images/simple-illustrations/simple-illustration__tag.svg index 0cac51679a5e..0a93014d11b3 100644 --- a/assets/images/simple-illustrations/simple-illustration__tag.svg +++ b/assets/images/simple-illustrations/simple-illustration__tag.svg @@ -1,33 +1 @@ - - - - - - - - - - - - - - - - - - - + \ No newline at end of file diff --git a/assets/images/simple-illustrations/simple-illustration__teachers-unite.svg b/assets/images/simple-illustrations/simple-illustration__teachers-unite.svg index b4edd9513722..27ce709889dd 100644 --- a/assets/images/simple-illustrations/simple-illustration__teachers-unite.svg +++ b/assets/images/simple-illustrations/simple-illustration__teachers-unite.svg @@ -1,49 +1 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + \ No newline at end of file diff --git a/assets/images/simple-illustrations/simple-illustration__workflows.svg b/assets/images/simple-illustrations/simple-illustration__workflows.svg index b684c58126f7..c11d2663997f 100644 --- a/assets/images/simple-illustrations/simple-illustration__workflows.svg +++ b/assets/images/simple-illustrations/simple-illustration__workflows.svg @@ -1,153 +1 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + \ No newline at end of file diff --git a/assets/images/stopwatch.svg b/assets/images/stopwatch.svg index 0f26af219e04..b8ca46fd1fa1 100644 --- a/assets/images/stopwatch.svg +++ b/assets/images/stopwatch.svg @@ -1 +1 @@ - \ No newline at end of file + \ No newline at end of file diff --git a/assets/images/suitcase.svg b/assets/images/suitcase.svg new file mode 100644 index 000000000000..452c44f73e22 --- /dev/null +++ b/assets/images/suitcase.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/assets/images/tag.svg b/assets/images/tag.svg index f5e13b8135cb..f25bcbe47f71 100644 --- a/assets/images/tag.svg +++ b/assets/images/tag.svg @@ -1,12 +1 @@ - - - - - - + \ No newline at end of file diff --git a/assets/images/tax.svg b/assets/images/tax.svg deleted file mode 100644 index aa3c68e72ea8..000000000000 --- a/assets/images/tax.svg +++ /dev/null @@ -1,8 +0,0 @@ - - - - - - - - diff --git a/assets/images/thread.svg b/assets/images/thread.svg index 3b8f334fafdd..9f01ce7b2c06 100644 --- a/assets/images/thread.svg +++ b/assets/images/thread.svg @@ -1,3 +1 @@ - - - + \ No newline at end of file diff --git a/assets/images/x-circle.svg b/assets/images/x-circle.svg new file mode 100644 index 000000000000..5fa5f3741567 --- /dev/null +++ b/assets/images/x-circle.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/babel.config.js b/babel.config.js index 9f8b7a711d78..060bc0313950 100644 --- a/babel.config.js +++ b/babel.config.js @@ -10,6 +10,9 @@ const defaultPlugins = [ '@babel/transform-runtime', '@babel/plugin-proposal-class-properties', + // This will serve to map the classes correctly in FullStory + '@fullstory/babel-plugin-annotate-react', + // We use `transform-class-properties` for transforming ReactNative libraries and do not use it for our own // source code transformation as we do not use class property assignment. 'transform-class-properties', @@ -35,6 +38,17 @@ const metro = { ['@babel/plugin-proposal-private-property-in-object', {loose: true}], // The reanimated babel plugin needs to be last, as stated here: https://docs.swmansion.com/react-native-reanimated/docs/fundamentals/installation 'react-native-reanimated/plugin', + + /* Fullstory */ + '@fullstory/react-native', + [ + '@fullstory/babel-plugin-annotate-react', + { + native: true, + setFSTagName: true, + }, + ], + // Import alias for native devices [ 'module-resolver', diff --git a/config/webpack/webpack.common.ts b/config/webpack/webpack.common.ts index 7cafafca9973..9d397b9557a3 100644 --- a/config/webpack/webpack.common.ts +++ b/config/webpack/webpack.common.ts @@ -98,7 +98,7 @@ const getCommonConfiguration = ({file = '.env', platform = 'web'}: Environment): {from: 'web/apple-touch-icon.png'}, {from: 'assets/images/expensify-app-icon.svg'}, {from: 'web/manifest.json'}, - {from: 'web/gtm.js'}, + {from: 'web/thirdPartyScripts.js'}, {from: 'assets/css', to: 'css'}, {from: 'assets/fonts/web', to: 'fonts'}, {from: 'assets/sounds', to: 'sounds'}, diff --git a/config/webpack/webpack.dev.ts b/config/webpack/webpack.dev.ts index 8f32a2d95c99..7a196da6b691 100644 --- a/config/webpack/webpack.dev.ts +++ b/config/webpack/webpack.dev.ts @@ -21,16 +21,12 @@ const getConfiguration = (environment: Environment): Promise => process.env.USE_WEB_PROXY === 'false' ? {} : { - proxy: { - // eslint-disable-next-line @typescript-eslint/naming-convention - '/api': 'http://[::1]:9000', - // eslint-disable-next-line @typescript-eslint/naming-convention - '/staging': 'http://[::1]:9000', - // eslint-disable-next-line @typescript-eslint/naming-convention - '/chat-attachments': 'http://[::1]:9000', - // eslint-disable-next-line @typescript-eslint/naming-convention - '/receipts': 'http://[::1]:9000', - }, + proxy: [ + { + context: ['/api', '/staging', '/chat-attachments', '/receipts'], + target: 'http://[::1]:9000', + }, + ], }; const baseConfig = getCommonConfiguration(environment); diff --git a/contributingGuides/ACCESSIBILITY.md b/contributingGuides/ACCESSIBILITY.md index b94cbf3087c8..ff73eaf2942e 100644 --- a/contributingGuides/ACCESSIBILITY.md +++ b/contributingGuides/ACCESSIBILITY.md @@ -19,9 +19,9 @@ When implementing pressable components, it's essential to create accessible flow - ensure that after performing press focus is set on the correct next element - this is especially important for keyboard users who rely on focus to navigate the app. All Pressable components have a `nextFocusRef` prop that can be used to set the next focusable element after the pressable component. This prop accepts a ref to the next focusable element. For example, if you have a button that opens a modal, you can set the next focus to the first focusable element in the modal. This way, when the user presses the button, focus will be set on the first focusable element in the modal, and the user can continue navigating the modal using the keyboard. -- size of any pressable component should be at least 44x44dp. This is the minimum size recommended by Apple and Google for touch targets. If the pressable component is smaller than `44x44dp`, it will be difficult for users with motor disabilities to interact with it. Pressable components have a `autoHitSlop` prop that can be used to automatically increase the size of the pressable component to `44x44dp`. This prop accepts a boolean value. If set to true, the pressable component will automatically increase its touchable size to 44x44dp. If set to false, the pressable component will not increase its size. By default, this prop is set to false. +- size of any pressable component should be at least 44x44dp. This is the minimum size recommended by Apple and Google for touch targets. If the pressable component is smaller than `44x44dp`, it will be difficult for users with motor disabilities to interact with it. Pressable components have a `autoHitSlop` prop that can be used to automatically increase the size of the pressable component to `44x44dp`. This prop accepts a boolean value. If set to true, the pressable component will automatically increase its touchable size to `44x44dp`. If set to false, the pressable component will not increase its size. By default, this prop is set to false. -- ensure that the pressable component has a label and hint. This is especially important for users with visual disabilities who rely on screen readers to navigate the app. All Pressable components have a `accessibilitylabel` prop that can be used to set the label of the pressable component. This prop accepts a string value. All Pressable components also have a `accessibilityHint` prop that can be used to set the hint of the pressable component. This prop accepts a string value. The accessibilityHint prop is optional. If not set, the pressable component will fallback to the accessibilityLabel prop. For example, if you have a button that opens a modal, you can set the accessibilityLabel to "Open modal" and the accessibilityHint to "Opens a modal with more information". This way, when the user focuses on the button, the screen reader will read "Open modal. Opens a modal with more information". This will help the user understand what the button does and what to expect after pressing it. +- ensure that the pressable component has a label and hint. This is especially important for users with visual disabilities who rely on screen readers to navigate the app. All Pressable components have a `accessibilitylabel` prop that can be used to set the label of the pressable component. This prop accepts a string value. All Pressable components also have a `accessibilityHint` prop that can be used to set the hint of the pressable component. This prop accepts a string value. The `accessibilityHint` prop is optional. If not set, the pressable component will fallback to the `accessibilityLabel` prop. For example, if you have a button that opens a modal, you can set the `accessibilityLabel` to "Open modal" and the `accessibilityHint` to "Opens a modal with more information". This way, when the user focuses on the button, the screen reader will read "Open modal. Opens a modal with more information". This will help the user understand what the button does and what to expect after pressing it. - the `enableInScreenReaderStates` prop proves invaluable when aiming to enhance the accessibility of clickable elements, particularly when desiring to enlarge the clickable area of a component, such as an entire row. This can be especially useful, for instance, when dealing with tables where only a small portion of a row, like a checkbox, might traditionally trigger an action. By employing this prop, developers can ensure that the entirety of a designated component, in this case a row, is made accessible to users employing screen readers. This creates a more inclusive user experience, allowing individuals relying on screen readers to interact with the component effortlessly. For instance, in a table, using this prop on a row component enables users to click anywhere within the row to trigger an action, significantly improving accessibility and user-friendliness. diff --git a/contributingGuides/API.md b/contributingGuides/API.md index aea684417e41..cf17b4093244 100644 --- a/contributingGuides/API.md +++ b/contributingGuides/API.md @@ -8,11 +8,11 @@ These are best practices related to the current API used for App. - Data is pushed to the client and put straight into Onyx by low-level libraries. - Clients should be kept up-to-date with many small incremental changes to data. - Creating data needs to be optimistic on every connection (offline, slow 3G, etc), eg. `RequestMoney` or `SplitBill` should work without waiting for a server response. -- For new objects created from the client (reports, reportActions, policies) we're going to generate a random string ID immediately on the client, rather than needing to wait for the server to give us an ID for the created object. +- For new objects created from the client (reports, reportActions, policies), we're going to generate a random string ID immediately on the client, rather than needing to wait for the server to give us an ID for the created object. - This client-generated ID will become the primary key for that record in the database. This will provide more offline functionality than was previously possible. ## Response Handling -When the web server responds to an API call the response is sent to the server in one of two ways. +When the web server responds to an API call, the response is sent to the server in one of two ways. 1. **HTTPS Response** - Data that is returned with the HTTPS response is only sent to the client that initiated the request. The network library will look for any `onyxData` in the response and send it straight to `Onyx.update(response.onyxData)`. @@ -20,31 +20,38 @@ When the web server responds to an API call the response is sent to the server i 1. **Pusher Event** (web socket) - Data returned with a Pusher event is sent to all currently connected clients for the user that made the request, as well as any other necessary participants (eg. like other people in the chat) Pusher listens for an `onyxApiUpdate` event and sends the data straight to `Onyx.update(pushJSON)`. + ### READ Responses This is a response that returns data from the database. -A READ response is very specific to the client making the request, so it's data is returned with the **HTTPS Response**. This prevents a lot of unnecessary data from being sent to other clients that will never use it. +A READ response is very specific to the client making the request, so its data is returned with the **HTTPS Response**. This prevents a lot of unnecessary data from being sent to other clients that will never use it. In PHP, the response is added like this: + ```php $response['onyxData'][] = blahblahblah; ``` + The data will be returned with the HTTPS response. + ### WRITE Responses This response happens when new data is created in the database. New data (`jsonCode===200`) should be sent to all connected clients so a **Pusher Event** is used to update another currently connected clients with the new data that was created. In PHP, the response is added like this: + ```php $onyxUpdate[] = blahblahblah; ``` + The data will automatically be sent to the user via Pusher. #### WRITE Response Errors When there is an error on a WRITE response (`jsonCode!==200`), the error must come back to the client on the HTTPS response. The error is only relevant to the client that made the request and it wouldn't make sense to send it out to all connected clients. Error messages should be returned and stored as an object under the `errors` property, keyed by an integer [microtime](https://github.com/Expensify/Web-Expensify/blob/25d056c9c531ea7f12c9bf3283ec554dd5d1d316/lib/Onyx.php#L148-L154). It's also common to store errors keyed by microtime under `errorFields.fieldName`. Use this format when error messages should be saved on a general object but are only relevant to a specific field / key on the object. If absolutely needed, additional error properties can be stored under other, more specific fields that sit at the same level as `errors`: + ```php [ 'onyxMethod' => Onyx::METHOD_MERGE, diff --git a/contributingGuides/APPLE_GOOGLE_SIGNIN.md b/contributingGuides/APPLE_GOOGLE_SIGNIN.md index e6b999b7cb01..cc3e256be399 100644 --- a/contributingGuides/APPLE_GOOGLE_SIGNIN.md +++ b/contributingGuides/APPLE_GOOGLE_SIGNIN.md @@ -41,7 +41,7 @@ The redirect URI must match a URI in the Google or Apple client ID configuration Pop-up mode opens a pop-up window to show the third-party sign-in form. But it also changes how tokens are given to the client app. Instead of an HTTPS request, they are returned by the JS libraries in memory, either via a callback (Google) or a promise (Apple). -Apple and Google both check that the client app is running on an allowed domain. The sign-in process will fail otherwise. Google allows localhost, but Apple does not, and so testing Apple in development environments requires hosting the client app on a domain that the Apple client ID (or "service ID", in Apple's case) has been configured with. +Apple and Google both check that the client app is running on an allowed domain. The sign-in process will fail otherwise. Google allows `localhost`, but Apple does not, and so testing Apple in development environments requires hosting the client app on a domain that the Apple client ID (or "service ID", in Apple's case) has been configured with. In the case of Apple, sometimes it will silently fail at the very end of the sign-in process, where the only sign that something is wrong is that the pop-up fails to close. In this case, it's very likely that configuration mismatch is the issue. @@ -96,6 +96,79 @@ These steps are covered in more detail in the "testing" section below. Due to some technical constraints, Apple and Google sign-in may require additional configuration to be able to work in the development environment as expected. This document describes any additional steps for each platform. +## Show Apple / Google SSO buttons development environment + +The Apple/Google Sign In button renders differently in development mode. To prevent confusion +for developers about a possible regression, we decided to not render third party buttons in +development mode. + +To re-enable the SSO buttons in development mode, remove this [condition](https://github.com/Expensify/App/blob/c2a718c9100e704c89ad9564301348bc53a49777/src/pages/signin/LoginForm/BaseLoginForm.tsx#L300) so that we always render the SSO button components: + +```diff +diff --git a/src/pages/signin/LoginForm/BaseLoginForm.tsx b/src/pages/signin/LoginForm/BaseLoginForm.tsx +index 4286a26033..850f8944ca 100644 +--- a/src/pages/signin/LoginForm/BaseLoginForm.tsx ++++ b/src/pages/signin/LoginForm/BaseLoginForm.tsx +@@ -288,7 +288,7 @@ function BaseLoginForm({account, credentials, closeAccount, blurOnSubmit = false + // for developers about possible regressions, we won't render buttons in development mode. + // For more information about these differences and how to test in development mode, + // see`Expensify/App/contributingGuides/APPLE_GOOGLE_SIGNIN.md` +- CONFIG.ENVIRONMENT !== CONST.ENVIRONMENT.DEV && ( ++ ( + + `Swift Default Apps` => `URI Schemes` => `new-expensify` and select `New Expensify.app` +4. Note that a dev build of the desktop app will not work. You'll create and install a local staging build: + 1. Update `build-desktop.sh` replacing `--publish always` with `--publish never`. + 2. Run `npm run desktop-build-staging` and install the locally-generated desktop app to test. +5. (Google only) apply the following diff: + + ```diff + diff --git a/src/components/DeeplinkWrapper/index.website.tsx b/src/components/DeeplinkWrapper/index.website.tsx + index 765fbab038..4318528b4c 100644 + --- a/src/components/DeeplinkWrapper/index.website.tsx + +++ b/src/components/DeeplinkWrapper/index.website.tsx + @@ -63,14 +63,7 @@ function DeeplinkWrapper({children, isAuthenticated, autoAuthState}: DeeplinkWra + const isUnsupportedDeeplinkRoute = routeRegex.test(window.location.pathname); + + // Making a few checks to exit early before checking authentication status + - if ( + - !isMacOSWeb() || + - isUnsupportedDeeplinkRoute || + - hasShownPrompt || + - CONFIG.ENVIRONMENT === CONST.ENVIRONMENT.DEV || + - autoAuthState === CONST.AUTO_AUTH_STATE.NOT_STARTED || + - Session.isAnonymousUser() + - ) { + + if (!isMacOSWeb() || isUnsupportedDeeplinkRoute || hasShownPrompt || autoAuthState === CONST.AUTO_AUTH_STATE.NOT_STARTED || Session.isAnonymousUser()) { + return; + } + // We want to show the prompt immediately if the user is already authenticated. + diff --git a/src/libs/Navigation/linkingConfig/prefixes.ts b/src/libs/Navigation/linkingConfig/prefixes.ts + index ca2da6f56b..2c191598f0 100644 + --- a/src/libs/Navigation/linkingConfig/prefixes.ts + +++ b/src/libs/Navigation/linkingConfig/prefixes.ts + @@ -8,6 +8,7 @@ const prefixes: LinkingOptions['prefixes'] = [ + 'https://www.expensify.cash', + 'https://staging.expensify.cash', + 'https://dev.new.expensify.com', + + 'http://localhost', + CONST.NEW_EXPENSIFY_URL, + CONST.STAGING_NEW_EXPENSIFY_URL, + ]; + ``` + +6. Run `npm run web` + ## Apple #### Port requirements @@ -125,24 +198,24 @@ If you need to check that you received the correct data, check it on [jwt.io](ht Hardcode this token into `Session.beginAppleSignIn`, and but also verify a valid token was passed into the function, for example: -``` +```js function beginAppleSignIn(idToken) { -+ // Show that a token was passed in, without logging the token, for privacy -+ window.alert(`ORIGINAL ID TOKEN LENGTH: ${idToken.length}`); -+ const hardcodedToken = '...'; + // Show that a token was passed in, without logging the token, for privacy + window.alert(`ORIGINAL ID TOKEN LENGTH: ${idToken.length}`); + const hardcodedToken = '...'; const {optimisticData, successData, failureData} = signInAttemptState(); -+ API.write('SignInWithApple', {idToken: hardcodedToken}, {optimisticData, successData, failureData}); -- API.write('SignInWithApple', {idToken}, {optimisticData, successData, failureData}); + API.write('SignInWithApple', {idToken: hardcodedToken}, {optimisticData, successData, failureData}); + API.write('SignInWithApple', {idToken}, {optimisticData, successData, failureData}); } ``` #### Configure the SSH tunneling -You can use any SSH tunneling service that allows you to configure custom subdomains so that we have a consistent address to use. We'll use ngrok in these examples, but ngrok requires a paid account for this. If you need a free option, try serveo.net. +You can use any SSH tunneling service that allows you to configure custom subdomains so that we have a consistent address to use. We'll use ngrok in these examples, but ngrok requires a paid account for this. If you need a free option, try [serveo.net](https://serveo.net). After you've set ngrok up to be able to run on your machine (requires configuring a key with the command line tool, instructions provided by the ngrok website after you create an account), test hosting the web app on a custom subdomain. This example assumes the development web app is running at `dev.new.expensify.com:8082`: -``` +```shell ngrok http 8082 --host-header="dev.new.expensify.com:8082" --subdomain=mysubdomain ``` @@ -183,7 +256,7 @@ Desktop will require the same configuration, with these additional steps: #### Configure web app URL in .env -Add `NEW_EXPENSIFY_URL` to .env, and set it to the HTTPS URL where the web app can be found, for example: +Add `NEW_EXPENSIFY_URL` to `.env`, and set it to the HTTPS URL where the web app can be found, for example: ``` NEW_EXPENSIFY_URL=https://subdomain.ngrok-free.app @@ -193,57 +266,11 @@ This is required because the desktop app needs to know the address of the web ap Note that changing this value to a domain that isn't configured for use with Expensify will cause Android to break, as it is still using the real client ID, but now has an incorrect value for `redirectURI`. -#### Set Environment to something other than "Development" - -The DeepLinkWrapper component will not handle deep links in the development environment. To be able to test deep linking, you must set the environment to something other than "Development". - -Within the `.env` file, set `envName` to something other than "Development", for example: - -``` -envName=Staging -``` +## Google -Alternatively, within the `DeepLinkWrapper/index.website.js` file you can set the `CONFIG.ENVIRONMENT` to something other than "Development". +Unlike with Apple, to test Google Sign-In we don't need to set up any http/ssh tunnels. We can just use `localhost`. But we need to set up the web and desktop environments to use `localhost` instead of `dev.new.expensify.com` -#### Handle deep links in dev on MacOS - -If developing on MacOS, the development desktop app can't handle deeplinks correctly. To be able to test deeplinking back to the app, follow these steps: - -1. Create a "real" build of the desktop app, which can handle deep links, open the build folder, and install the dmg there: - -``` -npm run desktop-build -open desktop-build -# Then double-click "NewExpensify.dmg" in Finder window -``` - -2. Even with this build, the deep link may not be handled by the correct app, as the development Electron config seems to intercept it sometimes. To manage this, install [SwiftDefaultApps](https://github.com/Lord-Kamina/SwiftDefaultApps), which adds a preference pane that can be used to configure which app should handle deep links. - -### Test the Apple / Google SSO buttons in development environment - -The Apple/Google Sign In button renders differently in development mode. To prevent confusion -for developers about a possible regression, we decided to not render third party buttons in -development mode. - -Here's how you can re-enable the SSO buttons in development mode: - -- Remove this [condition](https://github.com/Expensify/App/blob/c2a718c9100e704c89ad9564301348bc53a49777/src/pages/signin/LoginForm/BaseLoginForm.tsx#L300) so that we always render the SSO button components - ```diff - diff --git a/src/pages/signin/LoginForm/BaseLoginForm.tsx b/src/pages/signin/LoginForm/BaseLoginForm.tsx - index 4286a26033..850f8944ca 100644 - --- a/src/pages/signin/LoginForm/BaseLoginForm.tsx - +++ b/src/pages/signin/LoginForm/BaseLoginForm.tsx - @@ -288,7 +288,7 @@ function BaseLoginForm({account, credentials, closeAccount, blurOnSubmit = false - // for developers about possible regressions, we won't render buttons in development mode. - // For more information about these differences and how to test in development mode, - // see`Expensify/App/contributingGuides/APPLE_GOOGLE_SIGNIN.md` - - CONFIG.ENVIRONMENT !== CONST.ENVIRONMENT.DEV && ( - + ( - - { + @@ -246,7 +246,7 @@ const mainWindow = (): Promise => { + let deeplinkUrl: string; + let browserWindow: BrowserWindow; + + - const loadURL = __DEV__ ? (win: BrowserWindow): Promise => win.loadURL(`https://dev.new.expensify.com:${port}`) : serve({directory: `${__dirname}/www`}); + + const loadURL = __DEV__ ? (win: BrowserWindow): Promise => win.loadURL(`http://localhost:${port}`) : serve({directory: `${__dirname}/www`}); + + // Prod and staging set the icon in the electron-builder config, so only update it here for dev + if (__DEV__) { + ``` -The DeepLinkWrapper component will not handle deep links in the development environment. To be able to test deep linking, you must set the environment to something other than "Development". diff --git a/contributingGuides/CONTRIBUTING.md b/contributingGuides/CONTRIBUTING.md index 25f54c668b24..aec527edabe0 100644 --- a/contributingGuides/CONTRIBUTING.md +++ b/contributingGuides/CONTRIBUTING.md @@ -34,7 +34,7 @@ At this time, we are not hiring contractors in Crimea, North Korea, Russia, Iran ## Slack channels All contributors should be a member of a shared Slack channel called [#expensify-open-source](https://expensify.slack.com/archives/C01GTK53T8Q) -- this channel is used to ask **general questions**, facilitate **discussions**, and make **feature requests**. -Before requesting an invite to Slack please ensure your Upwork account is active, since we only pay via Upwork (see [below](https://github.com/Expensify/App/blob/main/contributingGuides/CONTRIBUTING.md#payment-for-contributions)). To request an invite to Slack, email contributors@expensify.com with the subject `Slack Channel Invites`. We'll send you an invite! +Before requesting an invite to Slack, please ensure your Upwork account is active, since we only pay via Upwork (see [below](https://github.com/Expensify/App/blob/main/contributingGuides/CONTRIBUTING.md#payment-for-contributions)). To request an invite to Slack, email contributors@expensify.com with the subject `Slack Channel Invites`. We'll send you an invite! Note: Do not send direct messages to the Expensify team in Slack or Expensify Chat, they will not be able to respond. @@ -44,7 +44,7 @@ Note: if you are hired for an Upwork job and have any job-specific questions, pl If you've found a vulnerability, please email security@expensify.com with the subject `Vulnerability Report` instead of creating an issue. ## Payment for Contributions -We hire and pay external contributors via Upwork.com. If you'd like to be paid for contributing, please create an Upwork account, apply for an available job in [GitHub](https://github.com/Expensify/App/issues?q=is%3Aopen+is%3Aissue+label%3A%22Help+Wanted%22), and finally apply for the job in Upwork once your proposal gets selected in GitHub. Please make sure your Upwork profile is **fully verified** before applying, otherwise you run the risk of not being paid. If you think your compensation should be increased for a specific job, you can request a reevaluation by commenting in the Github issue where the Upwork job was posted. +We hire and pay external contributors via [Upwork.com](https://www.upwork.com). If you'd like to be paid for contributing, please create an Upwork account, apply for an available job in [GitHub](https://github.com/Expensify/App/issues?q=is%3Aopen+is%3Aissue+label%3A%22Help+Wanted%22), and finally apply for the job in Upwork once your proposal gets selected in GitHub. Please make sure your Upwork profile is **fully verified** before applying, otherwise you run the risk of not being paid. If you think your compensation should be increased for a specific job, you can request a reevaluation by commenting in the Github issue where the Upwork job was posted. Payment for your contributions will be made no less than 7 days after the pull request is deployed to production to allow for [regression](https://github.com/Expensify/App/blob/main/contributingGuides/CONTRIBUTING.md#regressions) testing. If you have not received payment after 8 days of the PR being deployed to production, and there are no [regressions](https://github.com/Expensify/App/blob/main/contributingGuides/CONTRIBUTING.md#regressions), please add a comment to the issue mentioning the BugZero team member (Look for the melvin-bot "Triggered auto assignment to... (`Bug`)" to see who this is). @@ -75,10 +75,10 @@ This is the most common scenario for contributors. The Expensify team posts new >**Solution:** Start up time will perceptibly decrease by 1042ms if we prevent the unnecessary re-renders of this component. ## Working on Expensify Jobs -*Reminder: For technical guidance please refer to the [README](https://github.com/Expensify/App/blob/main/README.md)*. +*Reminder: For technical guidance, please refer to the [README](https://github.com/Expensify/App/blob/main/README.md)*. ## Posting Ideas -Additionally if you want to discuss an idea with the open source community without having a P/S statement yet, you can post it in #expensify-open-source with the prefix `IDEA:`. All ideas to build the future of Expensify are always welcome! i.e.: "`IDEA:` I don't have a P/S for this yet, but just kicking the idea around... what if we [insert crazy idea]?". +Additionally, if you want to discuss an idea with the open source community without having a P/S statement yet, you can post it in #expensify-open-source with the prefix `IDEA:`. All ideas to build the future of Expensify are always welcome! i.e.: "`IDEA:` I don't have a P/S for this yet, but just kicking the idea around... what if we [insert crazy idea]?". #### Make sure you can test on all platforms * Expensify requires that you can test the app on iOS, MacOS, Android, Web, and mWeb. @@ -95,9 +95,10 @@ Additionally if you want to discuss an idea with the open source community witho #### Propose a solution for the job 4. You can propose solutions on any issue at any time, but if you propose solutions to jobs before the `Help Wanted` label is applied, you do so at your own risk. Proposals will not be reviewed until the label is added and there is always a chance that we might not add the label or hire an external contributor for the job. -5. After you reproduce the issue, complete the [proposal template here](./PROPOSAL_TEMPLATE.md) and post it as a comment in the corresponding GitHub issue (linked in the Upwork job). +5. Contributors should **not** submit proposals on issues when they have assigned issues or PRs that are awaiting an action from them. If so, they will be in violation of Rule #1 (Get Shit Done) in our [Code of Conduct](https://github.com/Expensify/App/blob/main/CODE_OF_CONDUCT.md) and will receive a warning. Multiple warnings can lead to removal from the program. +6. After you reproduce the issue, complete the [proposal template here](./PROPOSAL_TEMPLATE.md) and post it as a comment in the corresponding GitHub issue (linked in the Upwork job). - Note: Before submitting a proposal on an issue, be sure to read any other existing proposals. ALL NEW PROPOSALS MUST BE DIFFERENT FROM EXISTING PROPOSALS. The *difference* should be important, meaningful or considerable. -6. Refrain from leaving additional comments until someone from the Contributor-Plus team and / or someone from Expensify provides feedback on your proposal (do not create a pull request yet). +7. Refrain from leaving additional comments until someone from the Contributor-Plus team and / or someone from Expensify provides feedback on your proposal (do not create a pull request yet). - Do not leave more than one proposal. - Do not make extensive changes to your current proposal until after it has been reviewed. - If you want to make an entirely new proposal or update an existing proposal, please go back and edit your original proposal, then post a new comment to the issue in this format to alert everyone that it has been updated: @@ -105,8 +106,8 @@ Additionally if you want to discuss an idea with the open source community witho ## Proposal [Updated](link to proposal) ``` -7. If your proposal is accepted by the Expensify engineer assigned to the issue, Expensify will hire you on Upwork and assign the GitHub issue to you. -8. Once hired, post a comment in the Github issue stating when you expect to have your PR ready for review. +8. If your proposal is accepted by the Expensify engineer assigned to the issue, Expensify will hire you on Upwork and assign the GitHub issue to you. +9. Once hired, post a comment in the Github issue stating when you expect to have your PR ready for review. #### Begin coding your solution in a pull request 9. When you are ready to start, fork the repository and create a new branch. @@ -147,7 +148,7 @@ Additionally if you want to discuss an idea with the open source community witho #### Timeline expectations and asking for help along the way - If you have made a change to your pull request and are ready for another review, leave a comment that says "Updated" on the pull request itself. - Please keep the conversation in GitHub, and do not ping individual reviewers in Slack or Upwork to get their attention. -- Pull Request reviews can sometimes take a few days. If your pull request has not been addressed after four days please let us know via the #expensify-open-source Slack channel. +- Pull Request reviews can sometimes take a few days. If your pull request has not been addressed after four days, please let us know via the #expensify-open-source Slack channel. - On occasion, our engineers will need to focus on a feature release and choose to place a hold on the review of your PR. #### Important note about JavaScript Style diff --git a/contributingGuides/FORMS.md b/contributingGuides/FORMS.md index b2f912277dc5..e53be6f6b269 100644 --- a/contributingGuides/FORMS.md +++ b/contributingGuides/FORMS.md @@ -5,7 +5,8 @@ This document lists specific guidelines for using our Form component and general ## General Form UI/UX ### Inputs -Any form input needs to be wrapped in [InputWrapper](https://github.com/Expensify/App/blob/029d009731dcd3c44cd1321672b9672ef0d3d7d9/src/components/Form/InputWrapper.js) and passed as `InputComponent` property additionally it's necessary po pass an unique `inputID`. All other props of the input can be passed as `InputWrapper` props. +Any form input needs to be wrapped in [InputWrapper](https://github.com/Expensify/App/blob/029d009731dcd3c44cd1321672b9672ef0d3d7d9/src/components/Form/InputWrapper.js) and passed as `InputComponent` property, additionally, it's necessary to pass an unique `inputID`. All other props of the input can be passed as `InputWrapper` props. + ```jsx ``` -We also have [keyboardType](https://github.com/Expensify/App/blob/9418b870515102631ea2156b5ea253ee05a98ff1/src/CONST.js#L760-L763) and should be used for specific use cases when there is no `inputMode` equivalent of the value exist. and should be used like so: +We also have [keyboardType](https://github.com/Expensify/App/blob/9418b870515102631ea2156b5ea253ee05a98ff1/src/CONST.js#L760-L763) and should be used for specific use cases when there is no `inputMode` equivalent of the value exist, and should be used like so: ```jsx ` is inside a `` we will want to disable the default safe area padding applied there e.g. +Any `FormProvider.js` that has a button will also add safe area padding by default. If the `` is inside a ``, we will want to disable the default safe area padding applied there e.g. -```js +```jsx {...} diff --git a/contributingGuides/HOW_TO_BECOME_A_CONTRIBUTOR_PLUS.md b/contributingGuides/HOW_TO_BECOME_A_CONTRIBUTOR_PLUS.md index e7dcf5404c34..a72af88ae00b 100644 --- a/contributingGuides/HOW_TO_BECOME_A_CONTRIBUTOR_PLUS.md +++ b/contributingGuides/HOW_TO_BECOME_A_CONTRIBUTOR_PLUS.md @@ -6,7 +6,7 @@ C+ are contributors who are experienced at working with Expensify and have gaine ## Why would someone want to be a C+ - C+ are compensated the same price as the contributor for reviewing proposals and the associated PR. (ie. if a job is listed at $1000, that’s how much the C+ will make if they review both the proposals and PR). If regressions are found that should have* been caught after the PR has been approved, C+ payment is reduced by 50% for each regression found. - * Should have = C+ should have caught the bug by fully following the PR checklist. If C+ skips a step or completed the checklist incompletely, payment will be cut in half. -- C+ can also work on jobs as a contributor +- C+ can also work on jobs as a contributor. - Earning potential is variable, it depends on how much a C+ wants to work and other jobs they’re hired for. We’ve seen C+ make ~$100k/year. - There isn’t a set number of hours a C+ needs to work in a week. Proposals and PRs reviews are expected to be addressed within 24 hours on weekdays. - Dedicated #contributor-plus Slack room to discuss issues, processes and proposals. diff --git a/contributingGuides/HOW_TO_BUILD_APP_ON_PHYSICAL_IOS_DEVICE.md b/contributingGuides/HOW_TO_BUILD_APP_ON_PHYSICAL_IOS_DEVICE.md index a62939aebc25..0f90d71d52ea 100644 --- a/contributingGuides/HOW_TO_BUILD_APP_ON_PHYSICAL_IOS_DEVICE.md +++ b/contributingGuides/HOW_TO_BUILD_APP_ON_PHYSICAL_IOS_DEVICE.md @@ -11,7 +11,7 @@ > [!Important] > You must have a Apple Developer account to run your app on a physical device. If you don't have one, you can register here: [Apple Developer Program](https://developer.apple.com/). - 2.1. Go to `Signing and Capabilities` then in the section called `Signing (Debug/Development and Release/Development)` + 2.1. Go to `Signing and Capabilities`, then in the section called `Signing (Debug/Development and Release/Development)` ![Step 2.1 Screenshot](https://github.com/Expensify/App/assets/104348397/4c668612-ab29-4a91-8e2d-a146e2940017) @@ -24,7 +24,7 @@ ![Step 2.4 Screenshot](https://github.com/Expensify/App/assets/104348397/4ce3f250-4b7c-4e7c-9f1d-09df7bdfc5e0) > [!Note] ->Please be aware that the app built with your own bundle id doesn't support authenticated services like push notification, apple signin, deeplinking etc. which should be only available in Expensify developer account. +>Please be aware that the app built with your own bundle id doesn't support authenticated services like push notification, Apple signin, deeplinking etc. which should be only available in Expensify developer account. 2.5. Scroll down and Remove Associated Domains, Communication Notifications, Push Notifications, and Sign In With Apple capabilities diff --git a/contributingGuides/HOW_TO_CREATE_A_PLAN.md b/contributingGuides/HOW_TO_CREATE_A_PLAN.md index 28ebf1502e71..4f1b2e78a650 100644 --- a/contributingGuides/HOW_TO_CREATE_A_PLAN.md +++ b/contributingGuides/HOW_TO_CREATE_A_PLAN.md @@ -34,7 +34,7 @@ Once you have your solutions, it’s time to decide on the preferred solution (a - Future-proofing - does one solution have more longevity or pave the way for future development? - Independence - does a solution rely on a different problem to be solved first? Does it rely on another piece to be done later? -If you are finding the solution to be difficult, go back and beat harder on the problem to break it up into smaller pieces. Keep repeating until you have a general list of prioritized stages, with early stages solving the dependencies required by later stages, each of which is extremely well defined, with a reasonably obvious preferred solution. +If you are finding the solution to be difficult, go back and beat harder on the problem to break it up into smaller pieces. Keep repeating until you have a general list of prioritized stages, with early stages solving the dependencies required by later stages, each of which is extremely well-defined, with a reasonably obvious preferred solution. ## Step 3: Write out each problem and solution (P/S statement) Have a trusted peer or two proof your P/S statement and help you ensure it is well-defined. If you're in need of a peer to proof, post in #expensify-open-source to ask for help. Refine it and then share with another peer or two until you have a clear, understandable P/S statement. The more complex the problem and solution, the more people should review it. Keep going back to step 1 if needed. diff --git a/contributingGuides/KSv2.md b/contributingGuides/KSv2.md index 881d191ad886..44b1f5b36fe8 100644 --- a/contributingGuides/KSv2.md +++ b/contributingGuides/KSv2.md @@ -24,13 +24,13 @@ In the dashboard, you can first see the PRs assigned to you as `Reviewer`. As pa ### Issues assigned to you -In the next section you can see all issues assigned to you, prioritized from most urgent (on the left) to least urgent (on the right). Issues will also change color depending on other factors - e.g. if they have "HOLD" in the title or if they have the `Overdue`, `Planning`, or `Waiting for copy` labels applied. +In the next section, you can see all issues assigned to you, prioritized from most urgent (on the left) to least urgent (on the right). Issues will also change color depending on other factors - e.g. if they have "HOLD" in the title or if they have the `Overdue`, `Planning`, or `Waiting for copy` labels applied. If a GitHub issue has the `Overdue` label, the text will be red. This means that the issue hasn't been updated in the amount of time allotted for an update (ex - A weekly issue becomes overdue if it hasn't been updated in a week). ### Your Pull Requests -After the issues section you will find a section that lists the PRs you've created. +After the issues section, you will find a section that lists the PRs you've created. diff --git a/contributingGuides/NAVIGATION.md b/contributingGuides/NAVIGATION.md index 5bb6dfb85851..6c0a5b460654 100644 --- a/contributingGuides/NAVIGATION.md +++ b/contributingGuides/NAVIGATION.md @@ -4,13 +4,13 @@ The navigation in the App consists of a top-level Stack Navigator (called `RootS ## Terminology -`RHP` - Right hand panel that shows content inside a dismissible modal that takes up a partial portion of the screen on large format devices e.g. desktop/web/tablets. On smaller screens the content shown in the RHP fills the entire screen. +`RHP` - Right hand panel that shows content inside a dismissible modal that takes up a partial portion of the screen on large format devices e.g. desktop/web/tablets. On smaller screens, the content shown in the RHP fills the entire screen. Navigation Actions - User actions correspond to resulting navigation actions that we will define now. The navigation actions are: `Back`, `Up`, `Dismiss`, `Forward` and `Push`. - `Back` - Moves the user “back” in the history stack by popping the screen or stack of screens. Note: This can potentially make the user exit the app itself (native) or display a previous app (not Expensify), or just the empty state of the browser. -- `Up` - Pops the current screen off the current stack. This action is very easy to confuse with `Back`. Unless you’ve navigated from one screen to a nested screen in a stack of screens these actions will almost always be the same. Unlike a “back” action, “up” should never result in the user exiting the app and should only be an option if there is somewhere to go “up” to. +- `Up` - Pops the current screen off the current stack. This action is very easy to confuse with `Back`. Unless you’ve navigated from one screen to a nested screen in a stack of screens, these actions will almost always be the same. Unlike a “back” action, “up” should never result in the user exiting the app and should only be an option if there is somewhere to go “up” to. - `Dismiss` - Closes any modals (outside the navigation hierarchy) or pops a nested stack of screens off returning the user to the previous screen in the main stack. @@ -26,7 +26,7 @@ Most of the time, if you want to add some of the flows concerning one of your re - If you want to create new flow, add a `Screen` in `RightModalNavigator.tsx` and make new modal in `ModalStackNavigators.tsx` with chosen pages. -When creating RHP flows, you have to remember a couple things: +When creating RHP flows, you have to remember a couple of things: - Since you can deeplink to different pages inside the RHP navigator, it is important to provide the possibility for the user to properly navigate back from any page with UP press (`HeaderWithBackButton` component). @@ -87,7 +87,7 @@ Using [react-freeze](https://github.com/software-mansion/react-freeze) allows us - The wide layout is rendered with our custom `ThreePaneView.js` and the narrow layout is rendered with `StackView` from `@react-navigation/stack` -- To make sure that we have the correct navigation state after changing the layout we need to force react to create new instance of the `NavigationContainer`. Without this, the navigation state could be broken after changing the type of layout. +- To make sure that we have the correct navigation state after changing the layout, we need to force react to create new instance of the `NavigationContainer`. Without this, the navigation state could be broken after changing the type of layout. - We are getting the new instance by changing the `key` prop of `NavigationContainer` that depends on the `isSmallScreenWidth`. @@ -115,10 +115,11 @@ Broken behavior is the outcome of two things: The reason why `getActionFromState` provided by `react-navigation` is dispatched at the top level of the navigation hierarchy is that it doesn't know about current navigation state, only about desired one. -In this example it doesn't know if the `RightModalNavigator` and `Settings` are already mounted. +In this example, it doesn't know if the `RightModalNavigator` and `Settings` are already mounted. -The action for the first step looks like that: +The action for the first step looks like that: + ```json { "type": "NAVIGATE", @@ -193,7 +194,7 @@ If we can create simple action that will only push one screen to the existing na The `getMinimalAction` compares action generated by the `getActionFromState` with the current navigation state and tries to find the smallest action possible. -The action for the first step created with `getMinimalAction` looks like this: +The action for the first step created with `getMinimalAction` looks like this: ```json { diff --git a/contributingGuides/OFFLINE_UX.md b/contributingGuides/OFFLINE_UX.md index cd45bebdce4b..9fefbaeea111 100644 --- a/contributingGuides/OFFLINE_UX.md +++ b/contributingGuides/OFFLINE_UX.md @@ -60,7 +60,7 @@ This is the pattern where we queue the request to be sent when the user is onlin - the user should be given instant feedback and - the user does not need to know when the change is done on the server in the background -**How to implement:** Use [`API.write()`](https://github.com/Expensify/App/blob/3493f3ca3a1dc6cdbf9cb8bd342866fcaf45cf1d/src/libs/API.js#L7-L28) to implement this pattern. For this pattern we should only put `optimisticData` in the options. We don't need `successData` or `failureData` as we don't care what response comes back at all. +**How to implement:** Use [`API.write()`](https://github.com/Expensify/App/blob/3493f3ca3a1dc6cdbf9cb8bd342866fcaf45cf1d/src/libs/API.js#L7-L28) to implement this pattern. For this pattern, we should only put `optimisticData` in the options. We don't need `successData` or `failureData` as we don't care what response comes back at all. **Example:** Pinning a chat. @@ -77,7 +77,7 @@ When the user is offline: **How to implement:** - Use API.write() to implement this pattern - Optimistic data should include `pendingAction` ([with these possible values](https://github.com/Expensify/App/blob/15f7fa622805ee2971808d6bc67181c4715f0c62/src/CONST.js#L775-L779)) -- To ensure the UI is shown as described above, you should enclose the components that contain the data that was added/updated/deleted with the OfflineWithFeedback component +- To ensure the UI is shown as described above, you should enclose the components that contain the data that was added/updated/deleted with the `OfflineWithFeedback` component - Include this data in the action call: - `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) @@ -86,9 +86,9 @@ When the user is offline: **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) -- When dismissing the error, the onClose prop will be called, there we need to call an action that either: +- When dismissing the error, the `onClose` prop will be called, there we need to call an action that either: - If the pendingAction was `delete`, it removes the data altogether - - Otherwise, it would clear the errors and pendingAction properties from the data + - Otherwise, it would clear the errors and `pendingAction` properties from the data - We also need to show a Red Brick Road (RBR) guiding the user to the error. We need to manually do this for each piece of data using pattern B Optimistic WITH Feedback. Some common components like `MenuItem` already have a prop for it (`brickRoadIndicator`) - A Brick Road is the pattern of guiding members towards places that require their attention by following a series of UI elements that have the same color diff --git a/contributingGuides/PERFORMANCE.md b/contributingGuides/PERFORMANCE.md index 0e8ee14d70a4..1942c97af913 100644 --- a/contributingGuides/PERFORMANCE.md +++ b/contributingGuides/PERFORMANCE.md @@ -4,7 +4,7 @@ - Use [`PureComponent`](https://reactjs.org/docs/react-api.html#reactpurecomponent), [`React.memo()`](https://reactjs.org/docs/react-api.html#reactmemo), and [`shouldComponentUpdate()`](https://reactjs.org/docs/react-component.html#shouldcomponentupdate) to prevent re-rendering expensive components. - Using a combination of [React DevTools Profiler](https://chrome.google.com/webstore/detail/react-developer-tools/fmkadmapgofadopljbjfkapdkoienihi?hl=en) and [Chrome Dev Tools Performance Timing](https://calibreapp.com/blog/react-performance-profiling-optimization) can help identify unnecessary re-renders. Both tools can be used to time an interaction like the app starting up or navigating to a new screen. - Watch out for [very large lists](https://reactnative.dev/docs/optimizing-flatlist-configuration) and things like `Image` components re-fetching images on render when a remote uri did not change. -- Avoid the temptation to over-optimize. There is added cost in both code complexity and performance when adding checks like `shouldComponentUpdate()`. Be selective about when you use this and make sure there is a measureable difference before proposing the change. As a very general rule it should be measurably faster to run logic to avoid the re-render (e.g. do a deep comparison) than it would be to let React take care of it without any extra intervention from us. +- Avoid the temptation to over-optimize. There is added cost in both code complexity and performance when adding checks like `shouldComponentUpdate()`. Be selective about when you use this and make sure there is a measureable difference before proposing the change. As a very general rule, it should be measurably faster to run logic to avoid the re-render (e.g. do a deep comparison) than it would be to let React take care of it without any extra intervention from us. - Use caution when adding subscriptions that might re-render very large trees of components e.g. subscribing to state that changes often (current report, current route, etc) in the app root. - Avoid using arrow function callbacks in components that are expensive to re-render. React will re-render this component since each time the parent renders it creates a new instance of that function. **Alternative:** Bind the method in the constructor instead. @@ -22,12 +22,12 @@ It's possible, but slightly trickier to profile the JS running on Android devices as it does not run in a browser but a JS VM that React Native must spin up first then run the app code. The VM we are currently using on both Android and iOS is called [Hermes](https://reactnative.dev/docs/profile-hermes) and is developed by Facebook. -In order to profile with Hermes follow these steps: +In order to profile with Hermes, follow these steps: -- In the metro bundler window press `d` on your keyboard to bring up the developer menu on your device. +- In the metro bundler window, press `d` on your keyboard to bring up the developer menu on your device. - Select "Settings" - Select "Start Sampling Profiler on Init" -- In metro bundler refresh by pressing r +- In metro bundler, refresh by pressing r - The app will start up and a profile will begin - Once the app loads take whatever action you want to profile - Press `d` again and select "Disable Sampling Profiler" @@ -52,7 +52,7 @@ In order to profile with Hermes follow these steps: ### Performance Metrics (Opt-In on local release builds) -To capture reliable performance metrics for native app launch we must test against a release build. To make this easier for everyone to do we created an opt-in tool (using [`react-native-performance`](https://github.com/oblador/react-native-performance) that will capture metrics and display them in an alert once the app becomes interactive. To set this up just set `CAPTURE_METRICS=true` in your `.env` file then create a release build on iOS or Android. The metrics this tool shows are as follows: +To capture reliable performance metrics for native app launch, we must test against a release build. To make this easier for everyone to do, we created an opt-in tool (using [`react-native-performance`](https://github.com/oblador/react-native-performance)) that will capture metrics and display them in an alert once the app becomes interactive. To set this up, just set `CAPTURE_METRICS=true` in your `.env` file, then create a release build on iOS or Android. The metrics this tool shows are as follows: - `nativeLaunch` - Total time for the native process to initialize - `runJSBundle` - Total time to parse and execute the JS bundle @@ -73,22 +73,23 @@ signingConfigs { keyAlias 'your_key_alias' keyPassword 'Password1' } +} ``` - Delete any existing apps off emulator or device - Run `react-native run-android --variant release` ## Reconciliation -React is pretty smart and in many cases is able to tell if something needs to update. The process by which React goes about updating the "tree" or view heirarchy is called reconciliation. If React thinks something needs to update it will render it again. React also assumes that if a parent component rendered then it's child should also re-render. +React is pretty smart and in many cases is able to tell if something needs to update. The process by which React goes about updating the "tree" or view hierarchy is called reconciliation. If React thinks something needs to update, it will render it again. React also assumes that if a parent component rendered, then its child should also re-render. -Re-rendering can be expensive at times and when dealing with nested props or state React may render when it doesn't need to which can be wasteful. A good example of this is a component that is being passed an object as a prop. Let's say the component only requires one or two properties from that object in order to build it's view, but doesn't care about some others. React will still re-render that component even if nothing it cares about has changed. Most of the time this is fine since reconciliation is pretty fast. But we might run into performance issues when re-rendering massive lists. +Re-rendering can be expensive at times and when dealing with nested props or state React may render when it doesn't need to which can be wasteful. A good example of this is a component that is being passed an object as a prop. Let's say the component only requires one or two properties from that object in order to build its view, but doesn't care about some others. React will still re-render that component even if nothing it cares about has changed. Most of the time this is fine since reconciliation is pretty fast. But we might run into performance issues when re-rendering massive lists. In this example, the most preferable solution would be to **only pass the properties that the object needs to know about** to the component in the first place. Another option would be to use `shouldComponentUpdate` or `React.memo()` to add more specific rules comparing `props` to **explicitly tell React not to perform a re-render**. -React might still take some time to re-render a component when it's parent component renders. If it takes a long time to re-render the child even though we have no props changing then we can use `PureComponent` or `React.memo()` (without a callback) which will "shallow compare" the `props` to see if a component should re-render. +React might still take some time to re-render a component when its parent component renders. If it takes a long time to re-render the child even though we have no props changing, then we can use `PureComponent` or `React.memo()` (without a callback) which will "shallow compare" the `props` to see if a component should re-render. -If you aren't sure what exactly is changing about some deeply nested object prop you can use `Performance.diffObject()` method in `componentDidUpdate()` which should show you exactly what is changing from one update to the next. +If you aren't sure what exactly is changing about some deeply nested object prop, you can use `Performance.diffObject()` method in `componentDidUpdate()` which should show you exactly what is changing from one update to the next. **Suggested:** [React Docs - Reconciliation](https://reactjs.org/docs/reconciliation.html) diff --git a/contributingGuides/PROPOSAL_TEMPLATE.md b/contributingGuides/PROPOSAL_TEMPLATE.md index 53330dfe96c9..8c9fa7968fe2 100644 --- a/contributingGuides/PROPOSAL_TEMPLATE.md +++ b/contributingGuides/PROPOSAL_TEMPLATE.md @@ -14,13 +14,13 @@ For almost all of our code style rules, refer to the [Airbnb JavaScript Style Guide](https://github.com/airbnb/javascript). When writing ES6 or React code, please also refer to the [Airbnb React/JSX Style Guide](https://github.com/airbnb/javascript/tree/master/react). @@ -10,13 +68,508 @@ We use Prettier to automatically style our code. There are a few things that we have customized for our tastes which will take precedence over Airbnb's guide. +## TypeScript guidelines + +### General rules + +Strive to type as strictly as possible. + +```ts +type Foo = { + fetchingStatus: "loading" | "success" | "error"; // vs. fetchingStatus: string; + person: { name: string; age: number }; // vs. person: Record; +}; +``` + +### `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](https://www.typescriptlang.org/docs/handbook/declaration-merging.html#module-augmentation). + +> Why? Type errors in `d.ts` files are not checked by TypeScript. + +### Type Alias vs. Interface + +Do not use `interface`. Use `type`. eslint: [`@typescript-eslint/consistent-type-definitions`](https://typescript-eslint.io/rules/consistent-type-definitions/) + +> Why? In TypeScript, `type` and `interface` can be used interchangeably to declare types. Use `type` for consistency. + +```ts +// BAD +interface Person { + name: string; +} + +// GOOD +type Person = { + name: string; +}; +``` + +### Enum vs. Union Type + +Do not use `enum`. Use union types. eslint: [`no-restricted-syntax`](https://eslint.org/docs/latest/rules/no-restricted-syntax) + +> Why? Enums come with several [pitfalls](https://blog.logrocket.com/why-typescript-enums-suck/). Most enum use cases can be replaced with union types. + +```ts +// Most simple form of union type. +type Color = "red" | "green" | "blue"; +function printColors(color: Color) { + console.log(color); +} + +// When the values need to be iterated upon. +import { TupleToUnion } from "type-fest"; + +const COLORS = ["red", "green", "blue"] as const; +type Color = TupleToUnion; // type: 'red' | 'green' | 'blue' + +for (const color of COLORS) { + printColor(color); +} + +// When the values should be accessed through object keys. (i.e. `COLORS.Red` vs. `"red"`) +import { ValueOf } from "type-fest"; + +const COLORS = { + Red: "red", + Green: "green", + Blue: "blue", +} as const; +type Color = ValueOf; // type: 'red' | 'green' | 'blue' + +printColor(COLORS.Red); +``` + +### `unknown` vs. `any` + +Don't use `any`. Use `unknown` if type is not known beforehand. eslint: [`@typescript-eslint/no-explicit-any`](https://typescript-eslint.io/rules/no-explicit-any/) + +> Why? `any` type bypasses type checking. `unknown` is type safe as `unknown` type needs to be type narrowed before being used. + +```ts +const value: unknown = JSON.parse(someJson); +if (typeof value === 'string') {...} +else if (isPerson(value)) {...} +... +``` + +### `T[]` vs. `Array` + +Use `T[]` or `readonly T[]` for simple types (i.e. types which are just primitive names or type references). Use `Array` or `ReadonlyArray` for all other types (union types, intersection types, object types, function types, etc). eslint: [`@typescript-eslint/array-type`](https://typescript-eslint.io/rules/array-type/) + +```ts +// Array +const a: Array = ["a", "b"]; +const b: Array<{ prop: string }> = [{ prop: "a" }]; +const c: Array<() => void> = [() => {}]; + +// T[] +const d: MyType[] = ["a", "b"]; +const e: string[] = ["a", "b"]; +const f: readonly string[] = ["a", "b"]; +``` + +### `@ts-ignore` + +Do not use `@ts-ignore` or its variant `@ts-nocheck` to suppress warnings and errors. + +### Type Inference + +When possible, allow the compiler to infer type of variables. + +```ts +// BAD +const foo: string = "foo"; +const [counter, setCounter] = useState(0); + +// GOOD +const foo = "foo"; +const [counter, setCounter] = useState(0); +const [username, setUsername] = useState(undefined); // Username is a union type of string and undefined, and its type cannot be inferred from the default value of undefined +``` + +For function return types, default to always typing them unless a function is simple enough to reason about its return type. + +> Why? Explicit return type helps catch errors when implementation of the function changes. It also makes it easy to read code even when TypeScript intellisense is not provided. + +```ts +function simpleFunction(name: string) { + return `hello, ${name}`; +} + +function complicatedFunction(name: string): boolean { +// ... some complex logic here ... + return foo; +} +``` + +### Utility Types + +Use types from [TypeScript utility types](https://www.typescriptlang.org/docs/handbook/utility-types.html) and [`type-fest`](https://github.com/sindresorhus/type-fest) when possible. + +```ts +type Foo = { + bar: string; +}; + +// BAD +type ReadOnlyFoo = { + readonly [Property in keyof Foo]: Foo[Property]; +}; + +// GOOD +type ReadOnlyFoo = Readonly; + +// BAD +type FooValue = Foo[keyof Foo]; + +// GOOD +type FooValue = ValueOf; + +``` + +### `object` type + +Don't use `object` type. eslint: [`@typescript-eslint/ban-types`](https://typescript-eslint.io/rules/ban-types/) + +> Why? `object` refers to "any non-primitive type," not "any object". Typing "any non-primitive value" is not commonly needed. + +```ts +// BAD +const foo: object = [1, 2, 3]; // TypeScript does not error +``` + +If you know that the type of data is an object but don't know what properties or values it has beforehand, use `Record`. + +> Even though `string` is specified as a key, `Record` type can still accept objects whose keys are numbers. This is because numbers are converted to strings when used as an object index. Note that you cannot use [symbols](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Symbol) for `Record`. + +```ts +function logObject(object: Record) { + for (const [key, value] of Object.entries(object)) { + console.log(`${key}: ${value}`); + } +} +``` + +### Prop Types + +Don't use `ComponentProps` to grab a component's prop types. Go to the source file for the component and export prop types from there. Import and use the exported prop types. + +> Why? Importing prop type from the component file is more common and readable. Using `ComponentProps` might cause problems in some cases (see [related GitHub issue](https://github.com/piotrwitek/react-redux-typescript-guide/issues/170)). Each component with props has it's prop type defined in the file anyway, so it's easy to export it when required. + +Don't export prop types from component files by default. Only export it when there is a code that needs to access the prop type directly. + +```tsx +// MyComponent.tsx +export type MyComponentProps = { + foo: string; +}; + +export default function MyComponent({ foo }: MyComponentProps) { + return {foo}; +} + +// BAD +import { ComponentProps } from "React"; +import MyComponent from "./MyComponent"; +type MyComponentProps = ComponentProps; + +// GOOD +import MyComponent, { MyComponentProps } from "./MyComponent"; +``` + +### File organization + +In modules with platform-specific implementations, create `types.ts` to define shared types. Import and use shared types in each platform specific files. Do not use [`satisfies` operator](#satisfies-operator) for platform-specific implementations, always define shared types that complies with all variants. + +> Why? To encourage consistent API across platform-specific implementations. If you're migrating module that doesn't have a default implementation (i.e. `index.ts`, e.g. `getPlatform`), refer to [Migration Guidelines](#migration-guidelines) for further information. + +Utility module example + +```ts +// types.ts +type GreetingModule = { + getHello: () => string; + getGoodbye: () => string; +}; + +// index.native.ts +import { GreetingModule } from "./types"; +function getHello() { + return "hello from mobile code"; +} +function getGoodbye() { + return "goodbye from mobile code"; +} +const Greeting: GreetingModule = { + getHello, + getGoodbye, +}; +export default Greeting; + +// index.ts +import { GreetingModule } from "./types"; +function getHello() { + return "hello from other platform code"; +} +function getGoodbye() { + return "goodbye from other platform code"; +} +const Greeting: GreetingModule = { + getHello, + getGoodbye, +}; +export default Greeting; +``` + +Component module example + +```ts +// types.ts +export type MyComponentProps = { + foo: string; +} + +// index.ios.ts +import { MyComponentProps } from "./types"; + +export MyComponentProps; +export default function MyComponent({ foo }: MyComponentProps) { /* ios specific implementation */ } + +// index.ts +import { MyComponentProps } from "./types"; + +export MyComponentProps; +export default function MyComponent({ foo }: MyComponentProps) { /* Default implementation */ } +``` + +### Reusable Types + +Reusable type definitions, such as models (e.g., Report), must have their own file and be placed under `src/types/`. The type should be exported as a default export. + +```ts +// src/types/Report.ts + +type Report = {...}; + +export default Report; +``` + +### `tsx` extension + +Use `.tsx` extension for files that contain React syntax. + +> Why? It is a widely adopted convention to mark any files that contain React specific syntax with `.jsx` or `.tsx`. + +### No inline prop types + +Do not define prop types inline for components that are exported. + +> Why? Prop types might [need to be exported from component files](#export-prop-types). If the component is only used inside a file or module and not exported, then inline prop types can be used. + +```ts +// BAD +export default function MyComponent({ foo, bar }: { foo: string, bar: number }){ + // component implementation +}; + +// GOOD +type MyComponentProps = { foo: string, bar: number }; +export default MyComponent({ foo, bar }: MyComponentProps){ + // component implementation +} +``` + +### Satisfies Operator + +Use the `satisfies` operator when you need to validate that the structure of an expression matches a specific type, without affecting the resulting type of the expression. + +> Why? TypeScript developers often want to ensure that an expression aligns with a certain type, but they also want to retain the most specific type possible for inference purposes. The `satisfies` operator assists in doing both. + +```ts +// BAD +const sizingStyles = { + w50: { + width: '50%', + }, + mw100: { + maxWidth: '100%', + }, +} as const; + +// GOOD +const sizingStyles = { + w50: { + width: '50%', + }, + mw100: { + maxWidth: '100%', + }, +} as const satisfies Record; +``` + +The example above results in the most narrow type possible, also the values are `readonly`. There are some cases in which that is not desired (e.g. the variable can be modified), if so `as const` should be omitted. + +### Type imports/exports + +Always use the `type` keyword when importing/exporting types + +> Why? In order to improve code clarity and consistency and reduce bundle size after typescript transpilation, we enforce the all type imports/exports to contain the `type` keyword. This way, TypeScript can automatically remove those imports from the transpiled JavaScript bundle + +Imports: +```ts +// BAD +import {SomeType} from './a' +import someVariable from './a' + +import {someVariable, SomeOtherType} from './b' + +// GOOD +import type {SomeType} from './a' +import someVariable from './a' +``` + + Exports: +```ts +// BAD +export {SomeType} +export someVariable +// or +export {someVariable, SomeOtherType} + +// GOOD +export type {SomeType} +export someVariable +``` + +### Refs + +Avoid using HTML elements while declaring refs. Please use React Native components where possible. React Native Web handles the references on its own. It also extends React Native components with [Interaction API](https://necolas.github.io/react-native-web/docs/interactions/) which should be used to handle Pointer and Mouse events. Exception of this rule is when we explicitly need to use functions available only in DOM and not in React Native, e.g. `getBoundingClientRect`. Then please declare ref type as `union` of React Native component and HTML element. When passing it to React Native component assert it as soon as possible using utility methods declared in `src/types/utils`. + +Normal usage: +```tsx +const ref = useRef(); + + {#DO SOMETHING}}> +``` + +Exceptional usage where DOM methods are necessary: +```tsx +import viewRef from '@src/types/utils/viewRef'; + +const ref = useRef(); + +if (ref.current && 'getBoundingClientRect' in ref.current) { + ref.current.getBoundingClientRect(); +} + + {#DO SOMETHING}}> +``` + +### Other Expensify Resources on TypeScript + +- [Expensify TypeScript React Native CheatSheet](./TS_CHEATSHEET.md) + ## Naming Conventions +### Type names + + - Use PascalCase for type names. eslint: [`@typescript-eslint/naming-convention`](https://typescript-eslint.io/rules/naming-convention/) + + ```ts + // BAD + type foo = ...; + type BAR = ...; + + // GOOD + type Foo = ...; + type Bar = ...; + ``` + + - Do not postfix type aliases with `Type`. + + ```ts + // BAD + type PersonType = ...; + + // GOOD + type Person = ...; + ``` + + - Use singular name for union types. + + ```ts + // BAD + type Colors = "red" | "blue" | "green"; + + // GOOD + type Color = "red" | "blue" | "green"; + ``` + + - Use `{ComponentName}Props` pattern for prop types. + + ```ts + // BAD + type Props = { + // component's props + }; + + function MyComponent({}: Props) { + // component's code + } + + // GOOD + type MyComponentProps = { + // component's props + }; + + function MyComponent({}: MyComponentProps) { + // component's code + } + ``` + + - For generic type parameters, use `T` if you have only one type parameter. Don't use the `T`, `U`, `V`... sequence. Make type parameter names descriptive, each prefixed with `T`. + + > Prefix each type parameter name to distinguish them from other types. + + ```ts + // BAD + type KeyValuePair = { key: K; value: U }; + + type Keys = Array; + + // GOOD + type KeyValuePair = { key: TKey; value: TValue }; + + type Keys = Array; + type Keys = Array; + ``` + +### Prop callbacks + - Prop callbacks should be named for what has happened, not for what is going to happen. Components should never assume anything about how they will be used (that's the job of whatever is implementing it). + + ```ts + // Bad + type ComponentProps = { + /** A callback to call when we want to save the form */ + onSaveForm: () => void; + }; + + // Good + type ComponentProps = { + /** A callback to call when the form has been submitted */ + onFormSubmitted: () => void; + }; + ``` + + * Do not use underscores when naming private methods. + ### Event Handlers - When you have an event handler, do not prefix it with "on" or "handle". The method should be named for what it does, not what it handles. This promotes code reuse by minimizing assumptions that a method must be called in a certain fashion (eg. only as an event handler). - One exception for allowing the prefix of "on" is when it is used for callback `props` of a React component. Using it in this way helps to distinguish callbacks from public component methods. - ```javascript + ```ts // Bad const onSubmitClick = () => { // Validate form items and submit form @@ -32,7 +585,7 @@ There are a few things that we have customized for our tastes which will take pr - Boolean props or variables must be prefixed with `should` or `is` to make it clear that they are `Boolean`. Use `should` when we are enabling or disabling some features and `is` in most other cases. -```javascript +```tsx // Bad @@ -46,11 +599,11 @@ const valid = props.something && props.somethingElse; const isValid = props.something && props.somethingElse; ``` -## Functions +### Functions Any function declared in a library module should use the `function myFunction` keyword rather than `const myFunction`. -```javascript +```tsx // Bad const myFunction = () => {...}; @@ -68,25 +621,34 @@ export { } ``` -Using arrow functions is the preferred way to write an anonymous function such as a callback method. +You can still use arrow function for declarations or simple logics to keep them readable. -```javascript +```tsx // Bad -someArray.map(function (item) {...}); +randomList.push({ + onSelected: Utils.checkIfAllowed(function checkTask() { return Utils.canTeamUp(people); }), +}); +routeList.filter(function checkIsActive(route) { + return route.isActive; +}); // Good -someArray.map((item) => {...}); -``` - -Empty functions (noop) should be declare as arrow functions with no whitespace inside. Avoid _.noop() - -```javascript -// Bad -const callback = _.noop; -const callback = () => { }; +randomList.push({ + onSelected: Utils.checkIfAllowed(() => Utils.canTeamUp(people)), +}); +routeList.filter((route) => route.isActive); +const myFunction = () => {...}; +const person = { getName: () => {} }; +Utils.connect({ + callback: (val) => {}, +}); +useEffect(() => { + if (isFocused) { + return; + } + setError(null, {}); +}, [isFocused]); -// Good -const callback = () => {}; ``` ## `var`, `const` and `let` @@ -96,7 +658,7 @@ const callback = () => {}; - Try to write your code in a way where the variable reassignment isn't necessary - Use `let` only if there are no other options -```javascript +```tsx // Bad let array = []; @@ -112,55 +674,99 @@ if (someCondition) { } ``` -## Accessing Object Properties and Default Values +## Object / Array Methods -Use `lodashGet()` to safely access object properties and `||` to short circuit null or undefined values that are not guaranteed to exist in a consistent way throughout the codebase. In the rare case that you want to consider a falsy value as usable and the `||` operator prevents this then be explicit about this in your code and check for the type. +We have standardized on using the native [Array instance methods](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array#instance_methods) instead of [lodash](https://lodash.com/) methods for objects and collections. As the vast majority of code is written in TypeScript, we can safely use the native methods. -```javascript +```ts // Bad -const value = somePossiblyNullThing ?? 'default'; +_.each(myArray, item => doSomething(item)); // Good -const value = somePossiblyNullThing || 'default'; +myArray.forEach(item => doSomething(item)); + // Bad -const value = someObject.possiblyUndefinedProperty?.nestedProperty || 'default'; +const myArray = _.map(someObject, (value, key) => doSomething(value)); +// Good +const myArray = Object.keys(someObject).map((key) => doSomething(someObject[key])); + +// Bad +_.contains(myCollection, 'item'); +// Good +myCollection.includes('item'); + // Bad -const value = (someObject && someObject.possiblyUndefinedProperty && someObject.possiblyUndefinedProperty.nestedProperty) || 'default'; +const modifiedArray = _.chain(someArray) + .filter(filterFunc) + .map(mapFunc) + .value(); // Good -const value = lodashGet(someObject, 'possiblyUndefinedProperty.nestedProperty', 'default'); +const modifiedArray = someArray.filter(filterFunc).map(mapFunc); ``` -## JSDocs +## Accessing Object Properties and Default Values + +Use optional chaining (`?.`) to safely access object properties and nullish coalescing (`??`) to short circuit null or undefined values that are not guaranteed to exist in a consistent way throughout the codebase. Don't use the `lodashGet()` function. eslint: [`no-restricted-imports`](https://eslint.org/docs/latest/rules/no-restricted-imports) + +```ts +// BAD +import lodashGet from "lodash/get"; +const name = lodashGet(user, "name", "default name"); -- Always document parameters and return values. -- Optional parameters should be enclosed by `[]` e.g. `@param {String} [optionalText]`. -- Document object parameters with separate lines e.g. `@param {Object} parameters` followed by `@param {String} parameters.field`. -- If a parameter accepts more than one type use `*` to denote there is no single type. -- Use uppercase when referring to JS primitive values (e.g. `Boolean` not `bool`, `Number` not `int`, etc). -- When specifying a return value use `@returns` instead of `@return`. If there is no return value do not include one in the doc. +// BAD +const name = user?.name || "default name"; +// GOOD +const name = user?.name ?? "default name"; +``` + +## JSDocs + +- Omit comments that are redundant with TypeScript. Do not declare types in `@param` or `@return` blocks. Do not write `@implements`, `@enum`, `@private`, `@override`. eslint: [`jsdoc/no-types`](https://github.com/gajus/eslint-plugin-jsdoc/blob/main/.README/rules/no-types.md) +- Only document params/return values if their names are not enough to fully understand their purpose. Not all parameters or return values need to be listed in the JSDoc comment. If there is no comment accompanying the parameter or return value, omit it. +- When specifying a return value use `@returns` instead of `@return`. - Avoid descriptions that don't add any additional information. Method descriptions should only be added when it's behavior is unclear. - Do not use block tags other than `@param` and `@returns` (e.g. `@memberof`, `@constructor`, etc). - Do not document default parameters. They are already documented by adding them to a declared function's arguments. - Do not use record types e.g. `{Object.}`. -- Do not create `@typedef` to use in JSDocs. -- Do not use type unions e.g. `{(number|boolean)}`. -```javascript -// Bad +```ts +// BAD /** - * Populates the shortcut modal - * @param {bool} shouldShowAdvancedShortcuts whether to show advanced shortcuts - * @return {*} + * @param {number} age + * @returns {boolean} Whether the person is a legal drinking age or nots */ -function populateShortcutModal(shouldShowAdvancedShortcuts) { +function canDrink(age: number): boolean { + return age >= 21; } -// Good +// GOOD /** - * @param {Boolean} shouldShowAdvancedShortcuts - * @returns {Boolean} + * @returns Whether the person is a legal drinking age or nots */ -function populateShortcutModal(shouldShowAdvancedShortcuts) { +function canDrink(age: number): boolean { + return age >= 21; +} +``` + +In the above example, because the parameter `age` doesn't have any accompanying comment, it is completely omitted from the JSDoc. + +## Component props + +Do not use **`propTypes` and `defaultProps`**: . Use object destructing and assign a default value to each optional prop unless the default values is `undefined`. + +```tsx +type MyComponentProps = { + requiredProp: string; + optionalPropWithDefaultValue?: number; + optionalProp?: boolean; +}; + +function MyComponent({ + requiredProp, + optionalPropWithDefaultValue = 42, + optionalProp, +}: MyComponentProps) { + // component's code } ``` @@ -171,7 +777,7 @@ We should avoid using object destructuring in situations where it reduces code c - Avoid object destructuring for a single variable that you only use *once*. It's clearer to use dot notation for accessing a single variable. -```javascript +```ts // Bad const {data} = event.data; @@ -179,35 +785,6 @@ const {data} = event.data; const {name, accountID, email} = data; ``` -**React Components** - -Always use destructuring to get prop values. Destructuring is necessary to assign default values to props. - -```javascript -// Bad -function UserInfo(props) { - return ( - - Name: {props.name} - Email: {props.email} - -} - -UserInfo.defaultProps = { - name: 'anonymous'; -} - -// Good -function UserInfo({ name = 'anonymous', email }) { - return ( - - Name: {name} - Email: {email} - - ); -} -``` - ## Named vs Default Exports in ES6 - When to use what? ES6 provides two ways to export a module from a file: `named export` and `default export`. Which variation to use depends on how the module will be used. @@ -219,7 +796,7 @@ ES6 provides two ways to export a module from a file: `named export` and `defaul - All exports (both default and named) should happen at the bottom of the file - Do **not** export individual features inline. -```javascript +```ts // Bad export const something = 'nope'; export const somethingElse = 'stop'; @@ -236,10 +813,10 @@ export { ## Classes and constructors -#### Class syntax +### Class syntax Using the `class` syntax is preferred wherever appropriate. Airbnb has clear [guidelines](https://github.com/airbnb/javascript#classes--constructors) in their JS style guide which promotes using the _class_ syntax. Don't manipulate the `prototype` directly. The `class` syntax is generally considered more concise and easier to understand. -#### Constructor +### Constructor Classes have a default constructor if one is not specified. No need to write a constructor function that is empty or just delegates to a parent class. ```js @@ -272,110 +849,55 @@ class Rey extends Jedi { JavaScript is always changing. We are excited whenever it does! However, we tend to take our time considering whether to adopt the latest and greatest language features. The main reason for this is **consistency**. We have a style guide so that we don't have to have endless conversations about how our code looks and can focus on how it runs. -So, if a new language feature isn't something we have agreed to support it's off the table. Sticking to just one way to do things reduces cognitive load in reviews and also makes sure our knowledge of language features progresses at the same pace. If a new language feature will cause considerable effort for everyone to adapt to or we're just not quite sold on the value of it yet we won't support it. +So, if a new language feature isn't something we have agreed to support it's off the table. Sticking to just one way to do things reduces cognitive load in reviews and also makes sure our knowledge of language features progresses at the same pace. If a new language feature will cause considerable effort for everyone to adapt to or we're just not quite sold on the value of it yet, we won't support it. Here are a couple of things we would ask that you *avoid* to help maintain consistency in our codebase: - **Async/Await** - Use the native `Promise` instead -- **Optional Chaining** - Use `lodashGet()` to fetch a nested value instead -- **Null Coalescing Operator** - Use `lodashGet()` or `||` to set a default value for a possibly `undefined` or `null` variable - -# React Coding Standards - -# React specific styles -## Method Naming and Code Documentation -* Prop callbacks should be named for what has happened, not for what is going to happen. Components should never assume anything about how they will be used (that's the job of whatever is implementing it). +## React Coding Standards -```javascript -// Bad -const propTypes = { - /** A callback to call when we want to save the form */ - onSaveForm: PropTypes.func.isRequired, -}; - -// Good -const propTypes = { - /** A callback to call when the form has been submitted */ - onFormSubmitted: PropTypes.func.isRequired, -}; -``` +### Code Documentation -* Do not use underscores when naming private methods. -* Add descriptions to all `propTypes` using a block comment above the definition. No need to document the types (that's what `propTypes` is doing already), but add some context for each property so that other developers understand the intended use. +* Add descriptions to all component props using a block comment above the definition. No need to document the types, but add some context for each property so that other developers understand the intended use. -```javascript +```tsx // Bad -const propTypes = { - currency: PropTypes.string.isRequired, - amount: PropTypes.number.isRequired, - isIgnored: PropTypes.bool.isRequired +type ComponentProps = { + currency: string; + amount: number; + isIgnored: boolean; }; // Bad -const propTypes = { +type ComponentProps = { // The currency that the reward is in - currency: React.PropTypes.string.isRequired, + currency: string; // The amount of reward - amount: React.PropTypes.number.isRequired, + amount: number; // If the reward has been ignored or not - isIgnored: React.PropTypes.bool.isRequired + isIgnored: boolean; } // Good -const propTypes = { +type ComponentProps = { /** The currency that the reward is in */ - currency: React.PropTypes.string.isRequired, + currency: string; /** The amount of the reward */ - amount: React.PropTypes.number.isRequired, + amount: number; /** If the reward has not been ignored yet */ - isIgnored: React.PropTypes.bool.isRequired + isIgnored: boolean; } ``` -All `propTypes` and `defaultProps` *must* be defined at the **top** of the file in variables called `propTypes` and `defaultProps`. -These variables should then be assigned to the component at the bottom of the file. - -```js -MyComponent.propTypes = propTypes; -MyComponent.defaultProps = defaultProps; -export default MyComponent; -``` - -Any nested `propTypes` e.g. that may appear in a `PropTypes.shape({})` should also be documented. - -```javascript -// Bad -const propTypes = { - /** Session data */ - session: PropTypes.shape({ - authToken: PropTypes.string, - login: PropTypes.string, - }), -} - -// Good -const propTypes = { - /** Session data */ - session: PropTypes.shape({ - - /** Token used to authenticate the user */ - authToken: PropTypes.string, - - /** User email or phone number */ - login: PropTypes.string, - }), -} -``` - -## Inline Ternaries +### Inline Ternaries * Use inline ternary statements when rendering optional pieces of templates. Notice the white space and formatting of the ternary. -```javascript +```tsx // Bad { const optionalTitle = props.title ?
{props.title}
: null; @@ -388,7 +910,7 @@ const propTypes = { } ``` -```javascript +```tsx // Good { return ( @@ -402,7 +924,7 @@ const propTypes = { } ``` -```javascript +```tsx // Good { return ( @@ -417,11 +939,11 @@ const propTypes = { } ``` -### Important Note: +#### Important Note: In React Native, one **must not** attempt to falsey-check a string for an inline ternary. Even if it's in curly braces, React Native will try to render it as a `` node and most likely throw an error about trying to render text outside of a `` component. Use `!!` instead. -```javascript +```tsx // Bad! This will cause a breaking an error on native platforms { return ( @@ -447,69 +969,158 @@ In React Native, one **must not** attempt to falsey-check a string for an inline } ``` -## Function component style +### Function component style -When writing a function component you must ALWAYS add a `displayName` property and give it the same value as the name of the component (this is so it appears properly in the React dev tools) +When writing a function component, you must ALWAYS add a `displayName` property and give it the same value as the name of the component (this is so it appears properly in the React dev tools) -```javascript +```tsx +function Avatar(props: AvatarProps) {...}; - function Avatar(props) {...}; +Avatar.displayName = 'Avatar'; - Avatar.propTypes = propTypes; - Avatar.defaultProps = defaultProps; - Avatar.displayName = 'Avatar'; +export default Avatar; +``` + +### Forwarding refs + +When forwarding a ref define named component and pass it directly to the `forwardRef`. By doing this, we remove potential extra layer in React tree in the form of anonymous component. - export default Avatar; +```tsx +import type {ForwarderRef} from 'react'; + +type FancyInputProps = { + ... +}; + +function FancyInput(props: FancyInputProps, ref: ForwardedRef) { + ... + return +}; + +export default React.forwardRef(FancyInput) ``` -## Forwarding refs +If the ref handle is not available (e.g. `useImperativeHandle` is used) you can define a custom handle type above the component. -When forwarding a ref define named component and pass it directly to the `forwardRef`. By doing this we remove potential extra layer in React tree in form of anonymous component. +```tsx +import type {ForwarderRef} from 'react'; +import {useImperativeHandle} from 'react'; -```javascript - function FancyInput(props, ref) { - ... - return - } +type FancyInputProps = { + ... + onButtonPressed: () => void; +}; + +type FancyInputHandle = { + onButtonPressed: () => void; +} + +function FancyInput(props: FancyInputProps, ref: ForwardedRef) { + useImperativeHandle(ref, () => ({onButtonPressed})); + + ... + return ; +}; - export default React.forwardRef(FancyInput) +export default React.forwardRef(FancyInput) ``` -## Stateless components vs Pure Components vs Class based components vs Render Props - When to use what? +### Hooks and HOCs -Class components are DEPRECATED. Use function components and React hooks. +Use hooks whenever possible, avoid using HOCs. -[https://react.dev/reference/react/Component#migrating-a-component-with-lifecycle-methods-from-a-class-to-a-function](https://react.dev/reference/react/Component#migrating-a-component-with-lifecycle-methods-from-a-class-to-a-function) +> 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`. -## Composition vs Inheritance +Onyx now provides a `useOnyx` hook that should be used over `withOnyx` HOC. -From React's documentation - ->Props and composition give you all the flexibility you need to customize a component’s look and behavior in an explicit and safe way. Remember that components may accept arbitrary props, including primitive values, React elements, or functions. ->If you want to reuse non-UI functionality between components, we suggest extracting it into a separate JavaScript module. The components may import it and use that function, object, or a class, without extending it. +```tsx +// BAD +type ComponentOnyxProps = { + session: OnyxEntry; +}; -Use an HOC a.k.a. *[Higher order component](https://reactjs.org/docs/higher-order-components.html)* if you find a use case where you need inheritance. +type ComponentProps = ComponentOnyxProps & { + someProp: string; +}; -If several HOC need to be combined there is a `compose()` utility. But we should not use this utility when there is only one HOC. +function Component({session, someProp}: ComponentProps) { + const {windowWidth, windowHeight} = useWindowDimensions(); + const {translate} = useLocalize(); + // component's code +} -```javascript -// Bad -export default compose( - withLocalize, -)(MyComponent); +export default withOnyx({ + session: { + key: ONYXKEYS.SESSION, + }, +})(Component); -// Good -export default compose( - withLocalize, - withWindowDimensions, -)(MyComponent); +// GOOD +type ComponentProps = { + someProp: string; +}; -// Good -export default withLocalize(MyComponent) +function Component({someProp}: ComponentProps) { + const [session] = useOnyx(ONYXKEYS.SESSION) + + const {windowWidth, windowHeight} = useWindowDimensions(); + const {translate} = useLocalize(); + // component's code +} ``` +### Stateless components vs Pure Components vs Class based components vs Render Props - When to use what? + +Class components are DEPRECATED. Use function components and React hooks. + +[https://react.dev/reference/react/Component#migrating-a-component-with-lifecycle-methods-from-a-class-to-a-function](https://react.dev/reference/react/Component#migrating-a-component-with-lifecycle-methods-from-a-class-to-a-function) + +### Composition + +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. + +From React's documentation - +>Props and composition give you all the flexibility you need to customize a component’s look and behavior in an explicit and safe way. Remember that components may accept arbitrary props, including primitive values, React elements, or functions. +>If you want to reuse non-UI functionality between components, we suggest extracting it into a separate JavaScript module. The components may import it and use that function, object, or a class, without extending it. + + ```ts + // BAD + export default compose( + withCurrentUserPersonalDetails, + withReportOrNotFound(), + withOnyx({ + session: { + key: ONYXKEYS.SESSION, + }, + }), + )(Component); + + // GOOD + export default withCurrentUserPersonalDetails( + withReportOrNotFound()( + withOnyx({ + session: { + key: ONYXKEYS.SESSION, + }, + })(Component), + ), + ); + + // GOOD - alternative to HOC nesting + const ComponentWithOnyx = withOnyx({ + session: { + key: ONYXKEYS.SESSION, + }, + })(Component); + const ComponentWithReportOrNotFound = withReportOrNotFound()(ComponentWithOnyx); + export default withCurrentUserPersonalDetails(ComponentWithReportOrNotFound); + ``` + **Note:** If you find that none of these approaches work for you, please ask an Expensify engineer for guidance via Slack or GitHub. -## Use Refs Appropriately +### Use Refs Appropriately React's documentation explains refs in [detail](https://reactjs.org/docs/refs-and-the-dom.html). It's important to understand when to use them and how to use them to avoid bugs and hard to maintain code. @@ -517,43 +1128,43 @@ A common mistake with refs is using them to pass data back to a parent component There are several ways to use and declare refs and we prefer the [callback method](https://reactjs.org/docs/refs-and-the-dom.html#callback-refs). -## Are we allowed to use [insert brand new React feature]? Why or why not? +### Are we allowed to use [insert brand new React feature]? Why or why not? -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. +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 +## React Hooks: Frequently Asked Questions -## Are Hooks a Replacement for HOCs or Render Props? +### 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? +### 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? +### 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? +### 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? +### 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. +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? +### 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? +### 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. -## How do I auto-focus a TextInput using `useFocusEffect()`? +### How do I auto-focus a TextInput using `useFocusEffect()`? -```javascript +```tsx const focusTimeoutRef = useRef(null); useFocusEffect(useCallback(() => { @@ -573,16 +1184,32 @@ This works better than using `onTransitionEnd` because - Note - This is a solution from [this PR](https://github.com/Expensify/App/pull/26415). You can find detailed discussion in comments. -# Onyx Best Practices +## Onyx Best Practices [Onyx Documentation](https://github.com/expensify/react-native-onyx) -## Collection Keys +### Collection Keys -Our potentially larger collections of data (reports, policies, etc) are typically stored under collection keys. Collection keys let us group together individual keys vs. storing arrays with multiple objects. In general, **do not add a new collection key if it can be avoided**. There is most likely a more logical place to put the state. And failing to associate a state property with it's logical owner is something we consider to be an anti-pattern (unnecessary data structure adds complexity for no value). +Our potentially larger collections of data (reports, policies, etc) are typically stored under collection keys. Collection keys let us group together individual keys vs. storing arrays with multiple objects. In general, **do not add a new collection key if it can be avoided**. There is most likely a more logical place to put the state. And failing to associate a state property with its logical owner is something we consider to be an anti-pattern (unnecessary data structure adds complexity for no value). -For example, if you are storing a boolean value that could be associated with a `report` object under a new collection key it is better to associate this information with the report itself and not create a new collection key. +For example, if you are storing a boolean value that could be associated with a `report` object under a new collection key, it is better to associate this information with the report itself and not create a new collection key. -**Exception:** There are some [gotchas](https://github.com/expensify/react-native-onyx#merging-data) when working with complex nested array values in Onyx. So, this could be another valid reason to break a property off of it's parent object (e.g. `reportActions` are easier to work with as a separate collection). +**Exception:** There are some [gotchas](https://github.com/expensify/react-native-onyx#merging-data) when working with complex nested array values in Onyx. So, this could be another valid reason to break a property off of its parent object (e.g. `reportActions` are easier to work with as a separate collection). If you're not sure whether something should have a collection key reach out in [`#expensify-open-source`](https://expensify.slack.com/archives/C01GTK53T8Q) for additional feedback. + +## Learning Resources + +### Quickest way to learn TypeScript + +- Get up to speed quickly + - [TypeScript playground](https://www.typescriptlang.org/play?q=231#example) + - Go though all examples on the playground. Click on "Example" tab on the top +- Handy Reference + - [TypeScript CheatSheet](https://www.typescriptlang.org/cheatsheets) + - [Type](https://www.typescriptlang.org/static/TypeScript%20Types-ae199d69aeecf7d4a2704a528d0fd3f9.png) + - [Control Flow Analysis](https://www.typescriptlang.org/static/TypeScript%20Control%20Flow%20Analysis-8a549253ad8470850b77c4c5c351d457.png) +- TypeScript with React + - [React TypeScript CheatSheet](https://react-typescript-cheatsheet.netlify.app/) + - [List of built-in utility types](https://react-typescript-cheatsheet.netlify.app/docs/basic/troubleshooting/utilities) + - [HOC CheatSheet](https://react-typescript-cheatsheet.netlify.app/docs/hoc/) diff --git a/contributingGuides/STYLING.md b/contributingGuides/STYLING.md index 3842d983725c..1c8e96bf0c08 100644 --- a/contributingGuides/STYLING.md +++ b/contributingGuides/STYLING.md @@ -4,7 +4,7 @@ Styles can either be theme-related or not. "Theme-related" means that a style contains some sort of color attributes (backgroundColor, color, borderColor). "Non-theme-related" styles may not contain no color attributes. -All non-theme-related styles must be defined in the `/styles` directory and `styles.js` contains the final export after gathering all appropriate styles. Unlike some React Native applications we are not using `StyleSheet.create()` and instead store styles as plain JS objects. There are also many helper styles available for direct use in components. +All non-theme-related styles must be defined in the `/styles` directory and `styles.js` contains the final export after gathering all appropriate styles. Unlike some React Native applications, we are not using `StyleSheet.create()` and instead store styles as plain JS objects. There are also many helper styles available for direct use in components. All styles that contain theme colors have to be defined in the `ThemeStylesProvider` component, as those need to be dynamically created and animated. @@ -14,7 +14,7 @@ These helper styles are loosely based on the [Bootstrap system of CSS utility he ## When to Create a New Style -If we need some minimal set of styling rules applied to a single-use component then it's almost always better to use an array of helper styles rather than create an entirely new style if it will only be used once. Resist the urge to create a new style for every new element added to a screen. There is a very good chance the style we are adding is a "single-use" style. +If we need some minimal set of styling rules applied to a single-use component, then it's almost always better to use an array of helper styles rather than create an entirely new style if it will only be used once. Resist the urge to create a new style for every new element added to a screen. There is a very good chance the style we are adding is a "single-use" style. ```jsx // Bad - Since we only use this style once in this component @@ -37,17 +37,17 @@ const TextWithPadding = props => ( ); ``` -On the other hand, if we are copying and pasting some chunks of JSX from one place to another then that might be a sign that we need a new reusable style. +On the other hand, if we are copying and pasting some chunks of JSX from one place to another, then that might be a sign that we need a new reusable style. ## Use the "Rule of Three" -In order to resist the urge to preoptimize and have many single-use components we've adopted a main principle: +In order to resist the urge to preoptimize and have many single-use components, we've adopted a main principle: Any array of styles associated with a single type of React element that has at least 3 identical usages should be refactored into: - A new resusable style that can be used in many places e.g. `styles.button` -- If that style has modifiers or style variations then those styles should follow a naming convention of `styles.elementModifer` e.g. `styles.buttonSuccess` -- If a reusable style has 3 or more modifiers it should be refactored into a component with props to modify the styles e.g. +- If that style has modifiers or style variations, then those styles should follow a naming convention of `styles.elementModifer` e.g. `styles.buttonSuccess` +- If a reusable style has 3 or more modifiers, it should be refactored into a component with props to modify the styles e.g. ```jsx + + )} ) : (