-
-
Notifications
You must be signed in to change notification settings - Fork 832
refactor(scripts): use node --run instead of npm run to reduce overhead #677
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
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe npm scripts in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Node
participant Script
User->>Node: node --run <script>
Node->>Script: Execute <script>
Script-->>Node: Script completes
Node-->>User: Return result
✨ Finishing Touches🧪 Generate Unit Tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
|
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.
Summary of Changes
Hello @yoshi-taka, 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!
This pull request focuses on optimizing the execution of package.json
scripts by switching from npm run
to the more efficient node --run
command. The change aims to reduce startup overhead and provide a slight performance improvement for script execution within the project.
Highlights
- Performance Optimization: This pull request refactors the project's
package.json
scripts to utilizenode --run
instead ofnpm run
. This change is intended to reduce script execution overhead and improve performance, leveraging a feature available in Node.js 22 and later. - Script Execution Standardization: All relevant script commands within
package.json
that previously called othernpm run
scripts have been updated to use thenode --run
command, standardizing the internal script execution mechanism.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
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 .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and 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 to provide feedback.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
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
This PR refactors scripts in package.json
to use node --run
instead of npm run
, which is a good move for performance as it reduces overhead. The changes themselves appear correct and follow the intended functionality of node --run
.
One important consideration, detailed in the specific comment, is to ensure the engines.node
field in package.json
accurately reflects the minimum Node.js version required by these script changes, as node --run
has specific version prerequisites.
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 (1)
package.json (1)
30-32
: Ensure cross-platform quoting in scriptsThe single quotes around
src,tests
andwebsite
may not be interpreted correctly on Windows. For better cross-platform support, consider using double quotes or escaping:"repomix-src": "node --run repomix -- --include \"src,tests\"", "repomix-website": "node --run repomix -- --include \"website\"",
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
package.json
(1 hunks)
🔇 Additional comments (3)
package.json (3)
35-35
: Appropriate conversion fornpm-publish
The
npm-publish
script correctly replaces allnpm run
calls withnode --run
, preserving the finalnpm publish
. This reduces overhead without altering functionality.
37-39
: Consistent chaining in release scriptsThe
npm-release-*
scripts now chainnpm version
withnode --run npm-publish
, mirroring prior behavior while improving performance. All flows look correct.
23-24
: ```shell
grep -R '"engines"' -n package.json
awk '/"engines"/,/}/' package.json[web_search] ```web Node.js --run flag introduced in which version?
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #677 +/- ##
=======================================
Coverage 90.35% 90.35%
=======================================
Files 99 99
Lines 5536 5536
Branches 1181 1181
=======================================
Hits 5002 5002
Misses 534 534 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…s versions - Keep npm run for fixed version jobs (better maintainability) - Use npx only for matrix.node-version jobs to support Node.js 18.0.0 - This allows PR yamadashy#677 (node --run refactor) to work with all CI environments
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.
Pull Request Overview
This PR refactors existing npm scripts to use the new Node 22+ feature "node --run" for reduced startup overhead.
- Replaces "npm run" commands with "node --run" in package.json scripts
- Updates CI workflows to directly invoke binaries for testing and building
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
package.json | Updated scripts to use "node --run" for improved performance |
.github/workflows/ci.yml | Modified CI steps to invoke test and build commands directly |
Comments suppressed due to low confidence (1)
package.json:23
- Consider adding documentation in the package or README to indicate that 'node --run' is used instead of 'npm run' for improved performance, and that Node 22+ is required.
"build": "node --run clean && tsc -p tsconfig.build.json --sourceMap --declaration",
@yoshi-taka たしかに言われてみればその通りですね CIの古いNodeでのテストでエラーになっていたので、CI側を調整しました。CIの保守性は少々下がりますがこの程度なら問題ないかなと マージします。全部 node --run にするかは追って検討します |
node 23前提なのでnode 22からのnode --runを利用可能
これはnpm runよりも起動オーバーヘッドがないため若干高速化できる
time コマンドなどで確認するとわかりやすい
以下全部置き換え可能ですが一旦package.jsonのみ
https://github.com/search?q=repo%3Ayamadashy%2Frepomix+%22npm+run%22&type=code
Checklist
npm run test
npm run lint