Skip to content

Commit ac82603

Browse files
authored
Merge pull request Expensify#70394 from margelo/@chrispader/react-compiler-rules-of-react-ci-linting
CI: Add "Rules of React" + React Compiler CI check and companion dev tool
2 parents e2c807f + c9f3183 commit ac82603

10 files changed

Lines changed: 743 additions & 14 deletions

File tree

.github/actions/javascript/getPullRequestIncrementalChanges/index.js

Lines changed: 149 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12528,14 +12528,26 @@ function isEmptyObject(obj) {
1252812528
/***/ }),
1252912529

1253012530
/***/ 7037:
12531-
/***/ ((__unused_webpack_module, exports, __nccwpck_require__) => {
12531+
/***/ (function(__unused_webpack_module, exports, __nccwpck_require__) {
1253212532

1253312533
"use strict";
1253412534

12535+
var __importDefault = (this && this.__importDefault) || function (mod) {
12536+
return (mod && mod.__esModule) ? mod : { "default": mod };
12537+
};
1253512538
Object.defineProperty(exports, "__esModule", ({ value: true }));
12539+
exports.GIT_ERRORS = void 0;
12540+
const github_1 = __nccwpck_require__(5438);
1253612541
const child_process_1 = __nccwpck_require__(2081);
1253712542
const util_1 = __nccwpck_require__(3837);
12543+
const GithubUtils_1 = __importDefault(__nccwpck_require__(9296));
12544+
const Logger_1 = __nccwpck_require__(8891);
1253812545
const exec = (0, util_1.promisify)(child_process_1.exec);
12546+
const IS_CI = process.env.CI === 'true';
12547+
const GIT_ERRORS = {
12548+
FAILED_TO_FETCH_FROM_REMOTE: 'Failed to fetch from remote',
12549+
};
12550+
exports.GIT_ERRORS = GIT_ERRORS;
1253912551
/**
1254012552
* Utility class for git operations.
1254112553
*/
@@ -12774,10 +12786,146 @@ class Git {
1277412786
throw new Error(`Failed to fetch git reference ${ref}: ${error instanceof Error ? error.message : String(error)}`);
1277512787
}
1277612788
}
12789+
static getMainBaseCommitHash(remote) {
12790+
// Fetch the main branch from the specified remote to ensure it's available
12791+
try {
12792+
(0, child_process_1.execSync)(`git fetch ${remote} main --no-tags --depth=1 -q`, { encoding: 'utf8' });
12793+
}
12794+
catch (error) {
12795+
throw new Error(GIT_ERRORS.FAILED_TO_FETCH_FROM_REMOTE);
12796+
}
12797+
// In CI, use a simpler approach - just use the remote main branch directly
12798+
// This avoids issues with shallow clones and merge-base calculations
12799+
if (IS_CI) {
12800+
try {
12801+
const mergeBaseHash = (0, child_process_1.execSync)(`git rev-parse ${remote}/main`, { encoding: 'utf8' }).trim();
12802+
// Validate the output is a proper SHA hash
12803+
if (!mergeBaseHash || !/^[a-fA-F0-9]{40}$/.test(mergeBaseHash)) {
12804+
throw new Error(`git rev-parse returned unexpected output: ${mergeBaseHash}`);
12805+
}
12806+
return mergeBaseHash;
12807+
}
12808+
catch (error) {
12809+
(0, Logger_1.error)(`Failed to get commit hash for ${remote}/main:`, error);
12810+
throw new Error(`Could not get commit hash for ${remote}/main`);
12811+
}
12812+
}
12813+
// For local development, try to find the actual merge base
12814+
let mergeBaseHash;
12815+
try {
12816+
mergeBaseHash = (0, child_process_1.execSync)(`git merge-base ${remote}/main HEAD`, { encoding: 'utf8' }).trim();
12817+
}
12818+
catch (error) {
12819+
// If merge-base fails locally, fall back to using the remote main branch
12820+
try {
12821+
mergeBaseHash = (0, child_process_1.execSync)(`git rev-parse ${remote}/main`, { encoding: 'utf8' }).trim();
12822+
(0, Logger_1.error)(`Warning: Could not find merge base between ${remote}/main and HEAD. Using ${remote}/main as base.`);
12823+
}
12824+
catch (fallbackError) {
12825+
(0, Logger_1.error)(`Failed to find merge base with ${remote}/main:`, error);
12826+
(0, Logger_1.error)(`Fallback also failed:`, fallbackError);
12827+
throw new Error(`Could not determine merge base with ${remote}/main`);
12828+
}
12829+
}
12830+
// Validate the output is a proper SHA hash
12831+
if (!mergeBaseHash || !/^[a-fA-F0-9]{40}$/.test(mergeBaseHash)) {
12832+
throw new Error(`git merge-base returned unexpected output: ${mergeBaseHash}`);
12833+
}
12834+
return mergeBaseHash;
12835+
}
12836+
static async getChangedFiles(remote) {
12837+
if (IS_CI) {
12838+
const { data: changedFiles } = await GithubUtils_1.default.octokit.pulls.listFiles({
12839+
owner: 'Expensify',
12840+
repo: 'App',
12841+
// eslint-disable-next-line @typescript-eslint/naming-convention
12842+
pull_number: github_1.context.payload.pull_request?.number ?? 0,
12843+
});
12844+
return changedFiles.map((file) => file.filename);
12845+
}
12846+
try {
12847+
// Get files changed in the current branch/commit
12848+
const mainBaseCommitHash = this.getMainBaseCommitHash(remote);
12849+
// Get the diff output and check status
12850+
const gitDiffOutput = (0, child_process_1.execSync)(`git diff --diff-filter=AMR --name-only ${mainBaseCommitHash} HEAD`, {
12851+
encoding: 'utf8',
12852+
});
12853+
const files = gitDiffOutput.trim().split('\n');
12854+
return files;
12855+
}
12856+
catch (error) {
12857+
if (error instanceof Error && error.message === GIT_ERRORS.FAILED_TO_FETCH_FROM_REMOTE) {
12858+
throw error;
12859+
}
12860+
(0, Logger_1.error)('Could not determine changed files:', error);
12861+
throw error;
12862+
}
12863+
}
1277712864
}
1277812865
exports["default"] = Git;
1277912866

1278012867

12868+
/***/ }),
12869+
12870+
/***/ 8891:
12871+
/***/ ((__unused_webpack_module, exports) => {
12872+
12873+
"use strict";
12874+
12875+
Object.defineProperty(exports, "__esModule", ({ value: true }));
12876+
exports.bold = exports.formatLink = exports.success = exports.error = exports.note = exports.warn = exports.info = exports.log = void 0;
12877+
const COLOR_DIM = '\x1b[2m';
12878+
const COLOR_RESET = '\x1b[0m';
12879+
const COLOR_YELLOW = '\x1b[33m';
12880+
const COLOR_RED = '\x1b[31m';
12881+
const COLOR_GREEN = '\x1b[32m';
12882+
const COLOR_BOLD = '\x1b[1m';
12883+
const EMOJIS = {
12884+
// One column emojis need to be rendered with an extra space after to align with two column emojis
12885+
INFO: '▶️ ',
12886+
// Two column emojis can be rendered as-is
12887+
SUCCESS: '✅',
12888+
WARN: '⚠️',
12889+
ERROR: '🔴',
12890+
};
12891+
const log = (...args) => {
12892+
console.debug(...args);
12893+
};
12894+
exports.log = log;
12895+
const info = (...args) => {
12896+
const lines = [EMOJIS.INFO, ...args];
12897+
log(...lines);
12898+
};
12899+
exports.info = info;
12900+
const bold = (...args) => {
12901+
const lines = [COLOR_BOLD, ...args, COLOR_RESET];
12902+
log(...lines);
12903+
};
12904+
exports.bold = bold;
12905+
const success = (...args) => {
12906+
const lines = [`${EMOJIS.SUCCESS}${COLOR_GREEN}`, ...args, COLOR_RESET];
12907+
log(...lines);
12908+
};
12909+
exports.success = success;
12910+
const warn = (...args) => {
12911+
const lines = [`${EMOJIS.WARN}${COLOR_YELLOW}`, ...args, COLOR_RESET];
12912+
log(...lines);
12913+
};
12914+
exports.warn = warn;
12915+
const note = (...args) => {
12916+
const lines = [COLOR_DIM, ...args, COLOR_RESET];
12917+
log(...lines);
12918+
};
12919+
exports.note = note;
12920+
const error = (...args) => {
12921+
const lines = [`${EMOJIS.ERROR}${COLOR_RED}`, ...args, COLOR_RESET];
12922+
log(...lines);
12923+
};
12924+
exports.error = error;
12925+
const formatLink = (name, url) => `\x1b]8;;${url}\x1b\\${name}\x1b]8;;\x1b\\`;
12926+
exports.formatLink = formatLink;
12927+
12928+
1278112929
/***/ }),
1278212930

