-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
build: package react to the bundle file #214
Conversation
/next |
📝 Walkthrough📝 WalkthroughWalkthrough此次更改涉及修改Webpack配置文件 Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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
Documentation and Community
|
🎉 PR Next publish successful! 0.0.20241105083953-feateact.0 |
c23fd2c
to
0ea92b2
Compare
/next |
🎉 PR Next publish successful! 0.0.20241105124725-feateact.0 |
/next |
🎉 PR Next publish successful! 0.0.20241105131320-feateact.0 |
/next |
🎉 PR Next publish successful! 0.0.20241107032148-feateact.0 |
0d90bf6
to
df618b0
Compare
/next |
🎉 PR Next publish successful! 0.0.20241107131033-feateact.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
packages/toolkit/webpack/config.build.js (2)
Line range hint
1-48
: 建议更新 libBundle 的配置以匹配新的打包策略考虑到移除了 globalBundle,libBundle 的配置可能需要调整:
libraryTarget
仍然使用 'umd',注释提到是因为 external 原因,建议验证在新的打包策略下是否仍然需要。- library 名称仍然是 'AlexLib',与项目名称不符。
建议应用以下修改:
output: { - library: 'AlexLib', + library: 'CodeBlitzLib', libraryTarget: 'umd', },
Line range hint
1-99
: 架构建议:考虑提供迁移指南这是一个重要的架构变更,从全局模式迁移到 bundle 模式。建议:
- 在 PR 描述中添加迁移指南
- 说明此变更的动机和收益
- 提供具体的迁移步骤
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (3)
packages/core/package.json
(1 hunks)packages/core/src/api/require.ts
(3 hunks)packages/toolkit/webpack/config.build.js
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/package.json
🔇 Additional comments (5)
packages/toolkit/webpack/config.build.js (2)
99-99
: 确认移除 globalBundle 的影响范围
移除 globalBundle 是一个重大更改,需要确保:
- 所有依赖 global 模式的代码都已经更新
- 构建产物的体积变化在可接受范围内
建议执行以下验证:
#!/bin/bash
# 检查是否有代码仍在引用全局模式的构建产物
echo "检查对全局构建产物的引用:"
rg "appGlobalEntry|appGlobalMinEntry|editorAllGlobalEntry|editorAllGlobalMiniEntry"
49-51
: 请确认 WebAssembly 支持的必要性
在此次更改中添加了 WebAssembly 支持,但这与将 React 打包到 bundle 的主要目标似乎没有直接关联。
建议验证是否有相关模块需要 WebAssembly 支持:
packages/core/src/api/require.ts (3)
58-59
: 导入语句正确使用了 React 18 的客户端渲染模式
导入语句正确地从 'react-dom/client' 引入了 React 18 的客户端渲染 API,这与 React 18 推荐的用法一致。
114-115
: 类型声明与模块导出保持一致
类型声明正确地反映了导入的模块类型,保持了与其他模块声明相同的风格。
222-225
: 🛠️ Refactor suggestion
修复 ReactDOM 命名空间不一致问题
在 switch case 中返回的 ReactDOM
与类型声明中使用的 ReactDom
命名空间不一致。虽然目前可以工作,但为了保持代码的一致性,建议统一使用相同的命名空间。
建议修改如下:
case 'react':
return React;
case 'react-dom/client':
- return ReactDOM;
+ return ReactDom;
Likely invalid or redundant comment.
/next |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
packages/toolkit/webpack/config.build.js (1)
89-99
: 建议完善 React 版本兼容性说明注释提到"应对 React16 的集成",但缺少具体的版本兼容性范围说明。建议:
- 在注释中明确支持的 React 版本范围
- 考虑添加运行时版本检查以提前发现潜在的兼容性问题
- // 此处没有 external React,将 React 打包进去以应对 React16 的集成方 + // 此处没有 external React,将 React 打包进去以支持 React 16-18 版本的集成 + // 已在 packages/core/package.json 中声明支持 ^18.2.0 || ^16.0.0
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
📒 Files selected for processing (2)
packages/toolkit/webpack/config.build.js
(2 hunks)packages/toolkit/webpack/util/index.js
(1 hunks)
🔇 Additional comments (4)
packages/toolkit/webpack/util/index.js (1)
36-36
: 代码变更看起来没有问题!
新增的 appEntryWithReact
配置项符合现有的命名规范,并且与 PR 的目标(将 React 打包到 bundle 中)保持一致。
让我们验证一下这个入口文件是否存在:
✅ Verification successful
入口配置已正确验证!
通过检查代码库,我们可以确认:
appEntryWithReact
入口点已在webpack/config.build.js
中正确配置- 入口点指向
packages/core/src
目录,该目录确实存在并包含必要的源文件 - webpack 配置中特别注明了这个入口点不会 external React,符合"将 React 打包进去"的目标
- 整体配置结构完整且合理
这个变更是安全的,不需要额外修改。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述:验证 codeblitz-with-react 入口文件是否存在
# 预期:找到对应的入口文件
# 搜索可能的入口文件位置
fd -t f "codeblitz-with-react" -E "node_modules"
Length of output: 46
Script:
#!/bin/bash
# 搜索更广泛的模式,包括可能的入口文件
rg -l "codeblitz.*react" --type js
# 查找包含 appEntryWithReact 的文件
rg "appEntryWithReact" --type js
# 查找 webpack 配置文件中的入口点定义
fd -e js -e ts webpack.config
Length of output: 368
Script:
#!/bin/bash
# 检查入口点配置的实际路径
cat packages/toolkit/webpack/config.build.js
# 验证核心包中是否存在相关文件
fd -t f . packages/core/src -x ls -l
Length of output: 13470
packages/toolkit/webpack/config.build.js (3)
61-64
: WebAssembly 支持配置正确!
启用 WebAssembly 支持是一个很好的改进,这将有助于提高性能密集型代码的执行效率。
80-82
: 验证入口点配置的正确性
请确认 config.appEntryWithReact
在 packages/toolkit/webpack/util/index.js
中已正确配置。
155-155
: 验证所有导出的 bundle 是否都被使用
建议确认:
- 是否所有三个 bundle(libBundle、libBundleWithReact、globalBundle)都有其特定用途
- 是否需要为不同的使用场景提供清晰的文档说明
🎉 PR Next publish successful! 0.0.20241107133604-feateact.0 |
9e72381
to
21a2eea
Compare
/next |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
packages/toolkit/webpack/config.build.js (2)
61-63
: 需要补充 WebAssembly 支持的相关文档说明添加 WebAssembly 支持是一个重要的架构决策,建议:
- 在代码注释中说明添加 WebAssembly 支持的具体原因
- 在项目文档中补充相关使用说明
155-155
: 建议补充说明多个 bundle 的使用场景现在导出了三个不同的 bundle 配置:
- libBundle
- libBundleWithReact
- globalBundle
建议在代码注释中说明:
- 各个 bundle 的使用场景
- bundle 的加载顺序是否重要
- 如何选择合适的 bundle
packages/core/src/api/require.ts (2)
58-59
: 统一 React DOM 的导入方式当前的导入语句中存在命名不一致的情况:
- 类型声明使用
ReactDom
(命名空间导入)- switch 语句使用
ReactDOM
(默认导入)建议统一使用一种导入方式,以提高代码的一致性和可维护性。
-import ReactDOM, * as ReactDom from 'react-dom/client'; +import * as ReactDOM from 'react-dom/client';
114-115
: 类型声明需要与导入保持一致如果采纳了上述建议统一使用命名空间导入,这里的类型声明也需要相应更新:
export function requireModule(module: 'react'): typeof React; -export function requireModule(module: 'react-dom/client'): typeof ReactDom; +export function requireModule(module: 'react-dom/client'): typeof ReactDOM;
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (4)
packages/core/package.json
(1 hunks)packages/core/src/api/require.ts
(3 hunks)packages/toolkit/webpack/config.build.js
(2 hunks)packages/toolkit/webpack/util/index.js
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/core/package.json
- packages/toolkit/webpack/util/index.js
🔇 Additional comments (2)
packages/toolkit/webpack/config.build.js (1)
67-107
: 新增的 React 打包配置看起来没有问题,建议验证打包结果
配置逻辑正确:
- 移除了 React 相关的 external 配置
- 使用 appEntryWithReact 作为入口点
- 保持了与 libBundle 相同的其他配置项
建议执行以下脚本验证打包结果:
packages/core/src/api/require.ts (1)
222-225
: 验证 React 模块解析的正确性
模块解析的实现看起来正确,但建议验证以下几点:
- React 和 ReactDOM 的版本兼容性
- 模块解析的正确性
- 打包后的模块可用性
✅ Verification successful
React 模块解析实现正确且完整
根据代码分析,React 相关的模块解析实现是正确的:
-
require.ts
文件中正确导入了 React 和 ReactDOM:import * as React from 'react'; import ReactDOM, * as ReactDom from 'react-dom/client';
-
requireModule
函数的类型声明和实现完全匹配:export function requireModule(module: 'react'): typeof React; export function requireModule(module: 'react-dom/client'): typeof ReactDom;
-
switch 语句中的实现返回了正确的模块实例:
case 'react': return React; case 'react-dom/client': return ReactDOM;
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 验证 React 相关的导入和使用
echo "检查 React 相关的导入..."
rg --type ts "from ['\"](react|react-dom)" -l
echo "检查 requireModule 的使用..."
rg "requireModule\(['\"]react" -A 2
Length of output: 1765
Script:
#!/bin/bash
# 检查 React 和 ReactDOM 的导入声明
echo "检查 React 导入声明..."
ast-grep --pattern 'import $_ from "react"'
echo "检查 ReactDOM 导入声明..."
ast-grep --pattern 'import $_ from "react-dom/client"'
# 检查 require.ts 文件中的相关实现
echo "检查 require.ts 中的实现..."
cat packages/core/src/api/require.ts
# 检查是否有其他地方使用了 requireModule
echo "检查 requireModule 的使用模式..."
rg "requireModule" -A 2 -B 2
Length of output: 18004
🎉 PR Next publish successful! 0.0.20241108023217-feateact.0 |
Types
Background or solution
fix #213
example1 :
example2:
ChangeLog
Summary by CodeRabbit
Summary by CodeRabbit
新特性
@codeblitzjs/ide-core
包的peerDependencies
,支持 React 16 和 18 的版本兼容性。appEntryWithReact
属性。优化