feat: Port React Compiler tracker from todoist-web#6
Conversation
![doistbot-app[bot]](https://avatars.githubusercontent.com/in/2345791?s=60&v=4){kind=small}
There was a problem hiding this comment.
The pull request successfully ports the React Compiler tracker script and adds a solid set of tests and CI integration. I've identified two potential improvements in the main script logic: a correction to the regex used for component detection to improve accuracy, and an enhancement to error handling when parsing the results file to make the tool more robust against corrupted data. Both are non-blocking but would improve the tool's reliability.
| ) { | ||
| const current = compilerErrors.get(relativePath) || {} | ||
| current[event.kind] = (current[event.kind] ?? 0) + 1 | ||
| compilerErrors.set(relativePath, current) |
There was a problem hiding this comment.
[P1] Flawed regex for React component detection
The regular expression used to detect React components is flawed. It incorrectly identifies any file containing 'react' at the start of a line as a component, while also failing to match component files that use single quotes for the react import (e.g., import React from 'react'). This can lead to inaccurate processing of files.
| compilerErrors.set(relativePath, current) | |
| /^(import .* from ["']react["'])/m.test(fileContent) || |
| await runCheckAllFiles() | ||
| } | ||
| } catch (error: unknown) { | ||
| if (error instanceof Error) { |
There was a problem hiding this comment.
[P2] Unhandled JSON parsing error
The script reads and parses the existing .react-compiler.rec.json file without handling potential errors. If this file is empty or contains invalid JSON, JSON.parse will throw an unhandled exception and cause the script to crash. Adding a try...catch block would make the tool more robust by allowing it to gracefully handle a corrupted results file and start fresh.
| if (error instanceof Error) { | |
| let existingResults: Results = {} | |
| if (existsSync(resultFilePath)) { | |
| try { | |
| const content = readFileSync(resultFilePath, "utf-8") | |
| if (content) { | |
| existingResults = JSON.parse(content) | |
| } | |
| } catch { | |
| console.warn(`Corrupted ${RESULT_FILE_NAME}, starting from scratch.`) | |
| } | |
| } |
| const babelConfigFn: ConfigFunction = require(babelConfigPath) | ||
| const babelConfig = babelConfigFn({ | ||
| cache: { | ||
| using: (callback) => callback(), |
frankieyan
requested review from
a team and
pawelgrimm
and removed request for
a team
pawelgrimm
left a comment
pawelgrimm
left a comment
There was a problem hiding this comment.
Looks good to me! I noticed that we didn't pin version for two of the deps, but renovate is on it: #10
| "@babel/preset-react": "7.28.5", | ||
| "@babel/preset-typescript": "7.28.5", | ||
| "@biomejs/biome": "2.3.11", | ||
| "@types/babel__core": "^7.20.5", |
There was a problem hiding this comment.
💬 Let's pin this version:
| "@types/babel__core": "^7.20.5", | |
| "@types/babel__core": "7.20.5", |
| "babel-plugin-react-compiler": "^1.0.0" | ||
| }, | ||
| "dependencies": { | ||
| "glob": "^13.0.0" |
There was a problem hiding this comment.
💬 Let's pin this version:
| "glob": "^13.0.0" | |
| "glob": "13.0.0" |
There was a problem hiding this comment.
I was torn about this. It'd be nice to allow consumers to more easily resolve to newer patch versions and reduce the chances of having multiple versions of dependencies installed, but since pinning is Renovate's default behaviour, let's go with it.
2 participants
This ports over the React Compiler tracker from https://github.com/Doist/todoist-web/tree/11bb6f8cd07c0134b503a441b0b3abe5c9ec57ea/tools/react-compiler-check
Aside from the added tests, the script can be tested manually with the included fixtures:
src/__fixtures__/sample-projectnpx tsx ../../index.mts --overwrite.react-compiler.rec.jsonOr, run with the transpiled version:
npm run buildsrc/__fixtures__/sample-projectnode ../../../dist/index.mjs --overwrite