1278312931
/***/ 9491:

.github/workflows/preDeploy.yml

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,17 @@ jobs:
1616
prettier:
1717
uses: ./.github/workflows/prettier.yml
1818

19+
react-compiler-compliance:
20+
uses: ./.github/workflows/react-compiler-compliance.yml
21+
secrets: inherit
22+
1923
test:
2024
uses: ./.github/workflows/test.yml
2125
secrets: inherit
2226

2327
confirmPassingBuild:
2428
runs-on: ubuntu-latest
25-
needs: [typecheck, lint, test]
29+
needs: [typecheck, lint, react-compiler-compliance, test]
2630
if: ${{ always() }}
2731

2832
steps:
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
name: React Compiler Compliance Check
2+
3+
on:
4+
workflow_call:
5+
pull_request:
6+
types: [opened, synchronize]
7+
branches-ignore: [staging, production]
8+
paths: ['**.tsx']
9+
10+
concurrency:
11+
group: ${{ github.ref == 'refs/heads/main' && format('{0}-{1}', github.ref, github.sha) || github.ref }}-react-compiler-compliance
12+
cancel-in-progress: true
13+
jobs:
14+
react-compiler-compliance:
15+
name: React Compiler Compliance
16+
if: ${{ github.actor != 'OSBotify' }}
17+
runs-on: ubuntu-latest
18+
19+
steps:
20+
# v4
21+
- name: Checkout
22+
uses: actions/checkout@8ade135a41bc03ea155e62e844d188df1ea18608
23+
24+
- name: Setup Node
25+
uses: ./.github/actions/composite/setupNode
26+
27+
- name: Run React Compiler Compliance Check
28+
run: npm run react-compiler-compliance-check check-changed
29+
env:
30+
CI: true
31+
GITHUB_TOKEN: ${{ github.token }}

