-
Notifications
You must be signed in to change notification settings - Fork 440
fix(ai-native): use path.join for wasm file paths in WasmModuleManager #4304
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
Conversation
|
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
warning [email protected]: This version is no longer supported. Please see https://eslint.org/version-support for other options. 变更概览演练此次变更主要修改了 WebAssembly (WASM) 文件路径的构建方式。在 变更
可能相关的 PR
建议标签
建议审阅者
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 (
|
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
🧹 Nitpick comments (2)
packages/ai-native/src/browser/languages/tree-sitter/wasm-manager.ts (2)
32-32: 使用path.join拼接 wasm 路径能有效避免重复斜杠等路径问题。如果
baseUrl具有可能不一致的末尾斜杠或空字符串,这种方式更健壮且可读性更高。建议后续在运行环境或测试环境中验证最终生成的完整 URL 是否正确。
48-48: 同样使用path.join拼接语言相关的 wasm 文件路径,非常直观且一致。建议在使用
fetch时考虑错误处理,例如网络异常或文件不存在等情况,保证在发生异常时能够及时向上层抛出或记录错误日志。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/ai-native/src/browser/languages/tree-sitter/wasm-manager.ts(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: ubuntu-latest, Node.js 20.x
- GitHub Check: unittest (ubuntu-latest, 18.x, jsdom)
- GitHub Check: build (ubuntu-latest, 20.x)
- GitHub Check: unittest (ubuntu-latest, 18.x, node)
- GitHub Check: build (macos-latest, 20.x)
- GitHub Check: unittest (macos-latest, 18.x, jsdom)
- GitHub Check: build-windows
- GitHub Check: unittest (macos-latest, 18.x, node)
🔇 Additional comments (1)
packages/ai-native/src/browser/languages/tree-sitter/wasm-manager.ts (1)
5-5: 导入path很实用!此更改能够帮助以一致的方式处理路径拼接,避免了手动字符串连接可能带来的潜在路径问题。
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4304 +/- ##
==========================================
- Coverage 54.20% 54.20% -0.01%
==========================================
Files 1634 1634
Lines 99927 99927
Branches 21700 21692 -8
==========================================
- Hits 54169 54167 -2
- Misses 38015 38016 +1
- Partials 7743 7744 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
erha19
left a comment
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.
LGTM
| async initParser() { | ||
| const baseUrl = await this.resolvedResourceUriDeferred.promise; | ||
| const wasmPath = `${baseUrl}/tree-sitter.wasm`; | ||
| const wasmPath = path.join(baseUrl, 'tree-sitter.wasm'); |
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.
Types
Background or solution
解决请求
tree-sitter-wasm资源 404 问题.opensumi
3.7.0版本, AppConfig 配置如下时:会出现 url path 错误, 导致请求
https://registry.npmmirror.com/@opensumi/tree-sitter-wasm/0.0.2/files//tree-sitter.wasm404.相关源码:
https://github.com/opensumi/core/blob/v3.7/packages/core-browser/src/react-providers/config-provider.tsx#L390
Changelog
Summary by CodeRabbit
path.join()方法替代字符串拼接,提高路径构建的可靠性和跨平台兼容性