Skip to content

Add Vue integration test with 1000 components#20055

Merged
RobinMalfait merged 3 commits into
mainfrom
fix/lots-of-references
May 14, 2026
Merged

Add Vue integration test with 1000 components#20055
RobinMalfait merged 3 commits into
mainfrom
fix/lots-of-references

Conversation

@RobinMalfait
Copy link
Copy Markdown
Member

@RobinMalfait RobinMalfait commented May 14, 2026

This PR adds an integration test with Vue where we use a 1000 components and where each component references a CSS file via @reference. Each component has a unique class that uses @apply.

There are some discussions in #16429 that mention that this causes OOM issues. Right now I can't reproduce that, and even with a 1000 components, it produces CSS in a reasonable time:

vite v7.3.3 building client environment for production...
✓ 2011 modules transformed.
dist/index.html                   0.23 kB │ gzip:  0.18 kB
dist/assets/index-DNVNFkYQ.css  106.65 kB │ gzip: 10.79 kB
dist/assets/index-B8v7EbAN.js   223.84 kB │ gzip: 49.81 kB
✓ built in 3.17s

I also started a Vite server and triggered file changes to see if the memory would grow forever, which it didn't. After a 1000 changes, everything still behaves smoothly:
image

Making changes manually to a single component, result in proper HMR request that update the browser:

Screen.Recording.2026-05-14.at.15.06.03.mov

This test is here to make sure that it keeps working in the future.


If I remove all @reference references, and usages of @apply, then the build time is indeed faster:

vite v7.3.3 building client environment for production...
✓ 2011 modules transformed.
dist/index.html                   0.23 kB │ gzip:  0.18 kB
dist/assets/index-CcxXccJ1.css  106.61 kB │ gzip: 10.76 kB
dist/assets/index-M92YFF0G.js   223.84 kB │ gzip: 49.81 kB
✓ built in 1.97s

So we go from 1.97s3.17s, which is a 1.2s increase when you use @reference with @apply in 1000 files for a fresh build.

I also saw some comments about the CSS growing whenever @reference was used, but as you can see in the snippets above they are at a stable size.

Test plan

  1. All tests still pass

[ci-all] For testing on Windows / macOS

@RobinMalfait RobinMalfait requested a review from a team as a code owner May 14, 2026 13:08
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 14, 2026

Review Change Stack

Walkthrough

This PR updates the Vue Vite integration test file to expand test coverage. It imports a css helper utility, adds a new production-build test that generates 1,000 Vue components referencing a shared stylesheet via @reference directives, and verifies the resulting built CSS contains expected component-scoped rules. An existing error-case test is also adjusted to suppress stderr output during the build invocation.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The PR title clearly describes the main change—adding a Vue integration test with 1000 components—which directly matches the changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The pull request description clearly explains the purpose of the test: adding an integration test for Vue with 1000 components using @reference to CSS files with @apply directives, and documents performance findings and HMR behavior.

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

Warning

Review ran into problems

🔥 Problems

Stopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a @coderabbit review after the pipeline has finished.


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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
integrations/vite/vue.test.ts (1)

168-173: ⚡ Quick win

Strengthen the assertion to cover the 1000-component upper bound.

This test’s purpose is a 1,000-component stress case, but current checks stop at component-99. Add at least one near-tail index (for example 999) so truncation/regression past the first 100 components is caught.

Proposed diff
       await fs.expectFileToContain(files[0][0], [
         candidate`content-['component-0']`,
         candidate`component-0`,
         candidate`content-['component-99']`,
         candidate`component-99`,
+        candidate`content-['component-999']`,
+        candidate`component-999`,
       ])
🤖 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 `@integrations/vite/vue.test.ts` around lines 168 - 173, The test currently
asserts presence of only up to component-99, so extend the
fs.expectFileToContain call (the array passed to fs.expectFileToContain in the
test) to include a near-tail index such as candidate`content-['component-999']`
and candidate`component-999` (alongside the existing entries) so the
1,000-component stress case verifies elements near the end and catches
truncation/regressions; update the array in the await
fs.expectFileToContain(files[0][0], [...]) invocation accordingly.
🤖 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.

Nitpick comments:
In `@integrations/vite/vue.test.ts`:
- Around line 168-173: The test currently asserts presence of only up to
component-99, so extend the fs.expectFileToContain call (the array passed to
fs.expectFileToContain in the test) to include a near-tail index such as
candidate`content-['component-999']` and candidate`component-999` (alongside the
existing entries) so the 1,000-component stress case verifies elements near the
end and catches truncation/regressions; update the array in the await
fs.expectFileToContain(files[0][0], [...]) invocation accordingly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 0f708ed2-c7bb-4714-9f52-39b96a3414bc

📥 Commits

Reviewing files that changed from the base of the PR and between 559f92b and fc39a89.

📒 Files selected for processing (1)
  • integrations/vite/vue.test.ts

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 14, 2026

Confidence Score: 5/5

This PR only adds a new integration test and a minor fix to an existing test — no production code is touched.

The change is purely additive test infrastructure. The new test correctly exercises the @reference + @apply with 1000 components scenario, assertions cover components 0, 99, and 999 (spanning the full range), dependency installation and build comfortably fit within the default 60 s timeout, and path resolution for @reference '../main.css' from src/components/ is correct.

No files require special attention.

Reviews (2): Last reviewed commit: "Update integrations/vite/vue.test.ts" | Re-trigger Greptile

Comment thread integrations/vite/vue.test.ts
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@RobinMalfait RobinMalfait merged commit 0db226a into main May 14, 2026
23 checks passed
@RobinMalfait RobinMalfait deleted the fix/lots-of-references branch May 14, 2026 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant