Skip to content

fix(cli): traverse parent directories to find vite.config.ts in vp pack#1072

Merged
fengmk2 merged 10 commits intovoidzero-dev:mainfrom
kazupon:fix/942
Mar 22, 2026
Merged

fix(cli): traverse parent directories to find vite.config.ts in vp pack#1072
fengmk2 merged 10 commits intovoidzero-dev:mainfrom
kazupon:fix/942

Conversation

@kazupon
Copy link
Collaborator

@kazupon kazupon commented Mar 20, 2026

resolve #942

@netlify
Copy link

netlify bot commented Mar 20, 2026

Deploy Preview for viteplus-preview canceled.

Name Link
🔨 Latest commit 925fd97
🔍 Latest deploy log https://app.netlify.com/projects/viteplus-preview/deploys/69bfb8c68f584c00082cf550

@kazupon kazupon marked this pull request as ready for review March 21, 2026 05:49
@kazupon
Copy link
Collaborator Author

kazupon commented Mar 21, 2026

@codex review

@chatgpt-codex-connector
Copy link

Codex Review: Didn't find any major issues. More of your lovely PRs please.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@kazupon kazupon requested a review from fengmk2 March 21, 2026 06:06
@fengmk2
Copy link
Member

fengmk2 commented Mar 21, 2026

What about other commands? Does the build need to consider similar logic?

@kazupon
Copy link
Collaborator Author

kazupon commented Mar 21, 2026

Good point!

I investigated how each command resolves vite config:

Command Config resolution Call site traverseUp status
vp pack resolveViteConfig (JS) pack-bin.ts:131 OK: Added in this PR
vp staged resolveViteConfig (JS) staged/bin.ts:143 NG: Should add
vp build / vp dev delegate::execute → Vite's resolveConfig cli.rs:1876 → Vite NG: Should add
vp lint / vp fmt resolveUniversalViteConfig (Rust→JS) cli.rsbin.ts:92 NG: Needs investigation
vp run Rust workspace-aware task runner cli.rs:1262 OK: Already handled

Since vite+ is the unified toolchain and users interact through vp commands, config resolution (including traverseUp) should be vite+'s responsibility for all commands, not just vp pack.

For vp build/vp dev, we can find the config via findViteConfigUp and pass it as --config to Vite.
For vp staged, we can add { traverseUp: true } to the existing resolveViteConfig call.

@kazupon
Copy link
Collaborator Author

kazupon commented Mar 21, 2026

One thing I noticed during this investigation, the vite config resolution layer is currently split across JS and Rust:

  • JS side: vp pack, vp stagedresolveViteConfig
  • Rust→JS side: vp lint, vp fmt, vp runVitePlusConfigLoaderresolveUniversalViteConfig
  • Vite itself: vp build, vp dev → Vite's internal resolveConfig

The Rust side handles config resolution for vp lint/vp fmt/vp run because oxlint, oxfmt, and vite-task are Rust binaries that need the config.
This is reasonable, but the split means that cross-cutting features like traverseUp need to be applied in multiple places, which can lead to inconsistent behavior between commands.

Since vite+ unifies tool configuration through vite.config.ts (a JS/TS file), it might be worth considering consolidating config resolution to the JS side in the long term.
The Rust side's vite_static_config (static analysis) could remain as a performance optimization with JS as the fallback, which is already the current design.

The solution to this root cause goes well beyond the scope of this issue.
we should discuss it in a separate issue. /cc @sapphi-red

@Brooooooklyn Brooooooklyn added the test: e2e Auto run e2e tests label Mar 21, 2026
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

const packConfigs = Array.isArray(viteConfig.pack)
? viteConfig.pack
: [viteConfig.pack ?? {}];
for (const packConfig of packConfigs) {

P2 Badge Avoid building every root pack entry from a package subdirectory

If the newly discovered workspace-root vite.config.* contains pack: [...], this code will execute every entry even when vp pack is launched from a single package directory. There is no cwd-based narrowing before iterating viteConfig.pack, so a monorepo script like vp run foo#build can unexpectedly build sibling packages unless the user manually adds -F; that defeats the point of making package-local invocations reuse the parent config.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@kazupon
Copy link
Collaborator Author

kazupon commented Mar 21, 2026

@codex

If the newly discovered workspace-root vite.config.* contains pack: [...], this code will execute every entry even when vp pack is launched from a single package directory. There is no cwd-based narrowing before iterating viteConfig.pack, so a monorepo script like vp run foo#build can unexpectedly build sibling packages unless the user manually adds -F; that defeats the point of making package-local invocations reuse the parent config.

As I understand it, tasks are cached by vite-task, the task runner within vp, so they will not be executed again.
Verify test project is here:
https://github.com/kazupon/vp-942-array-test

@chatgpt-codex-connector
Copy link

To use Codex here, create an environment for this repo.

@kazupon
Copy link
Collaborator Author

kazupon commented Mar 21, 2026

I reconsidered the behavior of vp staged, vp lint, and vp fmt when run from a subpackage without a vite.config.ts.

vp staged is called from the pre-commit hook (.vite-hooks/pre-commit), which always runs from the repository root.
vp staged is unlikely to be called from a subpackage directory rarely.
I think that vp staged may not need traverseUp.

For vp lint / vp fmt, when run directly from a subpackage without a vite.config.ts, they fall back to default settings (no custom rules).
Fortunately, these commands should not result in an error and should run by default. If that happens, the user will realize that no configuration has been set and will configure vite.config.

I'll keep traverseUp scoped to vp pack for this PR.
The other commands can be addressed in a follow-up if needed.

@fengmk2
Copy link
Member

fengmk2 commented Mar 22, 2026

snap tests result change after vite-task update. I will fix it before merge.

Cache hit is not working for pack monorepo builds currently because
the build modifies its own input. Update the test to capture full
output instead of grepping for cache hit.
@fengmk2
Copy link
Member

fengmk2 commented Mar 22, 2026

snap tests result change after vite-task update. I will fix it before merge.

follow up here #1095

@fengmk2 fengmk2 merged commit 7e8d95e into voidzero-dev:main Mar 22, 2026
47 checks passed
@kazupon kazupon deleted the fix/942 branch March 22, 2026 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test: e2e Auto run e2e tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: vp pack does not traverse upward to read pack configuration from vite.config.ts

3 participants