-
Notifications
You must be signed in to change notification settings - Fork 107
feat(pack): support runtime entry bootstrap #2449
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
base: next
Are you sure you want to change the base?
Conversation
Summary of ChangesHello @fireairforce, 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! 此拉取请求引入了一个新的配置选项 Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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
|
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.
Code Review
这个 PR 增加了一个通用的 runtimeEntryBootstrap 配置,允许在运行时之前注入可执行代码,这对于适配 qiankun 等微前端框架非常有帮助。整体实现思路清晰,但在配置项的 enum 设计上存在一个严重问题,可能导致反序列化行为不符合预期。此外,处理不同配置类型的代码存在重复,可以进行重构以提高可维护性。
| #[derive( | ||
| Clone, Debug, PartialEq, Eq, Serialize, Deserialize, TraceRawVcs, NonLocalValue, OperationValue, | ||
| )] | ||
| #[serde(untagged)] | ||
| pub enum RuntimeBootstrapConfig { | ||
| /// Inline JavaScript code | ||
| Code(RcStr), | ||
| /// Path to a JavaScript/TypeScript file | ||
| Path(RcStr), | ||
| } |
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.
RuntimeBootstrapConfig 枚举的定义存在一个严重问题。当 #[serde(untagged)] 与两个都包装了 RcStr(类似于 String)的变体一起使用时,serde 无法区分它们。在反序列化时,任何字符串值(无论是内联代码还是文件路径)都将被错误地解析为第一个变体,即 Code(RcStr)。这会导致文件路径被当作内联代码处理,从而引发运行时错误。
为了解决这个问题,建议将其中一个变体(例如 Path)修改为结构体,以便 serde 能够根据 JSON 结构进行区分。例如,用户可以通过 { "path": "./bootstrap.js" } 来指定文件路径,通过 "console.log('hello')" 来指定内联代码。
同时,别忘了更新 crates/pack-core/src/client/context.rs 中对应的 match 分支来匹配新的结构 (e.g. RuntimeBootstrapConfig::Path { path } => ...)。
pub enum RuntimeBootstrapConfig {
/// Path to a JavaScript/TypeScript file, as an object: `{ "path": "./bootstrap.js" }`
Path { path: RcStr },
/// Inline JavaScript code, as a string
Code(RcStr),
}| if let Some(bootstrap_config) = bootstrap_config_opt.as_ref() { | ||
| match bootstrap_config { | ||
| crate::config::RuntimeBootstrapConfig::Code(code) => { | ||
| // Inline code: create a virtual source | ||
| let bootstrap_source = VirtualSource::new( | ||
| project_root.join("__runtime_bootstrap__.js")?, | ||
| AssetContent::file(File::from(code.clone()).into()), | ||
| ) | ||
| .to_resolved() | ||
| .await?; | ||
| runtime_entries.push( | ||
| RuntimeEntry::Source(ResolvedVc::upcast(bootstrap_source)).resolved_cell(), | ||
| ); | ||
| } | ||
| crate::config::RuntimeBootstrapConfig::Path(path) => { | ||
| // File path: use FileSource to load the file | ||
| let bootstrap_path = project_root.join(path.as_str())?; | ||
| let bootstrap_source = FileSource::new(bootstrap_path).to_resolved().await?; | ||
| runtime_entries.push( | ||
| RuntimeEntry::Source(ResolvedVc::upcast(bootstrap_source)).resolved_cell(), | ||
| ); | ||
| } | ||
| } |
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.
match 表达式的 Code 和 Path 分支中存在代码重复。创建 bootstrap_source 后的逻辑(即 runtime_entries.push(...))是完全相同的。建议将这部分通用逻辑提取到 match 表达式之外,以减少重复并提高代码的可读性和可维护性。
let bootstrap_source = match bootstrap_config {
crate::config::RuntimeBootstrapConfig::Code(code) => {
// Inline code: create a virtual source
VirtualSource::new(
project_root.join("__runtime_bootstrap__.js")?,
AssetContent::file(File::from(code.clone()).into()),
)
.to_resolved()
.await?
}
crate::config::RuntimeBootstrapConfig::Path(path) => {
// File path: use FileSource to load the file
let bootstrap_path = project_root.join(path.as_str())?;
FileSource::new(bootstrap_path).to_resolved().await?
}
};
runtime_entries.push(
RuntimeEntry::Source(ResolvedVc::upcast(bootstrap_source)).resolved_cell(),
);f77808c to
567e5ae
Compare
Summary
加个通用的 runtimeEntryBoostrap 配置来往运行时之前插入一些可执行的代码。
目前是为了去适配qiankun的加载。
Test Plan