feat: 新增内存 benchmark & update version #21
Conversation
WalkthroughThis update introduces new configuration files for Rolldown and Rolldown-Vite, removes several Babel-related and Mako configuration files, and significantly refactors the benchmarking script into a modular, extensible performance and memory benchmarking tool. Package scripts and dependencies are updated, notably upgrading React to v19 and adding new build tooling. Webpack and Rspack configurations are enhanced for optimization and modern browser targeting. Additional changes include npm registry configuration, source map conditional settings in multiple configs, and utility function improvements. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BenchmarkScript
participant BuildTool
participant MemoryMonitor
participant Puppeteer
User->>BenchmarkScript: Run benchmark
BenchmarkScript->>BuildTool: Detect and configure tools
BenchmarkScript->>BuildTool: Start server/build process
BuildTool->>MemoryMonitor: Start memory monitoring
BenchmarkScript->>Puppeteer: Launch browser for HMR tests
BenchmarkScript->>BuildTool: Trigger builds/HMR cycles
BuildTool->>MemoryMonitor: Collect memory stats
BenchmarkScript->>BuildTool: Stop server/cleanup
BenchmarkScript->>MemoryMonitor: Stop and aggregate memory data
BenchmarkScript->>User: Output performance and memory results
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (5)
📒 Files selected for processing (16)
💤 Files with no reviewable changes (5)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (5)
🧰 Additional context used🧬 Code Graph Analysis (1)rsbuild.config.ts (1)
🪛 Biome (1.9.4)benchmark.mjs[error] 1002-1280: Promise executor functions should not be (lint/suspicious/noAsyncPromiseExecutor) [error] 1297-1448: Promise executor functions should not be (lint/suspicious/noAsyncPromiseExecutor) utils.mjs[error] 14-14: Unexpected control character in a regular expression. Control characters are unusual and potentially incorrect inputs, so they are disallowed. (lint/suspicious/noControlCharactersInRegex) 🔇 Additional comments (7)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (6)
utils.mjs (1)
153-161: Consider the trade-offs of silencing cache deletion logs.While commenting out these logs reduces noise, it may make debugging cache-related issues more difficult. Consider using a debug log level instead of completely removing the logging.
- // logger.info(`Deleted cache folder: ${folderPath}`); + logger.debug(`Deleted cache folder: ${folderPath}`);vite.config.ts (2)
3-3: Remove unused import.The
pathmodule is imported but never used in this file.-import path from 'path';
7-7: Simplify the boolean expression.The ternary operator is unnecessary here. The comparison already returns a boolean value.
- sourcemap: process.env.NODE_ENV === 'development' ? true : false, + sourcemap: process.env.NODE_ENV === 'development',🧰 Tools
🪛 Biome (1.9.4)
[error] 7-7: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
rolldown-vite.mjs (1)
1-1: Remove unused import.The
pathmodule is imported but never used in this configuration.-import path from 'node:path';webpack.config.js (1)
9-9: Remove unusedcaseNamevariableThe
caseNamevariable is defined but never used in the configuration.-const caseName = process.env.CASE ?? 'medium';benchmark.mjs (1)
278-340: Simplify Unix process finding logicThe nested exec calls and callback handling in the Unix branch could be simplified for better readability and maintainability.
Consider using promises with async/await pattern or extracting the parent PID checking logic into a separate function:
async function findChildProcessByParentPid(candidatePids, parentPid) { for (const pid of candidatePids) { const ppid = await getParentPid(pid); if (ppid === parentPid) return pid; } return null; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
build.pngis excluded by!**/*.pngfull.pngis excluded by!**/*.pnghmr.pngis excluded by!**/*.pngpnpm-lock.yamlis excluded by!**/pnpm-lock.yamlstartup.pngis excluded by!**/*.png
📒 Files selected for processing (15)
.npmrc(1 hunks)benchmark.mjs(1 hunks)farm.config.mjs(1 hunks)mako.config.json(0 hunks)next.config.js(0 hunks)package.json(2 hunks)public-mako/index.html(0 hunks)rolldown-vite.mjs(1 hunks)rolldown.config.mjs(1 hunks)rspack.config.js(2 hunks)utils.mjs(3 hunks)vite.babel.config.ts(0 hunks)vite.config.ts(1 hunks)webpack.babel.config.js(0 hunks)webpack.config.js(1 hunks)
💤 Files with no reviewable changes (5)
- mako.config.json
- next.config.js
- vite.babel.config.ts
- public-mako/index.html
- webpack.babel.config.js
🧰 Additional context used
🪛 Biome (1.9.4)
vite.config.ts
[error] 7-7: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
(lint/complexity/noUselessTernary)
utils.mjs
[error] 14-14: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
benchmark.mjs
[error] 1002-1280: Promise executor functions should not be async.
(lint/suspicious/noAsyncPromiseExecutor)
[error] 1297-1448: Promise executor functions should not be async.
(lint/suspicious/noAsyncPromiseExecutor)
🔇 Additional comments (10)
.npmrc (1)
1-1: Consider the implications of using an alternative npm registry.Using
registry.npmmirror.cominstead of the default npm registry has several considerations:
- Security: Ensure the mirror registry is trusted and regularly synchronized
- Availability: The build process becomes dependent on this third-party service
- Team collaboration: All developers and CI/CD systems will use this registry
- Package integrity: Verify that package checksums and signatures are properly maintained
Consider documenting this choice in the README or using
.nvmrcfor environment-specific configurations.rspack.config.js (3)
24-24: Good practice: Excluding node_modules from transpilation.Excluding
node_modulesfrom the loader improves build performance by avoiding unnecessary transpilation of already-processed dependencies.
30-36: Modern browser targets are well-chosen.The specified browser targets (Chrome ≥87, Edge ≥88, Firefox ≥78, Safari ≥14) represent modern browsers with good ES2020+ support, enabling more efficient output while maintaining reasonable compatibility.
61-61: Incremental builds enhance development experience.Enabling incremental builds only in development mode is a good optimization that can significantly improve rebuild times during development.
utils.mjs (2)
15-29: Well-designed data normalization function.The
normalizeChartDatafunction properly handles both array and object inputs, and applies ANSI cleaning to keys. The defensive programming approach is commendable.
225-250: Excellent defensive programming in chart data generation.The refactored approach to conditionally include only existing fields prevents undefined values from appearing in chart data, improving robustness.
farm.config.mjs (1)
7-7:❓ Verification inconclusive
Verify the impact of disabling presetEnv.
Disabling
presetEnvmeans Farm won't apply automatic environment-specific transformations. Ensure this aligns with your browser support requirements and that other tools handle necessary polyfills and syntax transforms.
🏁 Script executed:
#!/bin/bash # Check if there are any modern JavaScript features that might need transformation echo "Checking for modern JavaScript features that might need transformation..." # Check for optional chaining rg -n "\\?\\." --type js --type ts --type jsx --type tsx # Check for nullish coalescing rg -n "\\?\\?" --type js --type ts --type jsx --type tsx # Check for private class fields rg -n "#[a-zA-Z]" --type js --type ts --type jsx --type tsx # Check for top-level await rg -n "^\\s*await\\s" --type js --type ts --type jsx --type tsxLength of output: 508
🏁 Script executed:
#!/bin/bash echo "Rechecking modern JS features across .js,.jsx,.ts,.tsx,.mjs files..." # Define globs globs="*.{js,jsx,ts,tsx,mjs}" echo "[1] Optional chaining (?.):" rg -n "\?\." -g "$globs" || echo "No optional chaining found" echo "[2] Nullish coalescing (??):" rg -n "\?\?" -g "$globs" || echo "No nullish coalescing found" echo "[3] Private class fields (#field):" rg -n "#[[:alpha:]]" -g "$globs" || echo "No private class fields found" echo "[4] Top-level await:" rg -n "^[[:space:]]*await[[:space:]]" -g "$globs" || echo "No top-level await found"Length of output: 2881
Review farm.config.mjs: Confirm compatibility with modern JS features
Disabling
presetEnvmeans Farm won’t transpile or polyfill new syntax. We’ve detected several ES2020+ features that your targets must natively support or be handled elsewhere:• Optional chaining in
benchmark.mjs(e.g. lines 1264–1265)
• Nullish coalescing inwebpack.config.js(line 9) androlldown.config.mjs(line 5)
• Top-levelawaitacrossutils.mjs(e.g. lines 36, 131–152, 170) and many uses inbenchmark.mjsPlease verify that all deployment environments and build pipelines can handle these features without Farm’s automatic transforms.
webpack.config.js (2)
18-40: Excellent migration to SWC loaderThe migration from Babel to SWC is a great performance improvement. The configuration properly handles TypeScript, React automatic runtime, and development-specific features.
42-72: Well-configured CSS extraction and optimizationThe setup properly extracts CSS files in production and includes appropriate minification for both CSS and JavaScript using SWC-based minifiers. The conditional optimization based on
isProdis correctly implemented.benchmark.mjs (1)
78-87: Well-structured test configurationThe TEST_CONFIG object provides clear, centralized control over test behavior with sensible defaults. The separation between performance and memory tests is well-designed.
| // 添加函数来清理ANSI转义序列 | ||
| function cleanVersionName(versionName) { | ||
| // 移除ANSI转义序列 | ||
| return versionName.replace(/\x1B\[[0-9;]*m/g, ''); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix the ANSI escape sequence regex pattern.
The regex contains a control character that static analysis tools flag as potentially problematic. Use a more explicit pattern for better compatibility.
-function cleanVersionName(versionName) {
- // 移除ANSI转义序列
- return versionName.replace(/\x1B\[[0-9;]*m/g, '');
-}
+function cleanVersionName(versionName) {
+ // 移除ANSI转义序列
+ return versionName.replace(/\u001B\[[0-9;]*m/g, '');
+}Using \u001B instead of \x1B provides the same functionality while avoiding control character warnings.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // 添加函数来清理ANSI转义序列 | |
| function cleanVersionName(versionName) { | |
| // 移除ANSI转义序列 | |
| return versionName.replace(/\x1B\[[0-9;]*m/g, ''); | |
| } | |
| // 添加函数来清理ANSI转义序列 | |
| function cleanVersionName(versionName) { | |
| // 移除ANSI转义序列 | |
| return versionName.replace(/\u001B\[[0-9;]*m/g, ''); | |
| } |
🤖 Prompt for AI Agents
In utils.mjs around lines 8 to 12, the regex pattern for removing ANSI escape
sequences uses \x1B, which triggers static analysis warnings due to the control
character. Replace \x1B with the Unicode escape \u001B in the regex to maintain
the same functionality while avoiding these warnings.
| experimental: { | ||
| enableNativePlugin: 'resolver', | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify experimental configuration validity.
Please ensure that enableNativePlugin: 'resolver' is a valid configuration option for rolldown-vite.
🌐 Web query:
What are the valid experimental configuration options for rolldown-vite enableNativePlugin?
💡 Result:
The rolldown-vite package offers an experimental configuration option that includes several experimental features. One of these features is enableNativePlugin, which, when set to true, enables the native plugin system within Rolldown. This system is designed to improve performance and compatibility by leveraging native code.
To enable this feature, you can add the following to your Vite configuration:
export default {
experimental: {
enableNativePlugin: true,
},
};Please note that this feature is experimental and may not be suitable for all projects. It's recommended to test thoroughly in your development environment before enabling it in production.
For more detailed information on the available experimental configuration options in rolldown-vite, you can refer to the official Rolldown documentation. (rolldown.rs)
Citations:
Fix invalid experimental.enableNativePlugin value
The enableNativePlugin option only accepts a boolean (true) to enable the native plugin system—not a string. Please update your Vite config accordingly:
• File: rolldown-vite.mjs (around lines 7–9)
experimental: {
- enableNativePlugin: 'resolver',
+ enableNativePlugin: true,
}This ensures you’re using the feature as documented in the official Rolldown reference.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| experimental: { | |
| enableNativePlugin: 'resolver', | |
| } | |
| experimental: { | |
| enableNativePlugin: true, | |
| } |
🤖 Prompt for AI Agents
In rolldown-vite.mjs around lines 7 to 9, the experimental configuration sets
enableNativePlugin to the string 'resolver', which is invalid. Update the value
of enableNativePlugin to the boolean true to correctly enable the native plugin
system as per the official Rolldown documentation.
|
|
||
| export default defineConfig({ | ||
| input: { | ||
| main: path.join(import.meta.dirname, './src', caseName, 'index.jsx'), |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify Node.js version compatibility for import.meta.dirname.
import.meta.dirname requires Node.js 20.11.0 or later. Please ensure your project's minimum Node.js version supports this feature.
🏁 Script executed:
#!/bin/bash
# Check if package.json specifies Node.js version requirements
if [ -f "package.json" ]; then
echo "Checking package.json for Node.js version requirements:"
jq -r '.engines.node // "No Node.js version specified"' package.json
fi
# Check if .nvmrc exists
if [ -f ".nvmrc" ]; then
echo "Found .nvmrc:"
cat .nvmrc
fi
# Check for any other Node version specifications
fd -t f ".*" | xargs rg -l "node.*version|version.*node" --ignore-caseLength of output: 354
Specify minimum Node.js version for import.meta.dirname usage
The import.meta.dirname feature requires Node.js 20.11.0 or later, but your project currently doesn’t declare any minimum Node.js version:
• package.json engines.node: not specified
• No .nvmrc file found
Please add a Node.js engine requirement (e.g., in package.json) or an .nvmrc to ensure all environments support this feature. For example:
// package.json
{
"engines": {
"node": ">=20.11.0"
}
}Or create a .nvmrc with:
20.11.0
This will prevent compatibility issues when bundling with Rollup.
🤖 Prompt for AI Agents
In rolldown.config.mjs at line 9, you are using import.meta.dirname which
requires Node.js 20.11.0 or later, but your project does not currently specify a
minimum Node.js version. To fix this, add a Node.js engine requirement in your
package.json file by including "engines": { "node": ">=20.11.0" }, or create a
.nvmrc file with the content "20.11.0" to ensure all environments support this
feature and avoid compatibility issues.
package.json
Outdated
| "start:rolldown-vite": "node ./node_modules/rolldown-vite/bin/vite.js -c rolldown-vite.config.mjs", | ||
| "build:rolldown-vite": "node ./node_modules/rolldown-vite/bin/vite.js build -c rolldown-vite.config.mjs", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify rolldown-vite script paths.
The scripts directly reference ./node_modules/rolldown-vite/bin/vite.js. Please confirm this is the correct binary path for rolldown-vite.
🏁 Script executed:
#!/bin/bash
# Check if rolldown-vite binary exists at the specified path
echo "Checking for rolldown-vite binary:"
if [ -f "./node_modules/rolldown-vite/bin/vite.js" ]; then
echo "✓ Binary exists at ./node_modules/rolldown-vite/bin/vite.js"
else
echo "✗ Binary not found at ./node_modules/rolldown-vite/bin/vite.js"
echo "Available binaries in rolldown-vite:"
find ./node_modules/rolldown-vite -name "*.js" -path "*/bin/*" 2>/dev/null || echo "No binaries found"
fi
# Check package.json bin field for rolldown-vite
echo "Checking rolldown-vite package.json bin field:"
jq '.bin' ./node_modules/rolldown-vite/package.json 2>/dev/null || echo "Could not read rolldown-vite package.json"Length of output: 708
Incorrect rolldown-vite binary path detected
The check shows there is no ./node_modules/rolldown-vite/bin/vite.js (nor any bin/*.js) and the package’s own package.json couldn’t be read—so the script path is invalid. Please either install the package or update your npm scripts to call the correct CLI entrypoint. For example, if rolldown-vite is listed in your dependencies, you can simplify to:
"scripts": {
"start:rolldown-vite": "rolldown-vite -c rolldown-vite.config.mjs",
"build:rolldown-vite": "rolldown-vite build -c rolldown-vite.config.mjs"
}• Ensure you’ve run npm install --save-dev rolldown-vite so the binary is available under node_modules/.bin/.
• Remove the hard-coded path to node_modules/.../bin/vite.js—npm scripts automatically resolve binaries in .bin.
• If the package’s entry file is different (e.g. a .cjs or located under dist/), point to that instead.
🤖 Prompt for AI Agents
In package.json lines 14 to 15, the scripts reference a non-existent binary path
./node_modules/rolldown-vite/bin/vite.js for rolldown-vite. To fix this, remove
the hard-coded path and update the scripts to simply use "rolldown-vite" and
"rolldown-vite build" commands, relying on npm to resolve the binary from
node_modules/.bin/. Also, ensure rolldown-vite is installed as a dev dependency
with npm install --save-dev rolldown-vite so the binary is available.
| "react": "^19.1.0", | ||
| "react-dom": "^19.1.0", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify React 19 compatibility.
The upgrade from React 18 to React 19 includes significant changes. Please ensure all React-related code and dependencies are compatible with React 19.
🌐 Web query:
What are the breaking changes in React 19 and how to migrate from React 18?
💡 Result:
React 19 introduces several significant improvements that require some breaking changes from React 18. To ensure a smooth transition, it's recommended to follow a structured migration approach.
Breaking Changes in React 19
ReactDOM Rendering API Changes
ReactDOM.renderis completely removed in React 19. This method was deprecated in React 18 with warnings and is now officially unsupported[7].ReactDOM.hydratehas been removed and needs to be replaced withReactDOM.hydrateRootfor server-side rendering applications[7].
Component API Changes
- String refs are no longer supported and must be replaced with callback refs or the createRef API[9].
- The
actimport location has changed, requiring updates to test files[9]. useFormStatehas been replaced with new form handling hooks[9].
Context API Changes
<Context.Provider>syntax is being phased out in favor of the new<Context>syntax as a provider. While it still works in React 19, it will be deprecated in future versions[1].
Ref Handling
- The way refs are handled has changed significantly. Function components can now accept
refas a prop directly without needing to useforwardRef[1].
Migration Guide
Step 1: Upgrade to React 18.3 First
Before jumping directly to React 19, it's recommended to upgrade to React 18.3 first:
npm install --save-exact [email protected] [email protected]React 18.3 is identical to 18.2 but adds warnings for deprecated APIs that will be removed in React 19, helping you identify potential issues[10][6].
Step 2: Update Root Rendering Method
Replace the legacy rendering method with the new createRoot API:
// Before
import {render} from 'react-dom';
render(<App />, document.getElementById('root'));
// After
import {createRoot} from 'react-dom/client';
const root = createRoot(document.getElementById('root'));
root.render(<App />);For server-side rendering applications, update hydration method:
// Before
import {hydrate} from 'react-dom';
hydrate(<App />, document.getElementById('root'));
// After
import {hydrateRoot} from 'react-dom/client';
hydrateRoot(document.getElementById('root'), <App />);Step 3: Install React 19
After addressing all warnings and making necessary changes, install React 19:
npm install --save-exact react@19 react-dom@19Or if using Yarn:
yarn add --exact react@19 react-dom@19If you're using TypeScript, you'll need to update the types as well[9].
Step 4: Run Codemods
React provides codemods to automate many of the required changes. Run the React 19 migration recipe:
npx codemod@latest react/19/migration-recipeThis recipe includes codemods that automatically fix many breaking changes, including[9]:
- Replacing ReactDOM render
- Replacing string refs
- Updating act imports
- Replacing useFormState
- Handling prop-types with TypeScript
Step 5: Update Component Refs
Take advantage of the new ref prop pattern instead of using forwardRef:
// Before
const MyInput = forwardRef(({placeholder}, ref) => {
return <input placeholder={placeholder} ref={ref} />;
});
// After
function MyInput({placeholder, ref}) {
return <input placeholder={placeholder} ref={ref} />;
}React 19 will be publishing a codemod to automatically update components to use the new ref prop approach[1].
Step 6: Update Context Providers
Start using the new Context provider syntax:
// Before
const ThemeContext = createContext('');
function App({children}) {
return (
<ThemeContext.Provider value="dark">
{children}
</ThemeContext.Provider>
);
}
// After
const ThemeContext = createContext('');
function App({children}) {
return (
<ThemeContext value="dark">
{children}
</ThemeContext>
);
}A codemod is also available to convert existing providers to the new syntax[1].
By following these steps and leveraging the automated tools provided, you can successfully migrate from React 18 to React 19 while minimizing disruption to your application.
Citations:
- 1: https://react.dev/blog/2024/12/05/react-19
- 2: https://colorwhistle.com/latest-react-features/
- 3: https://www.wisp.blog/blog/react-191-is-out-heres-what-you-need-to-know
- 4: https://tina.io/blog/react-19-support
- 5: https://weqtechnologies.com/react-19-features-updates-2025-whats-new-why-it-matters/
- 6: https://www.youtube.com/watch?v=fr376XAVFPg
- 7: https://javascript.plainenglish.io/react-v18-to-v19-upgrade-guide-for-production-level-projects-c62986f0f6f6
- 8: https://knock.app/changelog/2025-03-13
- 9: https://docs.codemod.com/guides/migrations/react-18-19
- 10: https://react.dev/blog/2024/04/25/react-19-upgrade-guide
Action Required: Complete React 19 Migration Steps
Before merging the React 19 upgrade, please apply the following breaking-change adjustments:
• Update your client entry point (e.g. src/index.js):
- Replace
with
import { render } from 'react-dom'; render(<App />, rootEl);
import { createRoot } from 'react-dom/client'; const root = createRoot(rootEl); root.render(<App />);
• For SSR/hydration (if applicable):
- Swap
with
import { hydrate } from 'react-dom'; hydrate(<App />, rootEl);
import { hydrateRoot } from 'react-dom/client'; hydrateRoot(rootEl, <App />);
• Refactor all string refs to use React.createRef() or callback refs.
• Adjust tests to import act from 'react-dom/test-utils' (or 'react/test-utils') per React 19.
• Run the official React 19 codemod to automate many updates:
npx codemod@latest react/19/migration-recipe• Finally, lock in exact versions and reinstall:
npm install --save-exact [email protected] [email protected]After these changes, run your full test suite and smoke-test key pages to confirm compatibility.
🤖 Prompt for AI Agents
In package.json lines 23-24, you have updated React and ReactDOM to version
19.1.0, but the codebase likely still uses React 18 APIs that are incompatible
with React 19. To fix this, update your client entry point to replace
ReactDOM.render with createRoot from 'react-dom/client', and if using SSR,
replace hydrate with hydrateRoot. Refactor all string refs to use createRef or
callback refs, update test imports for act, and run the official React 19
codemod to automate these changes. Finally, lock exact versions and reinstall
dependencies, then thoroughly test your application for compatibility before
merging.
| }, | ||
| cache: { | ||
| type: "filesystem", | ||
| extensions: ['.ts', '.tsx', '.js', '.jsx', '.ts', '.tsx'], |
There was a problem hiding this comment.
Remove duplicate extensions in resolve.extensions array
The extensions array contains duplicate entries for .ts and .tsx.
- extensions: ['.ts', '.tsx', '.js', '.jsx', '.ts', '.tsx'],
+ extensions: ['.ts', '.tsx', '.js', '.jsx'],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| extensions: ['.ts', '.tsx', '.js', '.jsx', '.ts', '.tsx'], | |
| extensions: ['.ts', '.tsx', '.js', '.jsx'], |
🤖 Prompt for AI Agents
In webpack.config.js at line 14, the resolve.extensions array contains duplicate
entries for '.ts' and '.tsx'. Remove the duplicate '.ts' and '.tsx' entries so
that each extension appears only once in the array.
| hackBinFile() { | ||
| try { | ||
| if (!existsSync(this.fullBinPath)) { | ||
| logger.warn(`⚠️ Executable not found: ${this.fullBinPath}`); | ||
| return; | ||
| } | ||
|
|
||
| const binFileContent = readFileSync(this.fullBinPath, 'utf-8'); | ||
|
|
||
| // 检查是否已有 process.title 设置 | ||
| const existingTitleMatch = binFileContent.match( | ||
| /process\.title\s*=\s*['"](.*?)['"]/ | ||
| ); | ||
| let processTitle; | ||
| let processMarker; | ||
|
|
||
| if (existingTitleMatch) { | ||
| // 使用现有的 process.title | ||
| processTitle = existingTitleMatch[1]; | ||
| processMarker = null; // 不需要注入新的 | ||
| } else { | ||
| console.log(this.name); | ||
| if (this.name.includes('Rsbuild')) { | ||
| processTitle = 'rsbuild-node'; | ||
| processMarker = null; | ||
| } else { | ||
| // 注入新的 process.title | ||
| processTitle = `benchmark-${this.originalName.toLowerCase()}`; | ||
| processMarker = `process.title = '${processTitle}';`; | ||
| } | ||
| } | ||
|
|
||
| let modified = false; | ||
| let newContent = binFileContent; | ||
|
|
||
| if (!binFileContent.includes(startConsole)) { | ||
| const lines = newContent.split('\n'); | ||
| lines.splice(1, 0, startConsole); | ||
| newContent = lines.join('\n'); | ||
| modified = true; | ||
| } | ||
|
|
||
| if (processMarker && !binFileContent.includes(processMarker)) { | ||
| const lines = newContent.split('\n'); | ||
| lines.splice(1, 0, processMarker); | ||
| newContent = lines.join('\n'); | ||
| modified = true; | ||
| } | ||
|
|
||
| if (modified) { | ||
| writeFileSync(this.fullBinPath, newContent); | ||
| if (processMarker) { | ||
| logger.info( | ||
| `(run ${this.script} failed) child process exited with code ${code}`, | ||
| `🔧 Injected process marker '${processTitle}' into ${bold( | ||
| this.name | ||
| )}`, | ||
| { | ||
| name: this.name, | ||
| brandColor: brandColor.get(this.name), | ||
| brandColor: this.brandColor.get(this.name), | ||
| } | ||
| ); | ||
| reject(code); | ||
| } else { | ||
| logger.info(`🔧 Injected start console into ${bold(this.name)}`, { | ||
| name: this.name, | ||
| brandColor: this.brandColor.get(this.name), | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| this.processTitle = processTitle; | ||
| } catch (error) { | ||
| logger.warn(`⚠️ Cannot modify executable: ${error.message}`); | ||
| this.processTitle = null; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid modifying files in node_modules
The hackBinFile method modifies executables in node_modules, which is fragile and will break on reinstalls. Consider these alternatives:
- Use wrapper scripts that set process titles before launching the actual tools
- Use process monitoring tools that can track child processes by command line
- Fork the process with a custom NODE_OPTIONS environment variable
Would you like me to provide an implementation using wrapper scripts that doesn't require modifying node_modules?
🤖 Prompt for AI Agents
In benchmark.mjs around lines 694 to 768, the hackBinFile method modifies
executable files directly in node_modules, which is fragile and breaks on
reinstall. Instead of modifying these files, refactor to use wrapper scripts
that set the desired process.title before launching the actual executables.
Create separate wrapper scripts that invoke the original binaries with the
necessary environment or arguments, avoiding direct file changes in
node_modules. This approach maintains stability and avoids issues on package
reinstall or updates.
| let hmrRootStart = -1; | ||
| let hmrLeafStart = -1; | ||
|
|
||
| return new Promise(async (resolve) => { |
There was a problem hiding this comment.
Remove async from Promise executor in testHMR
Same anti-pattern as in testWatchMemory - async Promise executors should be avoided.
- return new Promise(async (resolve) => {
+ return new Promise((resolve) => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return new Promise(async (resolve) => { | |
| return new Promise((resolve) => { |
🤖 Prompt for AI Agents
In benchmark.mjs at line 1261, the Promise executor function is marked as async,
which is an anti-pattern. Remove the async keyword from the Promise executor
function and handle any asynchronous operations inside the executor using
standard Promise methods or async functions called within the executor, ensuring
the Promise resolves correctly without using an async executor.
| async function testWatchMemory(buildTool) { | ||
| console.log('\n'); | ||
| logger.info( | ||
| `🧪 ${bold( | ||
| buildTool.name | ||
| )} (${bold( | ||
| TEST_CONFIG.MEMORY_WATCH_CYCLES | ||
| )} modifications)...`, | ||
| { | ||
| name: buildTool.name, | ||
| brandColor: buildTool.brandColor.get(buildTool.name), | ||
| } | ||
| ); | ||
|
|
||
| const browser = await puppeteer.launch({ headless: "new" }); | ||
| // 在函数级别定义文件路径和内容，确保finally块能访问 | ||
| const rootFile = path.resolve('src', 'comps', 'triangle.jsx'); | ||
| const leafFile = path.resolve('src', 'comps', 'triangle_1_1_2_1_2_2_1.jsx'); | ||
| let originalRootContent = null; | ||
| let originalLeafContent = null; | ||
|
|
||
| const n = 3; | ||
| return new Promise(async (resolve, reject) => { | ||
| try { |
There was a problem hiding this comment.
Remove async from Promise executor in testWatchMemory
Using async in a Promise executor is an anti-pattern that can lead to uncaught promise rejections.
Refactor the function to handle async operations properly:
- return new Promise(async (resolve, reject) => {
+ return new Promise((resolve, reject) => {
+ (async () => {
try {And close the async IIFE before the catch block:
await buildTool.stopServer();
reject(error);
+ }
+ })().catch(reject);
- } finally {
+ }).finally(() => {Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In benchmark.mjs around lines 961 to 982, the Promise executor function in
testWatchMemory is marked async, which is an anti-pattern and can cause
unhandled promise rejections. To fix this, remove the async keyword from the
Promise executor and instead wrap the async logic inside an immediately invoked
async function expression (IIFE) within the Promise. This way, you can use await
inside the IIFE and properly handle errors with try-catch, calling resolve or
reject accordingly.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores
Style