Update package:browser field on pre-publish to support webpack 4
#954
Conversation
|
@sifthedog is attempting to deploy a commit to the React Component Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Walkthrough向构建流程中添加了自动化功能，新增 npm 脚本与 Node.js 脚本以动态生成并更新 package.json 中的 browser 字段映射，支持库文件在浏览器环境中的正确解析。 Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
Summary of ChangesHello @sifthedog, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request implements a critical enhancement to the package's build process by introducing an automated mechanism to populate the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a script to dynamically generate the browser field in package.json before publishing. This is a good approach to ensure compatibility with older bundlers like webpack 4 that may not fully support the exports field. The script correctly maps library entry points to their ES module equivalents. The changes in package.json to integrate this script into the prepublishOnly lifecycle are also correct. I have one suggestion to make the script more robust.
scripts/update-browser-field.js
Outdated
| fs.readdirSync(dirPath) | ||
| .filter((file) => file.endsWith('.js')) | ||
| .sort() | ||
| .forEach((file) => { | ||
| const name = path.basename(file, '.js'); | ||
| const target = `${targetPrefix}/${file}`; | ||
|
|
||
| addEntry(`${browserPrefix}/${name}`, target); | ||
| addEntry(`${browserPrefix}/${name}.js`, target); | ||
| }); |
There was a problem hiding this comment.
The current implementation of addDirMappings doesn't check if a directory entry is a file. If a directory name happens to end with .js, it would be processed, leading to incorrect entries in the browser field.
To make this more robust, you can use the withFileTypes: true option in fs.readdirSync and filter for files only. This is more efficient than using fs.statSync in a loop and ensures only files are processed.
fs.readdirSync(dirPath, { withFileTypes: true })
.filter((dirent) => dirent.isFile() && dirent.name.endsWith('.js'))
.map((dirent) => dirent.name)
.sort()
.forEach((file) => {
const name = path.basename(file, '.js');
const target = `${targetPrefix}/${file}`;
addEntry(`${browserPrefix}/${name}`, target);
addEntry(`${browserPrefix}/${name}.js`, target);
});![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
scripts/update-browser-field.js (3)
8-8: 添加错误处理以提供更清晰的错误消息。如果
package.json不存在或格式错误，脚本会崩溃并显示不清晰的错误消息。由于此脚本在prepublishOnly钩子中运行，添加 try-catch 块以提供更有用的错误消息会改善开发者体验。应用此差异以添加错误处理：
-const pkg = JSON.parse(fs.readFileSync(pkgPath, 'utf8')); +let pkg; +try { + pkg = JSON.parse(fs.readFileSync(pkgPath, 'utf8')); +} catch (error) { + console.error('Failed to read or parse package.json:', error.message); + process.exit(1); +}
43-43: 为文件写入操作添加错误处理。
fs.writeFileSync可能因权限问题或磁盘空间不足而失败。由于这在发布前运行，添加错误处理以提供清晰的错误消息会很有帮助。应用此差异以添加错误处理：
-fs.writeFileSync(pkgPath, `${JSON.stringify(pkg, null, 2)}\n`); +try { + fs.writeFileSync(pkgPath, `${JSON.stringify(pkg, null, 2)}\n`); +} catch (error) { + console.error('Failed to write package.json:', error.message); + process.exit(1); +}
41-41: 浏览器字段的完全覆盖是预期的设计行为。该脚本作为构建工具在
prepublishOnly钩子中运行，用于在每次发布前自动重新生成browser字段。因此，browser字段不应手动编辑，而是由脚本根据es/locale和es/generate目录中的文件自动生成。这个设计是有意的，不存在问题。可选建议：在脚本顶部添加注释说明其目的是自动生成 browser 字段映射，以便后续维护者理解这个字段是自动生成的，不应手动修改。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
package.json(1 hunks)scripts/update-browser-field.js(1 hunks)
🔇 Additional comments (5)
package.json (2)
100-100: 确认新脚本的执行顺序正确。新增的
browser-field脚本在发布流程中的位置合理，但需要确保scripts/update-browser-field.js具有适当的错误处理机制，以避免在发布前阻塞。
102-102: 验证浏览器字段更新不会破坏现有发布流程。
prepublishOnly脚本的执行顺序正确：
- ✓
compile步骤先执行，生成构建产物- ✓
browser-field脚本随后运行，更新 package.json 的浏览器入口- ✓
rc-np最后执行，发布到 npm脚本间使用
&&链接，任何步骤失败都会中止后续执行，确保发布流程的原子性。无其他代码依赖旧的prepublishOnly行为，CI/CD 配置中也无相关影响。scripts/update-browser-field.js (3)
20-22: 良好的防御性编程实践。目录存在性检查可防止在编译输出尚未生成时脚本崩溃，这是一个很好的实践。
24-33: 此评论不适用于当前代码库。
es/locale和es/generate目录在此仓库中不存在，因此关于单层目录扫描的假设不成立。脚本已通过fs.existsSync()进行了防御性检查，可以安全地处理这些目录缺失的情况。如果将来这些目录被添加到项目中，且需要支持嵌套子目录，再考虑实现递归扫描逻辑。Likely an incorrect or invalid review comment.
36-37: 当前的浏览器字段映射策略对于 webpack 4 是正确完整的。webpack 4 不支持 package.json 的
exports字段，仅依赖browser、module、main字段进行解析。lines 36-37 创建的映射./locale/*→./es/locale/*和./generate/*→./es/generate/*已足以处理主要使用场景。如果用户明确引入./lib/*路径，说明其意图是使用 lib 版本而非浏览器版本，因此不应在浏览器字段中重写这些路径。
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
scripts/update-browser-field.js (2)
37-44: 请确认覆盖写pkg.browser（而非合并）是预期行为
当前会完全替换现有browser字段；如果项目/下游依赖曾手工维护额外映射，这里会被抹掉。若希望保留额外条目，建议“基于现有对象合并 + 覆盖冲突键”。
44-46: 写回package.json建议用“临时文件 + rename”提高原子性
writeFileSync直接覆盖在异常中断时可能留下半文件；发布链路里这类失败会比较难排查。- fs.writeFileSync(pkgPath, `${JSON.stringify(pkg, null, 2)}\n`); + const tmpPath = `${pkgPath}.tmp`; + fs.writeFileSync(tmpPath, `${JSON.stringify(pkg, null, 2)}\n`); + fs.renameSync(tmpPath, pkgPath);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/update-browser-field.js(1 hunks)
🔇 Additional comments (3)
scripts/update-browser-field.js (3)
1-9: 确认 Node 版本兼容性（withFileTypes/Object.fromEntries）
脚本依赖较新的 Node 特性；建议对照仓库engines.node/ CI / 发布机 Node 版本，避免prepublishOnly在旧环境直接炸。
10-18: 主入口映射固定为./lib/index.js -> ./es/index.js很清晰
逻辑直接、可预测，符合“主入口优先走 ES build”的意图。
19-35:addDirMappings实现稳健：只处理文件且按名称排序
withFileTypes: true+dirent.isFile()能避免把目录误当成.js文件；排序后输出稳定，利于可重复发布。
1 participant
After a discussion on ant-design/ant-design#56145, I believe the code change should be in this repository, so we can properly export consume it in
antdreproducible example
Summary by CodeRabbit
发布说明
✏️ Tip: You can customize this high-level summary in your review settings.
Summary by CodeRabbit
发布说明
✏️ Tip: You can customize this high-level summary in your review settings.