-
Notifications
You must be signed in to change notification settings - Fork 11
fix: suppress misleading oclif TypeScript warning #33
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
Add process.env.OCLIF_TS_NODE = '0' to bin/run.js to prevent the "Could not find typescript" warning when users run the CLI via npx. This warning is misleading as TypeScript is only needed for development, not for end users running the published package. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
WalkthroughAn environment variable assignment was added at the beginning of the script to suppress a TypeScript warning from the oclif framework during production. The variable Changes
Suggested labels
Poem
✨ Finishing Touches
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
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 (1)
bin/run.js (1)
3-4: Respect pre-existingOCLIF_TS_NODEsetting to avoid overriding user intentUnconditionally forcing the environment variable may surprise devs who explicitly set it (e.g. to
"1"for local TypeScript runs). Guard the assignment so you only supply the default when the variable is absent.-// Suppress oclif TypeScript warning for production usage -process.env.OCLIF_TS_NODE = '0' +// Suppress oclif TypeScript warning for production usage +if (process.env.OCLIF_TS_NODE === undefined) { + process.env.OCLIF_TS_NODE = '0' +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
bin/run.js(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-07T16:54:50.723Z
Learning: Applies to .claude/{settings.json,hooks/index.ts,hooks/lib.ts,hooks/session.ts} : The CLI generates a '.claude/' directory containing 'settings.json', 'hooks/index.ts', 'hooks/lib.ts', and 'hooks/session.ts' when initialized.
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-07T16:54:50.723Z
Learning: Applies to **/*.integration.{ts,js} : Integration tests should test the full CLI behavior.
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-07T16:54:50.723Z
Learning: Applies to tsconfig.json : TypeScript strict mode must be enabled.
bin/run.js (2)
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-07T16:54:50.723Z
Learning: Applies to tsconfig.json : TypeScript strict mode must be enabled.
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-07T16:54:50.723Z
Learning: Applies to .claude/{settings.json,hooks/index.ts,hooks/lib.ts,hooks/session.ts} : The CLI generates a '.claude/' directory containing 'settings.json', 'hooks/index.ts', 'hooks/lib.ts', and 'hooks/session.ts' when initialized.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: test (windows-latest, 20)
- GitHub Check: test (windows-latest, 18)
- GitHub Check: test (macos-latest, 20)
- GitHub Check: test (ubuntu-latest, 20)
- GitHub Check: test (ubuntu-latest, 18)
🔇 Additional comments (1)
bin/run.js (1)
1-15: Confirm ESM suitability for Node versions & package metadataThis file uses top-level
awaitand ESMimport, which only run when Node treats the file as an ES module (requires"type": "module"inpackage.jsonor a.mjsextension) and Node ≥ 14.8. Ensure:
package.jsonhas"type": "module"or the file is renamed torun.mjs.engines.nodeinpackage.jsonenforces a compatible Node version to prevent runtime failures for users installing globally.Please double-check the packaging before publishing.
|
🎉 This PR is included in version 1.5.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Summary
OCLIF_TS_NODE=0environment variable to prevent oclif from looking for TypeScript in productionProblem
When users run
npx claude-hooks, they see:This warning is confusing because:
Solution
Set
process.env.OCLIF_TS_NODE = '0'in the bin script to tell oclif not to look for TypeScript.Test Plan
npx claude-hooks help- no TypeScript warning should appear🤖 Generated with Claude Code
Summary by CodeRabbit