feat: Add bundleHTML option for HTML file handling#57
feat: Add bundleHTML option for HTML file handling#57pa001024 wants to merge 4 commits intoelysiajs:mainfrom
Conversation
WalkthroughAdds a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/index.ts (1)
229-246: Same type safety and error handling concerns apply here.This code segment has the same issues identified in lines 90-100:
- Missing explicit type annotation for
htmlFile- No error handling around the dynamic import at line 237
- Needs verification that HTML module imports work as expected
Apply similar error handling:
for (const absolutePath of htmls) { if (!absolutePath || shouldIgnore(absolutePath)) continue let relativePath = absolutePath.replace(assetsDir, '') const pathName = normalizePath(path.join(prefix, relativePath)) - const htmlFile = bundleHTML ? (await import(absolutePath)).default : getFile(absolutePath) + let htmlFile + try { + htmlFile = bundleHTML ? (await import(absolutePath)).default : getFile(absolutePath) + } catch (error) { + if (!silent) + console.error( + `[@elysiajs/static] Failed to load HTML file: ${absolutePath}`, + error + ) + continue + } app.get(pathName, htmlFile)
🧹 Nitpick comments (1)
src/types.ts (1)
104-109: Clarify the JSDoc description for better developer understanding.The description "Enable bun bundle of *.html" is brief and doesn't explain what bundling means or its implications. Consider expanding it to clarify:
- That this option only applies when running on Bun
- What "bundling" means (importing HTML as modules vs serving directly from disk)
- Performance implications (bundled at startup vs read on demand)
Example improvement:
/** * @default true * - * Enable bun bundle of *.html + * Enable bundling of HTML files (Bun only). + * When true, HTML files are imported as modules at startup for better performance. + * When false, HTML files are served directly from disk. */ bundleHTML?: boolean
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/index.ts(3 hunks)src/types.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/index.ts (1)
src/utils.ts (1)
getFile(194-199)
🔇 Additional comments (1)
src/index.ts (1)
33-33: LGTM!The default value of
truemaintains backward compatibility while allowing users to opt into direct file serving when needed.
Added error handling for loading HTML files to prevent crashes and provide console error messages.
Updated documentation for bundleHTML option to clarify functionality.
#54
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.