test(test-dev-server): drop the vite submodule, track rolldown-canary#10319
Merged
graphite-app[bot] merged 1 commit intoJul 17, 2026
Conversation
Member
Author
How to use the Graphite Merge QueueAdd the label graphite: merge-when-ready to this PR to add it to the merge queue. You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
shulaoda
marked this pull request as ready for review
July 16, 2026 14:44
shulaoda
requested review from
IWANABETHATGUY,
h-a-n-a,
hyfdev and
sapphi-red
as code owners
July 16, 2026 14:44
Member
|
Should reuse the checkout from #10318 (comment) |
sapphi-red
reviewed
Jul 17, 2026
h-a-n-a
approved these changes
Jul 17, 2026
Contributor
Merge activity
|
…#10319) Stacked on the vite-tests PR. Together they make both Vite harnesses run on the same code: the latest `rolldown-canary` rebased onto the latest `main`. ## Problem The dev-server test harness built the vite submodule at its pinned commit. Same pin-rot problem as vite-tests: `rolldown-canary` is force-pushed on every update, so a pinned commit ends up on no branch, and canary-side fixes need a manual pin bump to reach CI. The pin mechanism fundamentally conflicts with tracking a rebased branch. ## Change - **Remove the vite git submodule entirely**: the `.gitmodules` entry and the `vite` gitlink are gone. `vite/` at the repo root is now a plain, gitignored clone. - **`scripts/src/setup-vite/checkout.ts`**: when `vite/` is missing (fresh clone, CI), it clones vitejs/vite `rolldown-canary` and rebases it onto `origin/main`. An existing checkout is never moved: updating it stays a manual git step inside `vite/`, so local work in the checkout is never trampled. - **`reusable-node-dev-server-test.yml`**: adds a "Configure Git" step, since the rebase needs a committer identity in CI (the vite-test jobs in ci.yml already had one). - Wording updates everywhere the submodule or the pin was mentioned: `setup-vite` scripts, `vite-server.ts` comments and error messages, `.gitignore`, `justfile`, `testing.md`, `repo-structure.md`, `implementation.md`, `tests/AGENTS.md`. Fixes and test adjustments for the Vite side now always belong on the vitejs/vite `rolldown-canary` branch, for both harnesses. Jobs that check out with `submodules: true` are unaffected: only the `rollup` and `test262` submodules remain. ## Verification - Fresh `just setup-vite` end to end: clone `rolldown-canary`, rebase onto `main`, `vp install`, vite build, rolldown symlink swap and resolution check all pass. - Dev-server browser suite smoke (`hmr-client-invalidate`, 4 tests) passes on the resulting checkout with a current rolldown `main` build. - `just lint` fully green (fmt, oxlint/tsc, knip, publint, ls-lint). - A repo-wide sweep found no remaining references to the submodule or the pin mechanism.
graphite-app
Bot
force-pushed
the
07-16-test_vite-tests_track_the_latest_rolldown-canary_rebased_onto_vite_main
branch
from
July 17, 2026 04:25
9db3bd7 to
479f45d
Compare
graphite-app
Bot
force-pushed
the
07-16-test_test-dev-server_drop_the_vite_submodule_track_rolldown-canary
branch
from
July 17, 2026 04:25
d4411c9 to
da36734
Compare
graphite-app Bot
pushed a commit
that referenced
this pull request
Jul 17, 2026
Follow-up to the review comments on #10318 and #10319: both test projects should be set up from the single `vite/` checkout at the workspace root, via the `just setup-vite` checkout step. ## Change `packages/vite-tests/run.ts` no longer clones vitejs/vite from GitHub on its own. It now: 1. calls `ensureViteCheckout()` from `scripts/src/setup-vite/checkout.ts` (the checkout step of `just setup-vite`), which guarantees the root `vite/` checkout exists and is at the latest `rolldown-canary` rebased onto the latest `main`, exactly what #10319 set up for the dev-server tests. 2. clones that checkout locally into `./repo` and runs Vite's test suite there. The local clone shares objects via hardlinks, so no extra network download. It stays a throwaway copy because this suite must edit tracked files (rewrite the pnpm overrides to point `rolldown` at the workspace package, which also rewrites the lockfile on install), while `vite/` itself must stay unpatched: the dev-server setup installs it with `--frozen-lockfile` and builds it with Vite's own pinned rolldown, so the two harnesses would corrupt each other's state if they shared one working copy. Both harnesses therefore run on exactly the same Vite code: one checkout, one rebase result, no chance of the two suites drifting apart because `rolldown-canary` or `main` moved between two independent clones. Per the review discussion on #10319, an existing checkout is kept up to date as well: when it is clean and on `rolldown-canary` (the managed state), every setup fetches and re-derives the latest `rolldown-canary` rebased onto `main` (kept as-is when the fetch fails, e.g. offline). A checkout taken over by the developer (dirty, or switched to another branch) is used exactly as-is, with a notice, so local experiments inside `vite/` are never trampled and can still be tested through both harnesses. Also restores the `checkout.ts` include in `packages/vite-tests/tsconfig.json` and updates the docs accordingly. ## Verification - Local smoke run: `run.ts` picks up the shared checkout, clones it locally, and proceeds through install, build, and into the test suites. - All three checkout states exercised live: a clean checkout on `rolldown-canary` was updated to the latest upstream state; a dirty checkout and a detached HEAD were left as-is with a notice. - `just lint` fully green.
Base automatically changed from
07-16-test_vite-tests_track_the_latest_rolldown-canary_rebased_onto_vite_main
to
main
July 17, 2026 04:29
graphite-app
Bot
deleted the
07-16-test_test-dev-server_drop_the_vite_submodule_track_rolldown-canary
branch
July 17, 2026 04:29
hyfdev
reviewed
Jul 17, 2026
hyfdev
left a comment
hyfdev
left a comment
Member
There was a problem hiding this comment.
Post-merge note: dropping the submodule and cloning rolldown-canary rebased onto main is the right model for CI (fresh checkouts), and the Configure Git step for the rebase is correct. Dev-server Ubuntu jobs green on this head support that path.
I left one follow-up about leftover local submodule checkouts. If this review had landed before merge, the verdict would have been Approve with that note.
| // shallow `--depth 1` clone would leave the checkout grafted with no | ||
| // ancestry — fine for a one-off build, but not for development. | ||
| run('git submodule update --init vite', repoRoot); | ||
| if (!nodeFs.existsSync(nodePath.join(viteDir, 'package.json'))) { |
Member
There was a problem hiding this comment.
After the submodule is removed, a leftover local vite/ from git submodule update --init is still a detached HEAD at the old pin. That tree has package.json, so this branch never re-clones it.
The auto-update path added in #10325 also leaves it alone, because git rev-parse --abbrev-ref HEAD is HEAD, not rolldown-canary, so it is treated as developer takeover. Anyone who already had the submodule keeps testing the rotten pin until they delete vite/ by hand.
This does not block the CI path (fresh runners always clone). A short migration note in the testing docs (rm -rf vite && just setup-vite), or treating a clean detached/submodule leftover as re-bootstrapable, would close the gap for local checkouts.
This does not block this approval.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on the vite-tests PR. Together they make both Vite harnesses run on the same code: the latest
rolldown-canaryrebased onto the latestmain.Problem
The dev-server test harness built the vite submodule at its pinned commit. Same pin-rot problem as vite-tests:
rolldown-canaryis force-pushed on every update, so a pinned commit ends up on no branch, and canary-side fixes need a manual pin bump to reach CI. The pin mechanism fundamentally conflicts with tracking a rebased branch.Change
.gitmodulesentry and thevitegitlink are gone.vite/at the repo root is now a plain, gitignored clone.scripts/src/setup-vite/checkout.ts: whenvite/is missing (fresh clone, CI), it clones vitejs/viterolldown-canaryand rebases it ontoorigin/main. An existing checkout is never moved: updating it stays a manual git step insidevite/, so local work in the checkout is never trampled.reusable-node-dev-server-test.yml: adds a "Configure Git" step, since the rebase needs a committer identity in CI (the vite-test jobs in ci.yml already had one).setup-vitescripts,vite-server.tscomments and error messages,.gitignore,justfile,testing.md,repo-structure.md,implementation.md,tests/AGENTS.md.Fixes and test adjustments for the Vite side now always belong on the vitejs/vite
rolldown-canarybranch, for both harnesses.Jobs that check out with
submodules: trueare unaffected: only therollupandtest262submodules remain.Verification
just setup-viteend to end: clonerolldown-canary, rebase ontomain,vp install, vite build, rolldown symlink swap and resolution check all pass.hmr-client-invalidate, 4 tests) passes on the resulting checkout with a current rolldownmainbuild.just lintfully green (fmt, oxlint/tsc, knip, publint, ls-lint).