fix: 🐛 fix .gitignores not being included to the package #4
Conversation
📝 WalkthroughWalkthroughAdds a zip/unzip packaging workflow: a prepare script zips Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer
participant Publish as npm publish
participant Prepare as prepare.ts
participant FS as File System
participant Registry as npm Registry
participant Install as npm install
participant Postinstall as postinstall.ts
note over Prepare,FS `#DDEBF7`: Archive creation phase
Dev->>Publish: npm publish
Publish->>Prepare: run prepare script
Prepare->>FS: check `templates/` exists
FS-->>Prepare: found
Prepare->>FS: remove old `templates.zip` (if any)
Prepare->>FS: create `templates.zip` (AdmZip)
FS-->>Prepare: `templates.zip` created
Publish->>Registry: upload package (includes `templates.zip`)
note over Postinstall,FS `#E8F5E9`: Restore-on-install phase
Install->>Registry: fetch package
Registry-->>Install: package (contains `templates.zip`)
Install->>Postinstall: run postinstall script
Postinstall->>FS: check `templates/` (skip if exists)
Postinstall->>FS: validate `templates.zip` present
FS-->>Postinstall: `templates.zip` found
Postinstall->>FS: extract `templates.zip` → `templates/`
FS-->>Postinstall: extraction verified
Postinstall-->>Install: success
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
scripts/postinstall.ts (1)
14-17: Consider adding a log message for clarity.The early exit when templates already exist is correct behavior for source repos or re-installations. However, a brief log message could help developers understand why the script exits early.
🔎 Suggested enhancement
// Skip if templates folder already exists (source repo or already extracted) if (existsSync(templatesPath)) { + console.log('✓ Templates folder already exists, skipping extraction'); process.exit(0); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
.gitignorepackage.jsonscripts/postinstall.tsscripts/prepare.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Usebun <file>instead ofnode <file>orts-node <file>
Bun automatically loads .env files, so don't use the dotenv package
UseBun.serve()with built-in WebSocket, HTTPS, and route support instead ofexpress
Usebun:sqlitefor SQLite instead ofbetter-sqlite3
UseBun.redisfor Redis instead ofioredis
UseBun.sqlfor Postgres instead ofpgorpostgres.js
Use built-inWebSocketinstead ofwspackage
PreferBun.fileovernode:fsreadFile/writeFile for file operations
UseBun.$template literals for shell commands instead of execa
Files:
scripts/postinstall.tsscripts/prepare.ts
**/*.{ts,tsx,css,html}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
bun build <file.html|file.ts|file.css>instead ofwebpackoresbuild
Files:
scripts/postinstall.tsscripts/prepare.ts
**/*.{html,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use HTML imports with
Bun.serve()and don't usevitefor frontend bundling
Files:
scripts/postinstall.tsscripts/prepare.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use
bun --hot <file.ts>to run TypeScript files with hot module reloading enabled
Files:
scripts/postinstall.tsscripts/prepare.ts
package.json
📄 CodeRabbit inference engine (CLAUDE.md)
package.json: Usebun installinstead ofnpm install,yarn install, orpnpm install
Usebun run <script>instead ofnpm run <script>,yarn run <script>, orpnpm run <script>
Files:
package.json
🧠 Learnings (1)
📚 Learning: 2026-01-03T07:00:32.832Z
Learnt from: truehazker
Repo: truehazker/create-elysiajs PR: 2
File: templates/backend:1-1
Timestamp: 2026-01-03T07:00:32.832Z
Learning: In git diffs, symlinks (mode 120000) appear as added files containing the link target (e.g., +monorepo/apps/backend). Do not flag symlinks as placeholder files or accidental commits. This applies broadly; when reviewing files like templates/backend, verify whether it is a symlink and treat accordingly.
Applied to files:
package.json
🔇 Additional comments (6)
.gitignore (1)
9-10: LGTM! Correctly ignores the generated archive.The comment clearly documents why
templates.zipis ignored, and the placement in the output section is appropriate.scripts/prepare.ts (1)
1-39: LGTM! Clean implementation with proper error handling.The prepare script correctly:
- Validates prerequisites (templates folder exists)
- Cleans up existing archives before creating new ones
- Verifies the archive was created successfully
- Provides clear error messages
The workflow of adding folder contents to the zip root (line 28) correctly pairs with the postinstall extraction logic.
scripts/postinstall.ts (1)
25-36: LGTM! Robust extraction logic with good error handling.The extraction logic properly verifies success and provides helpful error messages including the GitHub issues link for troubleshooting.
package.json (3)
41-41: LGTM! Files array correctly configured for the zip workflow.The files array properly includes:
templates.zip: the packaged templates for distributionscripts/postinstall.ts: needed to extract templates after installation- Excludes
scripts/prepare.ts: correct, as it's only needed during development
45-46: LGTM! Scripts correctly implement the prepare/postinstall workflow.The scripts follow Bun best practices by using
bun runand will execute at the appropriate lifecycle stages:
prepare: runs before publishing to create the archivepostinstall: runs after installation to restore templates
57-57: adm-zip version 0.5.16 is the latest available release with no known security vulnerabilities.Version 0.5.16 is the current latest version (published August 30, 2024). The only historical CVE (CVE-2018-1002204) affected versions prior to 0.4.9 and has been resolved. No further action required.
Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
scripts/prepare.ts (1)
2-4: Consider using Bun-native file APIs instead of node:fs.Per coding guidelines, prefer
Bun.fileovernode:fsfor file operations. While the guideline specifically mentions readFile/writeFile, Bun provides native alternatives for the operations used here:
existsSync(path)→await Bun.file(path).exists()unlinkSync(path)→await Bun.$\rm ${path}`` or filesystem APIsHowever, since the current approach works and is well-supported in Bun, this is a minor optimization for consistency with Bun conventions.
🔎 Potential refactor using Bun-native APIs
-import { existsSync, unlinkSync } from 'node:fs'; import { join } from 'node:path'; import AdmZip from 'adm-zip';Then update the usage:
// Templates folder must exist -if (!existsSync(templatesPath)) { +if (!(await Bun.file(templatesPath).exists())) { console.error('ERROR: templates folder not found. Cannot create archive.'); process.exit(1); } try { // Remove existing zip if it exists - if (existsSync(zipPath)) { - unlinkSync(zipPath); + if (await Bun.file(zipPath).exists()) { + await Bun.$`rm ${zipPath}`; }Note: This would require making the script async or wrapping in an async IIFE.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
package.jsonscripts/postinstall.tsscripts/prepare.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- scripts/postinstall.ts
- package.json
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Usebun <file>instead ofnode <file>orts-node <file>
Bun automatically loads .env files, so don't use the dotenv package
UseBun.serve()with built-in WebSocket, HTTPS, and route support instead ofexpress
Usebun:sqlitefor SQLite instead ofbetter-sqlite3
UseBun.redisfor Redis instead ofioredis
UseBun.sqlfor Postgres instead ofpgorpostgres.js
Use built-inWebSocketinstead ofwspackage
PreferBun.fileovernode:fsreadFile/writeFile for file operations
UseBun.$template literals for shell commands instead of execa
Files:
scripts/prepare.ts
**/*.{ts,tsx,css,html}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
bun build <file.html|file.ts|file.css>instead ofwebpackoresbuild
Files:
scripts/prepare.ts
**/*.{html,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use HTML imports with
Bun.serve()and don't usevitefor frontend bundling
Files:
scripts/prepare.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use
bun --hot <file.ts>to run TypeScript files with hot module reloading enabled
Files:
scripts/prepare.ts
🔇 Additional comments (2)
scripts/prepare.ts (2)
26-29: Zip creation preserves directory structure correctly.The use of
addLocalFolder()ensures the templates directory contents are properly archived. This approach correctly preserves .gitignore files, which is the stated goal of this PR.
20-42: No action required. The code is already using adm-zip@0.5.16, which is the latest version available (released 2024-08-30).
1 participant
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.