fix(vite-plugin): properly remove css file from bundle when file is below inlineThreshold by mywill · Pull Request #299 · danielroe/beasties

mywill · 2026-04-13T00:58:13Z

Summary:

checkInlineThreshold in the vite-plugin-beasties has a parameter mismatch causing an error when the inlineThreshold is set preventing css from inlining

Solution:

added a test showing the issue
Updated the parameters for checkInlineThreshold to fix the mismatch

Resolves: #298

…elow inlineThreshold

coderabbitai · 2026-04-13T00:58:26Z

Walkthrough

The pull request fixes a bug where CSS files were not fully removed when inlineThreshold was configured and the CSS file size was below the threshold. The fix updates the checkInlineThreshold wrapper method signature from (style, before, sheetInverse) to (link, style, sheet) and corrects the emptiness check from !sheetInverse.length to !sheet.length.

Changes

Cohort / File(s)	Summary
Plugin implementation `packages/vite-plugin-beasties/src/index.ts`	Updated `beastiesInstance.checkInlineThreshold` wrapper to accept the correct parameter signature `(link, style, sheet)` and changed the conditional logic from `!sheetInverse.length` to `!sheet.length` to properly determine when to remove inlined CSS assets from the bundle.
Test coverage `packages/vite-plugin-beasties/test/index.test.ts`	Added test case verifying that CSS files below `inlineThreshold` are fully inlined into HTML and no external stylesheet remains in the bundle. Validates presence of inline `<style>` block with CSS rules and absence of `<link rel="stylesheet">` tags.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

fix: preserve external stylesheets order #246: Adapts the plugin's overridden checkInlineThreshold to the new (link, style, sheet) signature introduced by the beasties core refactor.
refactor(beasties-webpack-plugin): rewrite to typescript #213: Updates the webpack-plugin's checkInlineThreshold signature and logic to match the same parameter changes.
feat(vite-plugin-beasties): add vite plugin #119: Introduces the renamed/shifted parameters in the beasties API that this PR now aligns with.

Suggested reviewers

danielroe

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main fix: correcting parameter mismatch in checkInlineThreshold to properly remove CSS files when below the inlineThreshold.
Description check	✅ Passed	The description clearly relates to the changeset, explaining the parameter mismatch issue, the test addition, and the parameter fix applied.
Linked Issues check	✅ Passed	The PR successfully addresses issue `#298` by fixing the parameter mismatch in checkInlineThreshold (from (style, before, sheetInverse) to (link, style, sheet)) and adds a test case verifying CSS inlining and file removal work correctly.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to resolving the linked issue: parameter fix in index.ts and test case addition in index.test.ts with no extraneous modifications.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/vite-plugin-beasties/src/index.ts`:
- Around line 86-89: The current condition in
beastiesInstance.checkInlineThreshold uses "if (isStyleInlined ||
!sheet.length)" which causes assets to be deleted when the stylesheet is empty
even if originalCheckInline did not inline it; change the guard to only check
the inlining result by replacing that condition with "if (isStyleInlined)" so
that deletion logic runs only when originalCheckInline returned true (refer to
beastiesInstance.checkInlineThreshold and originalCheckInline in your diff).

🪄 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: 1da23d58-42a9-486f-84d8-341dcc176422

📥 Commits

Reviewing files that changed from the base of the PR and between b3254e6 and 3cc634d.

📒 Files selected for processing (2)

packages/vite-plugin-beasties/src/index.ts
packages/vite-plugin-beasties/test/index.test.ts

coderabbitai · 2026-04-13T01:01:12Z

+      beastiesInstance.checkInlineThreshold = function checkInlineThreshold(link, style, sheet) {
+        const isStyleInlined = originalCheckInline(link, style, sheet)

-        if (isStyleInlined || !sheetInverse.length) {
+        if (isStyleInlined || !sheet.length) {


⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash # Inspect where and how checkInlineThreshold is called, and whether # an empty `sheet` can occur without full inlining/link removal. rg -n -C6 '\bcheckInlineThreshold\s*\(' packages/beasties/src/index.ts packages/vite-plugin-beasties/src/index.ts rg -n -C6 '\binlineThreshold\b|\blink\.remove\s*\(' packages/beasties/src/index.ts

Repository: danielroe/beasties

Length of output: 3484

Condition at line 89 is unsafe and causes broken references when stylesheets are empty but not inlined.

The || !sheet.length clause deletes bundle assets unconditionally when the sheet is empty, even when inlineThreshold did not trigger inlining. This leaves the HTML with a <link> tag pointing to a deleted asset.

The condition must be narrowed to:

if (isStyleInlined) {

Only delete the asset when the original checkInlineThreshold actually inlined the stylesheet (returned true).

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@packages/vite-plugin-beasties/src/index.ts` around lines 86 - 89, The current condition in beastiesInstance.checkInlineThreshold uses "if (isStyleInlined || !sheet.length)" which causes assets to be deleted when the stylesheet is empty even if originalCheckInline did not inline it; change the guard to only check the inlining result by replacing that condition with "if (isStyleInlined)" so that deletion logic runs only when originalCheckInline returned true (refer to beastiesInstance.checkInlineThreshold and originalCheckInline in your diff).

So i can look to change this but i was keeping the logic the same as that was not the issue i was fixing. I don't think this would delete anything important as the sheet length would have to be of length 0 which would be an empty resource so would be something that shouldn't get bundled anyway

@mywill, you make a valid point. Looking more carefully at the diff, the || !sheet.length clause is pre-existing behavior — the old code had || !sheetInverse.length (checking the third parameter), and the new code has || !sheet.length (still checking the third parameter, just with the corrected name). This PR only renames the parameters to match the actual checkInlineThreshold(link, style, sheet) signature; the logic itself is unchanged.

Your reasoning is also sound: if sheet.length === 0, the stylesheet is empty, and removing an empty CSS asset from the bundle is a safe cleanup — there's nothing meaningful left to serve. My concern was overstated for this PR. I'll withdraw it.

( ˶ᵔ ᵕ ᵔ˶ )

fix(vite-plugin): properly remove css file from bundle when file is b…

3cc634d

…elow inlineThreshold

coderabbitai Bot reviewed Apr 13, 2026

View reviewed changes

Merge branch 'main' into fix/vite-plugin-inline-threshold

b8a312b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(vite-plugin): properly remove css file from bundle when file is below inlineThreshold#299

fix(vite-plugin): properly remove css file from bundle when file is below inlineThreshold#299
mywill wants to merge 2 commits into
danielroe:mainfrom
mywill:fix/vite-plugin-inline-threshold

mywill commented Apr 13, 2026

Uh oh!

coderabbitai Bot commented Apr 13, 2026 •

edited

Loading

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot Apr 13, 2026 •

edited

Loading

Uh oh!

mywill Apr 13, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot Apr 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

mywill commented Apr 13, 2026

Uh oh!

coderabbitai Bot commented Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mywill Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

coderabbitai Bot commented Apr 13, 2026 •

edited

Loading

coderabbitai Bot Apr 13, 2026 •

edited

Loading

mywill Apr 13, 2026 •

edited

Loading