fix(typecheck): declare bundled markdown and macro fields - #1562
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (4)**/*.{ts,tsx,js,jsx,py}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*⚙️ CodeRabbit configuration file
Files:
{bin/**,scripts/**,package.json,src/setup.ts,src/main.tsx,src/entrypoints/**}⚙️ CodeRabbit configuration file
Files:
**⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (1)
📝 WalkthroughWalkthroughPR extends TypeScript global types in ChangesGlobal Type Definitions and Build Wiring
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 7✅ Passed checks (7 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
jatmn
left a comment
There was a problem hiding this comment.
I found an issue that needs to be addressed before this is ready.
Findings
- [P2] Do not declare VERSION_CHANGELOG until the build provides it
src/global.d.ts:17
This PR now tells TypeScript thatMACRO.VERSION_CHANGELOGis a valid build-time field, but the OpenClaude build maps inscripts/build.tsstill only define the otherMACRO.*fields. After runningbun run buildon this branch,dist/cli.mjsstill contains a literalMACRO.VERSION_CHANGELOG, while the other declared macros are inlined. The Ant release-note paths read this field whenUSER_TYPE === 'ant', so this declaration removes the typecheck signal without actually making the macro exist at runtime. Please either add the corresponding builddefineentries or leave this field undeclared until the runtime build surface supplies it.
jatmn
left a comment
There was a problem hiding this comment.
Thanks for the update. I rechecked the previously discussed paths and do not see any remaining actionable issues from my side.
@kevincodex1 LGTM
Summary
MACRO.FEEDBACK_CHANNELandMACRO.VERSION_CHANGELOGbuild-time fields insrc/global.d.ts.*.mdmodule declaration for Bun text-loader imports used by bundled skill content.Why
bun run typecheckcurrently reports declaration drift in two places:MACRO.FEEDBACK_CHANNELandMACRO.VERSION_CHANGELOG, but the ambientMACROtype did not include those fields.src/skills/bundled/claudeApiContent.tsimports Markdown assets as strings, but TypeScript had no ambient declaration for Markdown modules.This PR keeps the fix to the ambient type surface only and does not change runtime behavior.
Refs #1486
Validation
bun run typecheckMACROfields.Summary by CodeRabbit
New Features
Chores