feat(cli): add --no-type-check to vp check#1417
feat(cli): add --no-type-check to vp check#1417jong-kyung wants to merge 10 commits intovoidzero-dev:mainfrom
Conversation
✅ Deploy Preview for viteplus-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e835c07184
ℹ️ 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".
| let raw_temp_path = std::env::temp_dir().join(filename_str); | ||
| let sidecar_path = AbsolutePathBuf::new(raw_temp_path).ok_or_else(|| { |
There was a problem hiding this comment.
This writes the --no-type-check override config under the system temp directory, which changes how Oxlint resolves config-file-relative entries. Oxlint resolves extends and JS plugin specifiers relative to the -c file path (for example, see https://oxc.rs/docs/guide/usage/linter/nested-config and https://oxc.rs/docs/guide/usage/linter/js-plugins), so a project using relative extends/jsPlugins will break or lint the wrong config when vp check --no-type-check points to /tmp/vite-plus-no-type-check-*.mjs instead of the project config location.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Keep sidecar config in project path scope
This writes the
--no-type-checkoverride config under the system temp directory, which changes how Oxlint resolves config-file-relative entries. Oxlint resolvesextendsand JS plugin specifiers relative to the-cfile path (for example, see https://oxc.rs/docs/guide/usage/linter/nested-config and https://oxc.rs/docs/guide/usage/linter/js-plugins), so a project using relativeextends/jsPluginswill break or lint the wrong config whenvp check --no-type-checkpoints to/tmp/vite-plus-no-type-check-*.mjsinstead of the project config location.Useful? React with 👍 / 👎.
This path doesn't go through standard oxlint config resolution. With VP_VERSION set, cli.ts uses loadVitePlusConfigs, which import()s the sidecar and hands .default.lint to Rust as a plain JSON object — the sidecar file path isn't a resolution root.
| resolver, | ||
| SynthesizableSubcommand::Lint { args }, | ||
| Some(&resolved_vite_config), | ||
| Some(lint_vite_config), |
There was a problem hiding this comment.
#1378 The -c parameter specifying oxlint configuration will be removed, so the sidecar configuration approach will have issues. Going forward, the configuration resolution logic for oxlint and oxfmt will no longer be controlled by Vite+, and will be handed back to oxlint/fmt themselves to handle.
There was a problem hiding this comment.
will be handed back to oxlint/fmt themselves to handle.
I think this is the only option.
Otherwise, Vite+ would end up handling everything, including searching for nested configs.
(That might be a valid decision in itself, but as far as I know, there are currently no plans to do so?)
There was a problem hiding this comment.
#1378 The
-cparameter specifying oxlint configuration will be removed, so the sidecar configuration approach will have issues. Going forward, the configuration resolution logic for oxlint and oxfmt will no longer be controlled by Vite+, and will be handed back to oxlint/fmt themselves to handle.
Thanks for the heads up on #1378. Since the sidecar + -c approach won't survive that change, what would be the recommended direction for implementing --no-type-check going forward?
There was a problem hiding this comment.
@fengmk2 what's the use case for the --no-type-check flag?
4 participants
Summary
Implements the 3-phase skip-flag matrix for
vp checkagreed on in #1275.Each of format / lint rules / type check can now be skipped independently:
--no-fmt--no-lint--no-type-checkvp check --fix --no-linton atypeCheck: trueproject is rejected up front to avoid leaving the working tree partially formatted inside pre-commit hooks.Request for feedback:
sidecar.rs--no-type-checkis implemented via a per-invocation.mjssidecar that overrideslint.options.typeCheck. It leans on oxlint'sloadVitePlusConfigsroute loading-c <path>via dynamicimport(), so we can toggle the flag without mutatingvite.config.ts.This is the part of the PR I'd especially appreciate reviewer input on.
One approach I also considered was a wrapper
.mjsthat imports the originalvite.config.tsand patchestypeCheckin place — this would preserve non-serializable lint values but complicates relative-path resolution from the temp dir. Happy to switch to that, or to another pattern if the project has a preferred direction for per-invocation config overrides in the Rust binding.Related
vp typecheckcommand #1275--type-check-onlyflag #1387