Steve/v3 perf #4610
Steve/v3 perf #4610
Conversation
Walkthrough此次提交在多个包中做出了调整，涉及 Webpack 优化配置、中间件逻辑以及辅助工具包的扩展。主要改动包括在 brick-container 中重构缓存组配置；在 brick-container 和 brick-playground 中统一使用 Changes
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
packages/brick-playground/scripts/start.jsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "@next-core/eslint-config-next" to extend from. Please check that the name of the config is correct. The config "@next-core/eslint-config-next" was referenced from the config file in "/.eslintrc". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. packages/brick-container/build.config.jsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "@next-core/eslint-config-next" to extend from. Please check that the name of the config is correct. The config "@next-core/eslint-config-next" was referenced from the config file in "/.eslintrc". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. packages/serve-helpers/index.d.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "@next-core/eslint-config-next" to extend from. Please check that the name of the config is correct. The config "@next-core/eslint-config-next" was referenced from the config file in "/.eslintrc". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team.
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 1
🧹 Nitpick comments (4)
packages/serve-helpers/src/tryFiles.js (1)
42-76: 建议验证区间有效性并处理流错误
sendMultiRanges函数中，对所有区间均直接解析后使用createReadStream。若请求区间越界(如start大于end或超出文件大小)，没有显式报错处理。同时，在管道传输过程中若出现错误或连接中断，也缺少监听与对应处理逻辑。建议在本函数中或调用层添加以下改进：
- 验证区间有效性，若存在越界或无效区间则返回 416（Requested Range Not Satisfiable）或进行相应提示。
- 监听流错误事件并在错误时及时中止传输，释放资源，以增强健壮性。
packages/serve-helpers/src/serveBricks.js (1)
25-25: 建议增加错误处理当
tryServeFiles执行失败时，应该有适当的错误处理机制。建议添加错误处理：
- tryServeFiles(files, req, res, next); + try { + tryServeFiles(files, req, res, next); + } catch (error) { + console.error('Error serving brick files:', error); + next(error); + }packages/brick-playground/scripts/start.js (1)
6-6: 中间件配置简化且合理！使用
serveBricks替换了原有的 express.static 配置，使中间件设置更加简洁。建议添加日志记录以便于调试。建议添加调试日志：
middlewares.push({ path: "/preview/bricks/", middleware: serveBricks({ localBrickFolders }), }); +console.log('Registered brick serving middleware for folders:', localBrickFolders);Also applies to: 55-58
packages/brick-container/build.config.js (1)
124-127: 缓存组配置优化合理！新增的
all缓存组配置具有以下优点：
- 优先级设置为 -10，低于 mockdate 缓存组，确保特定模块优先被处理
- 配置简单明了，易于维护
建议考虑添加注释说明此缓存组的用途。
all: { + // 通用缓存组，用于处理所有其他模块的分块 priority: -10, name: "all", },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
packages/brick-container/build.config.js(1 hunks)packages/brick-container/serve/middlewares/getMiddlewares.js(7 hunks)packages/brick-playground/scripts/start.js(2 hunks)packages/brick-playground/serve/examplesJson.js(1 hunks)packages/brick-playground/serve/index.js(2 hunks)packages/serve-helpers/index.d.ts(2 hunks)packages/serve-helpers/src/index.js(1 hunks)packages/serve-helpers/src/serveBricks.js(1 hunks)packages/serve-helpers/src/tryFiles.js(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build (20.x)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (8)
packages/serve-helpers/src/index.js (1)
3-3: 确认命名冲突与循环引用
使用export * from "./serveBricks.js";时，请确保不会造成与已有导出实体命名冲突，也要避免循环引用等风险。建议查看本模块内其他导出，确认不会造成重复命名或循环依赖。packages/brick-playground/serve/examplesJson.js (1)
13-13: 验证新路径是否可用
此处将路径指向../next-bricks/bricks，请确认该目录在所有运行环境中均存在且包含所需的示例文件。若不存在，会导致getExamples返回空或报错，建议在读取前进行目录存在性检查或在调用端处理异常。packages/serve-helpers/index.d.ts (1)
1-1: 类型定义清晰且符合 Express 中间件模式！类型定义准确地反映了 Express 中间件的函数签名，并且参数类型完整。
Also applies to: 22-24
packages/serve-helpers/src/serveBricks.js (1)
22-24: 建议增加路径安全性检查当前实现在处理请求路径时可能存在路径遍历漏洞。
建议添加路径验证以防止目录遍历攻击：
return function (req, res, next) { + const normalizedPath = path.normalize(req.path).replace(/^(\.\.[\/\\])+/, ''); const files = localBrickFolders.map((dir) => - path.join(dir, req.path.split("/").join(path.sep)) + path.join(dir, normalizedPath.split("/").join(path.sep)) ); tryServeFiles(files, req, res, next); };packages/brick-playground/serve/index.js (1)
7-7: 代码重构改进了静态文件服务的实现！使用
serveBricks替换原有的静态文件服务实现，使代码更加简洁和统一。Also applies to: 50-50
packages/brick-container/serve/middlewares/getMiddlewares.js (3)
7-7: 导入语句正确添加！从
@next-core/serve-helpers导入serveBricks函数的做法正确，这与其他中间件导入保持一致。
37-37: 参数命名优化得当！使用
_req替代req作为未使用参数的命名是一个很好的实践，它明确表示该参数在函数中未被使用。Also applies to: 48-48, 57-57
111-114: 中间件配置合理！新增的中间件配置正确：
- 路径使用
baseHref构建，保持了一致性- 使用新的
serveBricks函数处理请求
next-core
|
||||||||||||||||||||||||||||
| Project |
next-core
|
| Branch Review |
steve/v3-perf
|
| Run status |
|
| Run duration | 00m 26s |
| Commit |
|
| Committer | Shenwei Wang |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
16
|
| View all changes introduced in this branch ↗︎ | |
There was a problem hiding this comment.
Copilot reviewed 6 out of 9 changed files in this pull request and generated no comments.
Files not reviewed (3)
- packages/brick-container/build.config.js: Evaluated as low risk
- packages/brick-playground/scripts/start.js: Evaluated as low risk
- packages/brick-container/serve/middlewares/getMiddlewares.js: Evaluated as low risk
Comments suppressed due to low confidence (2)
packages/serve-helpers/src/tryFiles.js:22
- The new functionality for handling multiple ranges in the tryServeFiles function should be covered by tests.
export function tryServeFiles(files, req, res, next) {
packages/brick-playground/serve/examplesJson.js:13
- Verify that the new path '../next-bricks/bricks' is correct and accessible by the getExamples and getBrickManifests functions.
path.join(rootDir, "../next-bricks/bricks"),
| }, | ||
| defaultVendors: { | ||
| test: /[\\/]node_modules[\\/]/, | ||
| all: { |
There was a problem hiding this comment.
目标：通过合并 JS 输出，减少请求数。 副作用：请求体积增大；失去拆分 JS 分块缓存带来的的好处。
weareoutman
force-pushed
the
steve/v3-perf
branch
from
12c3d95 to
226a728
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/serve-helpers/src/tryFiles.js (1)
59-59: 建议移除注释掉的过期时间代码中包含了一个被注释掉的
Expires头部设置。建议直接删除这行注释，因为已经使用了Cache-Control来控制缓存。- // "Expires": "Thu, 12 Feb 2026 09:39:38 GMT",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
packages/brick-container/build.config.js(1 hunks)packages/brick-container/serve/middlewares/getMiddlewares.js(7 hunks)packages/brick-playground/scripts/start.js(2 hunks)packages/brick-playground/serve/index.js(2 hunks)packages/serve-helpers/index.d.ts(2 hunks)packages/serve-helpers/src/index.js(1 hunks)packages/serve-helpers/src/serveBricks.js(1 hunks)packages/serve-helpers/src/tryFiles.js(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/brick-container/build.config.js
- packages/brick-playground/serve/index.js
- packages/serve-helpers/src/index.js
- packages/serve-helpers/index.d.ts
- packages/serve-helpers/src/serveBricks.js
- packages/brick-container/serve/middlewares/getMiddlewares.js
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build (20.x)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
packages/brick-playground/scripts/start.js (1)
55-58: 代码重构改进了可维护性！通过使用
serveBricks替换express.static中间件，实现了以下改进：
- 集中了静态文件服务的逻辑
- 减少了对 express 的直接依赖
- 保持了原有功能的完整性
packages/serve-helpers/src/tryFiles.js (1)
25-35: 多区间请求的处理逻辑正确实现！代码正确地检测和解析多区间请求，并将单区间请求委托给 Express.js 原生处理。这种实现方式是合理的。
依赖检查
组件之间的依赖声明，是微服务组件架构下的重要信息，请确保其正确性。
请勾选以下两组选项其中之一：
或者：
提交信息检查
Git 提交信息将决定包的版本发布及自动生成的 CHANGELOG，请检查工作内容与提交信息是否相符，并在以下每组选项中都依次确认。
破坏性变更：
feat作为提交类型。BREAKING CHANGE: 你的变更说明。新特性：
feat作为提交类型。问题修复：
fix作为提交类型。杂项工作：
即所有对下游使用者无任何影响、且没有必要显示在 CHANGELOG 中的改动，例如修改注释、测试用例、开发文档等：
chore,docs,test等作为提交类型。Summary by CodeRabbit
新功能
重构