usage of javascript/node to allow cross-platform building #5825
Conversation
Osiris-Team
requested review from
ezailWang,
loatheb,
sidmorizon,
originalix and
huhuanming
as code owners
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
PR总结
|
|
Thanks for your contribution @Osiris-Team But we're not planning to introduce Python scripts into the project, you can consider converting the Python scripts to Node scripts. |
|
Oh yeah that makes sense, didn't think about that! |
|
Thanks for your contribution @Osiris-Team. but development/scripts/copy_injected.js is still a python file. Maybe you can make modifications based on this draft PR #5830 |
|
my bad forgot to copy over, fixed |
Osiris-Team
changed the title
|
and apps/desktop/dist/build_keytar.sh need to be fixed. After all the files have been modified, I will help you package and test it. |
Walkthrough此次更改引入了多个新的脚本文件，分别用于清理开发工作区、复制注入文件和执行安装后步骤。 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant NodeJS
participant FileSystem
User->>NodeJS: 执行 postinstall()
NodeJS->>FileSystem: 执行 yarn setup:env
NodeJS->>FileSystem: 执行 patch-package
NodeJS->>FileSystem: 执行 yarn copy:inject
NodeJS->>FileSystem: 检查并删除 node_modules/realm-flipper-plugin-device/src
Note over NodeJS: 所有步骤完成
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: 12
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
Files selected for processing (7)
- development/scripts/clean_workspace.js (1 hunks)
- development/scripts/clean_workspace.sh (0 hunks)
- development/scripts/copy-injected.sh (0 hunks)
- development/scripts/copy_injected.js (1 hunks)
- development/scripts/postinstall.js (1 hunks)
- development/scripts/postinstall.sh (0 hunks)
- package.json (2 hunks)
Files not reviewed due to no reviewable changes (3)
- development/scripts/clean_workspace.sh
- development/scripts/copy-injected.sh
- development/scripts/postinstall.sh
Additional context used
Biome
development/scripts/clean_workspace.js
[error] 1-1: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.(lint/style/useNodejsImportProtocol)
[error] 2-2: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.(lint/style/useNodejsImportProtocol)
[error] 21-21: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.(lint/style/useNodejsImportProtocol)
development/scripts/copy_injected.js
[error] 1-1: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.(lint/style/useNodejsImportProtocol)
[error] 2-2: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.(lint/style/useNodejsImportProtocol)
[error] 3-3: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.(lint/style/useNodejsImportProtocol)
development/scripts/postinstall.js
[error] 1-1: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.(lint/style/useNodejsImportProtocol)
[error] 2-2: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.(lint/style/useNodejsImportProtocol)
Additional comments not posted (8)
development/scripts/postinstall.js (1)
1-26: 总体评价脚本功能正确，但有改进空间：
- 用 node: 协议导入
- 加强错误处理
- 考虑用异步方法提高性能
整体来说，代码简洁明了，易于理解。修复这些小问题后会更稳健。
Tools
Biome
[error] 1-1: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.(lint/style/useNodejsImportProtocol)
[error] 2-2: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.(lint/style/useNodejsImportProtocol)
development/scripts/copy_injected.js (2)
5-12: copyFile 函数写得不错函数简单明了，还处理了源文件不存在的情况。日志输出也很清晰。干得好！
54-55: 函数调用没问题简单明了，很好。日志消息清晰地表明了操作完成。
development/scripts/clean_workspace.js (1)
4-16: 实用函数设计合理
removeDir和removeFile函数设计得很好。它们检查目标是否存在，执行删除操作，并记录行为。这种方法安全可靠。package.json (4)
15-15: 清理脚本更通用了新的
clean脚本用Node.js替代了bash命令。这样做：
- 提高了跨平台兼容性
- 与项目向JavaScript迁移的方向一致
好改动！
16-16: 安装后脚本更好了
postinstall脚本现在用Node.js代替了bash。这个改动：
- 保持了项目风格一致性
- 提高了跨平台兼容性
干得好！
17-17: 注入脚本更通用了
copy:inject脚本改用Node.js了。这样做：
- 与项目整体风格一致
- 提高了跨平台兼容性
很棒的改进！
91-91: 新依赖提升了兼容性添加
cross-env-shell依赖：
- 支持了
setup:env脚本的跨平台运行- 与项目提高兼容性的目标一致
好主意！
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
Files selected for processing (1)
- package.json (2 hunks)
Additional comments not posted (2)
package.json (2)
15-15: 新清理脚本更通用用Node.js脚本清理工作区很棒。这样在所有平台上都能正常运行。
16-17: 脚本更新提高兼容性把
postinstall和copy:inject改用Node.js脚本很好。这样在Windows上也能顺利运行了。
|
Is there something left to do? |
At the current state the build fails when following the simple instructions in the README on Windows.
This fixes it.
Summary by CodeRabbit
"cross-env-shell": "^7.0.3"。postinstall和copy:inject脚本为Node.js脚本。clean脚本用于清理工作区。