Chore/clean git history #19
Conversation
WalkthroughAlright, chat, here’s the rundown: We’ve got a totally revamped CI/CD pipeline with new workflows for testing, publishing, and compatibility checks. The package.json and TypeScript configs are now all about that sweet dist/ output, and there’s a new .npmignore to keep the npm package lean. src/types.ts lost some whitespace—classic cleanup. Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant GitHub Actions
participant Bun
participant NPM
Developer->>GitHub Actions: Push or PR to master
GitHub Actions->>Bun: Setup Bun environment
GitHub Actions->>GitHub Actions: Run tests, lint, format, build
alt On push to master
GitHub Actions->>GitHub Actions: Bump version, tag, push changes
GitHub Actions->>NPM: Publish package
GitHub Actions->>GitHub Actions: Create GitHub Release
end
GitHub Actions->>GitHub Actions: Run compatibility check
Poem
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
.github/workflows/publish.yml (2)
60-80:grep-based conventional-commit parsing = brittle
Single-line commit messages work, multiline bodies or merge commits… not so much. Considerconventional-changelog/standard-versionor at least a tiny Node script to parse the commit header robustly.
90-97: Potential infinite-loop dodged, but still spams history
npm version --no-git-tag-version+ manual commit is fine, but every publish adds another “bump version” commit. Over time the git log becomes a graveyard. Squash-merge the bumps or move to tags-only versioning (npm version --git-tag-version).package.json (1)
34-38:bun testis great, but CI also callsbun test– duplication?
Scripts are ⌨️ muscle memory. If a maintainer runsnpm testlocally it’s fine, but remember anyone without Bun installed is toast. Maybe add a Node-based fallback or document Bun as a hard dep in the README.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
.github/workflows/ci.yml(1 hunks).github/workflows/publish.yml(1 hunks).npmignore(1 hunks)package.json(1 hunks)src/types.ts(0 hunks)tsconfig.build.json(1 hunks)
💤 Files with no reviewable changes (1)
- src/types.ts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: MarcelOlsen
PR: MarcelOlsen/mini-react#8
File: README.md:701-708
Timestamp: 2025-06-17T12:40:39.737Z
Learning: MarcelOlsen prefers to keep test documentation simple with just one example of running specific test files, rather than listing detailed commands for each test suite in the README.
🔇 Additional comments (4)
tsconfig.build.json (1)
11-12:skipLibCheck: true– great for speed, terrible for catching dumb mistakes
Totally fine for CI speed-runs, but remember you’re shipping a library. If a downstream TS user hits a lib-type error we never saw, we look like clowns. Consider flipping this tofalseon release builds or adding a dedicated “strict” job..npmignore (1)
1-4: You’re nukingsrc/&index.tsfrom the published package – intentional?
Givenpackage.json#filesalready whitelistsdist/**/*, this ignore entry is fine, but double-check you don’t rely onsourcemaps pointing back to the original TS inside the tarball. If you do want source maps debuggable from NPM installs, you’ll need the sources too..github/workflows/ci.yml (1)
5-8: Pushes tomasterskip CI entirely – brave move
branches-ignore: [master]means a direct push to prod won’t even lint. If that’s by design (because the publish workflow re-runs tests) cool; otherwise rethink the trigger.package.json (1)
3-8: Shipping with0.0.0– NPM thinks this is pre-alpha
If you accidentlynpm publishlocally you’re stuck with0.0.0forever (NPM disallows re-use). Might be safer to start at0.1.0to reserve some semantic-version breathing room.
| "outDir": "./dist", | ||
| "rootDir": "./", | ||
| "declaration": true, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
rootDir set to “./” is a little too YOLO
Right now the compiler thinks the entire repo is fair game. If someone drops an extra file at the project root it’ll end up in dist/ faster than you can say chat, that’s a bug. Point this at "src" (and maybe the root index.ts) so we only emit what we actually ship.
- "rootDir": "./",
+ "rootDir": "./src",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "outDir": "./dist", | |
| "rootDir": "./", | |
| "declaration": true, | |
| "outDir": "./dist", | |
| - "rootDir": "./", | |
| + "rootDir": "./src", | |
| "declaration": true, |
🤖 Prompt for AI Agents
In tsconfig.build.json around lines 4 to 6, the rootDir is set to "./", which
causes the compiler to include the entire repository in the output. Change the
rootDir value to "src" to restrict compilation to the source directory and avoid
accidentally including unrelated files in the dist output.
| run: | | ||
| curl -fsSL https://bun.sh/install | bash -s "bun-v1.2.16" | ||
| - name: Add bun to PATH | ||
| run: echo "$HOME/.bun/bin" >> $GITHUB_PATH | ||
| BUNDLE_SIZE=$(du -sh dist/ | cut -f1) | ||
| echo "📦 Bundle size: $BUNDLE_SIZE" | ||
| echo "bundle-size=$BUNDLE_SIZE" >> $GITHUB_OUTPUT |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Step writes an output nobody can read – missing id tag
You’re echo-ing bundle-size to $GITHUB_OUTPUT, but without id: bundle (or similar) the value can’t be referenced later. Either add an id or ditch the output entirely.
- - name: 📏 Check bundle size
+ - id: bundle-size
+ name: 📏 Check bundle size📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| run: | | |
| curl -fsSL https://bun.sh/install | bash -s "bun-v1.2.16" | |
| - name: Add bun to PATH | |
| run: echo "$HOME/.bun/bin" >> $GITHUB_PATH | |
| BUNDLE_SIZE=$(du -sh dist/ | cut -f1) | |
| echo "📦 Bundle size: $BUNDLE_SIZE" | |
| echo "bundle-size=$BUNDLE_SIZE" >> $GITHUB_OUTPUT | |
| - id: bundle-size | |
| name: 📏 Check bundle size | |
| run: | | |
| BUNDLE_SIZE=$(du -sh dist/ | cut -f1) | |
| echo "📦 Bundle size: $BUNDLE_SIZE" | |
| echo "bundle-size=$BUNDLE_SIZE" >> $GITHUB_OUTPUT |
🤖 Prompt for AI Agents
In .github/workflows/ci.yml around lines 35 to 38, the step writes an output to
$GITHUB_OUTPUT without specifying an id for the step, so the output cannot be
referenced later. Fix this by adding an id field (e.g., id: bundle) to the step
that runs this script, enabling other steps to access the bundle-size output.
Alternatively, remove the output if it is not needed.
Summary by CodeRabbit
New Features
Chores
.npmignorefile to exclude unnecessary files from published packages.Style