fix: compatible with old version egg-core package name #38
Conversation
|
@rrbe is attempting to deploy a commit to the no-veronica Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe changes modify the Changes
Poem
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: 0
🧹 Nitpick comments (3)
src/plugin.ts (3)
136-137: Consider adding an English translation for this comment.While this comment is informative, it mixes Chinese text with potentially global code usage contexts. Adding an English note could help non-Chinese speakers understand how and why the package rename is handled here.
137-145: Good backward compatibility approach – consider reducing duplicate logic.Defining multiple package names in an array improves clarity and extensibility. However, the subsequent fallback logic in lines 148-154 repeats similar steps. Factoring this into a single function or more unified flow will enhance maintainability and readability.
- for (const name of names) { - try { - const { EggCore, EggLoader } = await importModule(name, { paths }); - if (EggLoader) { - return { EggCore, EggLoader }; - } - } catch (err: any) { - debug('[findEggCore] import "%s" from paths:%o error: %o', name, paths, err); - } - // ... - } +async function tryImportEggCore(name: string, paths?: string[]): Promise<{ EggCore?: object; EggLoader?: EggLoaderImplClass }> { + try { + const result = await importModule(name, paths ? { paths } : undefined); + if (result?.EggLoader) { + return result; + } + } catch (err) { + debug('[findEggCore] import "%s" from paths:%o error: %o', name, paths, err); + } + return {}; +} + +for (const name of names) { + let result = await tryImportEggCore(name, paths); + if (result?.EggLoader) return result; + result = await tryImportEggCore(name); + if (result?.EggLoader) return result; + // ... +}
148-154: Repeated fallback logic could be wrapped into a helper.You are attempting to import the same modules again but without custom paths. This repetition is functionally correct but could be cleaner if consolidated into a reusable helper or a single logic block.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/plugin.ts(1 hunks)
🔇 Additional comments (2)
src/plugin.ts (2)
166-166: Great error handling with a clear, consolidated message.This ensures the code fails explicitly when no appropriate package is found, preventing ambiguous misconfigurations.
157-163: Directly referencingnode_modulesmight fail in Yarn PnP or pnpm setups.This approach explicitly relies on
node_modulesfolder existence. It may not work in environments such as Yarn Plug’n’Play or pnpm which change or remove that traditional layout.Below is a script to detect any Yarn Berry PnP config in the repository:
commit: |
|
晚点我将 @egg/utils v4 改回去，它应该是要支持 egg v3 的，不应该 breaking。 |
先按你的方式保持兼容 |
[skip ci] ## [4.2.5](v4.2.4...v4.2.5) (2025-01-08) ### Bug Fixes * compatible with old version egg-core package name ([#38](#38)) ([c8b5dcc](c8b5dcc))
egg-bin v6 版本启动项目时，无法读到 cluster.listen.port 的配置
原因是 egg-bin 6.13.0 版本引入了 egg-utils v4，而 v4 在查找 egg-core 包时，包名从写死的
egg-core改成了@eggjs/core，如果 eggjs 版本没跟上，子依赖 egg-core 没升级，egg-utils 就会查找失败一种可能的解决方法是同时查找两个包名，如 PR 内容所示
Summary by CodeRabbit