.gitignore

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,8 @@ web-build/
145145
# Jeykll
146146
docs/.bundle
147147

148-
# Output of react compiler healthcheck dev script
148+
# React Compiler Compliance Check output
149+
react-compiler-report.json
149150
react-compiler-output.txt
150151

151152
# Rock Framework

babel.config.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ const ReactCompilerConfig = {
77
environment: {
88
enableTreatRefLikeIdentifiersAsRefs: true,
99
},
10-
// We exclude 'tests' directory from compilation, but still compile components imported in test files.
1110
sources: (filename) => !filename.includes('tests/') && !filename.includes('node_modules/'),
1211
};
1312

contributingGuides/REACT_COMPILER.md

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,46 @@
66

77
At Expensify, we are early adopters of this tool and aim to fully leverage its capabilities.
88

9-
## React Compiler compatibility check
9+
## React Compiler compliance checker
1010

11-
To check if your code can be compiled by React Compiler and hence gets all its optimizations "for free", you can run the `npm run react-compiler-healthcheck-test` locally and analyze the output.
11+
We provide a script, `scripts/react-compiler-compliance-check.ts`, which checks for "Rules of React" compliance locally and enforces these in PRs adding or changing React code through a CI check.
12+
13+
### What it does
14+
15+
Runs `react-compiler-healthcheck` in verbose mode, parses output, and summarizes which files compiled and which failed, including file, line, column, and reason. It can:
16+
17+
- Check all files or a specific file/glob
18+
- Check only files changed relative to a base branch
19+
- Optionally generate a machine-readable report `react-compiler-report.json`
20+
- Exit with non-zero code when failures are found (useful for CI)
21+
22+
### Usage
23+
24+
> [!NOTE]
25+
> This script uses `origin` as the base remote by default. If your GH remote is named differently, use the `--remote <name>` flag.
26+
27+
#### Check entire codebase or a specific file/glob
28+
29+
```bash
30+
npm run react-compiler-compliance-check check # Check all files
31+
npm run react-compiler-compliance-check check src/path/Component.tsx # Check specific file
32+
npm run react-compiler-compliance-check check "src/**/*.tsx" # Check glob pattern
33+
```
34+
35+
#### Check only changed files against a remote main (default remote is `origin`)
36+
37+
```bash
38+
npm run react-compiler-compliance-check check-changed
39+
```
40+
41+
#### Generate a detailed report (saved as `./react-compiler-report.json`)
42+
43+
You can use the `--report` flag with both of the above commands:
44+
45+
```bash
46+
npm run react-compiler-compliance-check check --report
47+
npm run react-compiler-compliance-check check-changed --report
48+
```
1249

