Fix: add bundleHTML param back#83
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThe PR reverses the silent bundling default introduced in a prior commit. ChangesOption Defaulting & Type Deprecation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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: 1
🧹 Nitpick comments (1)
test/index.test.ts (1)
721-761: Consider testing the fallback behavior explicitly~ ♪Ne ne~ While your test updates are all correct, you don't have any test that specifically verifies the
bundleHTML→bunFullstackfallback logic works as intended~ (´。＿。｀)Like, what if someone sets
bundleHTML: falsewithout settingbunFullstack? Does it correctly disable bundling? Your code should handle it, but there's no test proving it~Want me to help you write one so you don't look silly when someone files a bug report later? ♡
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/index.test.ts` around lines 721 - 761, Add a new test in index.test.ts that explicitly exercises the bundleHTML → bunFullstack fallback: instantiate the app with staticPlugin({ bundleHTML: false }) but without setting bunFullstack, await app.modules, then request the HTML route (e.g., req('/public/html')) and assert that bundling is disabled by verifying the response behavior that differs when bundling is off (for example: content-type still 'text/html' but ETag or cache headers reflect unbundled output or that the file is served directly and not from a bundled cache); also add a complementary case where bundleHTML is undefined and bunFullstack is true to ensure the reverse fallback works. Ensure you reference staticPlugin, bundleHTML and bunFullstack in the test name and assertions so the intent is clear.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/index.ts`:
- Around line 39-40: Update the project documentation to reflect the new
bunFullstack option and deprecation of bundleHTML: in the README config section
add an entry for bunFullstack describing its purpose and behavior (including
that it overrides bundleHTML when defined) and annotate the existing bundleHTML
entry with an `@deprecated` note pointing readers to bunFullstack; then add a new
CHANGELOG entry describing that bundleHTML is deprecated in favor of
bunFullstack and noting the behavioral change (fallback/override logic). Make
sure to reference the symbols bundleHTML and bunFullstack in both README and
CHANGELOG so users can find the new/old config keys.
---
Nitpick comments:
In `@test/index.test.ts`:
- Around line 721-761: Add a new test in index.test.ts that explicitly exercises
the bundleHTML → bunFullstack fallback: instantiate the app with staticPlugin({
bundleHTML: false }) but without setting bunFullstack, await app.modules, then
request the HTML route (e.g., req('/public/html')) and assert that bundling is
disabled by verifying the response behavior that differs when bundling is off
(for example: content-type still 'text/html' but ETag or cache headers reflect
unbundled output or that the file is served directly and not from a bundled
cache); also add a complementary case where bundleHTML is undefined and
bunFullstack is true to ensure the reverse fallback works. Ensure you reference
staticPlugin, bundleHTML and bunFullstack in the test name and assertions so the
intent is clear.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a47c3a39-2a39-4b65-bef8-55a1ba1e87e5
📒 Files selected for processing (3)
src/index.tssrc/types.tstest/index.test.ts
| bundleHTML = true, | ||
| bunFullstack, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for any other files that might reference these options or have stale defaults
# Check for other references to bunFullstack or bundleHTML in the codebase
echo "=== Checking for bunFullstack references ==="
rg -n 'bunFullstack' --type ts -g '!test/*' -g '!*.test.ts'
echo -e "\n=== Checking for bundleHTML references ==="
rg -n 'bundleHTML' --type ts -g '!test/*' -g '!*.test.ts'
# Check if there are any README or docs that need updating
echo -e "\n=== Checking documentation files ==="
fd -e md . | head -5Repository: elysiajs/elysia-static
Length of output: 1332
🏁 Script executed:
# Check README.md for bundleHTML and bunFullstack references
echo "=== README.md content ==="
cat README.md | head -100
echo -e "\n=== CHANGELOG.md content ==="
head -50 CHANGELOG.mdRepository: elysiajs/elysia-static
Length of output: 2410
Documentation got left behind~ ♡
Ohhh the code is smaaarrt~ The fallback logic is cleeeever, bunFullstack overriding bundleHTML when undefined~ (≧◡≦) But you sloppy developer, you totally forgot to update the documentation files!!
The README.md is missing both bundleHTML and bunFullstack from the config section - it only documents assets, prefix, staticLimit, and alwaysStatic~ Plus, CHANGELOG.md doesn't have any entry explaining the deprecation of bundleHTML or the new bunFullstack property~ ( ̄ω ̄)
You need to:
- Add documentation for
bunFullstackand markbundleHTMLas@deprecatedin README.md's config section - Add a CHANGELOG entry documenting this change
Tsk tsk, code changes but documentation rot~ How typical~ ♡
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/index.ts` around lines 39 - 40, Update the project documentation to
reflect the new bunFullstack option and deprecation of bundleHTML: in the README
config section add an entry for bunFullstack describing its purpose and behavior
(including that it overrides bundleHTML when defined) and annotate the existing
bundleHTML entry with an `@deprecated` note pointing readers to bunFullstack; then
add a new CHANGELOG entry describing that bundleHTML is deprecated in favor of
bunFullstack and noting the behavioral change (fallback/override logic). Make
sure to reference the symbols bundleHTML and bunFullstack in both README and
CHANGELOG so users can find the new/old config keys.
1 participant
If
bunFullstackisn't set, thebundleHTMLparameter is used instead, which defaults to true. (this preserves previous behavior)closes #63
Summary by CodeRabbit
Documentation
bundleHTMLoption is now deprecated; usebunFullstackinsteadbunFullstacktakes precedence when both options are specifiedRefactor