1350
## How can I check what exactly prevents file from successful optimization or whether my fix for passing `react-compiler` actually works?
1451

@@ -36,6 +73,7 @@ If you encounter this error, you need to add the `Ref` postfix to the variable n
3673
If you added a modification to `SharedValue`, you'll likely encounter this error. You can ignore this error for now because the current `react-native-reanimated` API is not compatible with `react-compiler` rules. Once [this PR](https://github.com/software-mansion/react-native-reanimated/pull/6312) is merged, we'll rewrite the code to be compatible with `react-compiler`. Until then, you can ignore this error.
3774

3875
### Existing manual memoization could not be preserved. [...]
76+
3977
These types of errors usually occur when the calls to `useMemo` that were made manually are too complex for react-compiler to understand. React compiler is still experimental so unfortunately this can happen.
4078

4179
Some specific cases of this error are described below.
@@ -96,7 +134,7 @@ return (
96134
<ToggleSettingOptionRow
97135
onToggle={item.onToggle}
98136
// ❌ such code triggers the error - `qboConfig?.pendingFields` is an external variable from the closure
99-
// so this code is pretty complicated for `react-compiler` optimizations
137+
// so this code is pretty complicated for `react-compiler` optimizations
100138
pendingAction={settingsPendingAction([item.subscribedSetting], qboConfig?.pendingFields)}
101139
// ❌ such code triggers the error - `qboConfig` is an external variable from the closure
102140
errors={ErrorUtils.getLatestErrorField(qboConfig, item.subscribedSetting)}

package.json

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,7 @@
7272
"gh-actions-unused-styles": "npx ts-node scripts/findUnusedStyles.ts",
7373
"setup-https": "mkcert -install && mkcert -cert-file config/webpack/certificate.pem -key-file config/webpack/key.pem dev.new.expensify.com localhost 127.0.0.1",
7474
"e2e-test-runner-build": "node --max-old-space-size=8192 node_modules/.bin/ncc build tests/e2e/testRunner.ts -o tests/e2e/dist/",
75-
"react-compiler-healthcheck": "react-compiler-healthcheck --verbose",
76-
"react-compiler-healthcheck-test": "react-compiler-healthcheck --verbose &> react-compiler-output.txt",
75+
"react-compiler-compliance-check": "ts-node scripts/react-compiler-compliance-check.ts",
7776
"generate-search-parser": "peggy --format es -o src/libs/SearchParser/searchParser.js src/libs/SearchParser/searchParser.peggy src/libs/SearchParser/baseRules.peggy",
7877
"generate-autocomplete-parser": "peggy --format es -o src/libs/SearchParser/autocompleteParser.js src/libs/SearchParser/autocompleteParser.peggy src/libs/SearchParser/baseRules.peggy && ./scripts/parser-workletization.sh src/libs/SearchParser/autocompleteParser.js",
7978
"web:prod": "http-server ./dist --cors",

0 commit comments

Comments
 (0)