Refactor plugin to use configResolved hook
#682
Conversation
…for improved configuration access.
🦋 Changeset detectedLatest commit: b2c00b9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThe vite-plugin is refactored to use Vite's Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
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. Comment |
commit: |
📦 Bundle Size Report
✅ All size limits passed! |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/vite-plugin/src/index.ts (1)
54-91: Move config.define mutation to theconfighook instead ofconfigResolved.Vite's plugin API documentation explicitly states that
configResolvedis for reading the final resolved config only—not for mutations. Theconfighook (which runs before resolution) is the standard approach for modifying configuration. While the current approach with@ts-expect-errorworks in practice, it violates Vite's documented API contract and could break if Vite enforces immutability on the resolved config object in future versions.The fix: Return a partial config object from the
confighook with thedefineoptions merged in, rather than mutating inconfigResolved. This aligns with Vite's plugin patterns and removes the type suppression.
🧹 Nitpick comments (1)
packages/vite-plugin/src/index.ts (1)
86-90: Document why@ts-expect-erroris necessary here.While the comment explains that
config.defineis mutable in practice despite being readonly in types, consider adding a more detailed explanation of:
- Why mutation in
configResolvedwas chosen over using theconfighook- The exploratory/experimental nature of this approach (as noted in the PR objectives)
- Any known limitations or future considerations
This helps future maintainers understand the intentional deviation from TypeScript's type safety.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.changeset/cold-lemons-cut.md.vscode/tasks.jsonpackages/vite-plugin/src/index.test.tspackages/vite-plugin/src/index.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)
**/*.{ts,tsx}: Prefertypeoverinterfacefor type definitions in TypeScript
Use TypeScript 5.1+ features when appropriate
Leverageconsttype parameters for better inference in TypeScript
Use JSDoc comments for public APIs
Use tabs for indentation (configured in Biome)
Use double quotes for strings (configured in Biome)
Organize imports automatically (Biome handles this)
Avoid explicit types when TypeScript can infer them (noInferrableTypeserror)
Useas constwhere appropriate for immutable values (useAsConstAssertionerror)
Don't reassign function parameters (noParameterAssignerror)
Place default parameters last in function signatures (useDefaultParameterLasterror)
Always initialize enum values (useEnumInitializerserror)
Declare one variable per statement (useSingleVarDeclaratorerror)
Avoid unnecessary template literals (noUnusedTemplateLiteralerror)
PreferNumber.parseIntover globalparseInt(useNumberNamespaceerror)
Use kebab-case for TypeScript filenames (e.g.,create-env.ts)
Use camelCase for function names (e.g.,createEnv)
Use PascalCase for type names (e.g.,ArkEnvError)
Use UPPER_SNAKE_CASE for environment variables and constants
Include examples in JSDoc comments when helpful for public APIs
Document complex type logic with JSDoc comments
UseArkEnvErrorfor environment variable validation errors
Provide clear, actionable error messages that include the variable name and expected type
**/*.{ts,tsx}: UsecreateEnv(schema)function (or default import asarkenv) to create validated environment objects in TypeScript
Use built-in validators (host, port, url, email) from ArkEnv when available instead of custom ArkType schemas
Provide default values for optional environment variables using ArkType syntax (e.g., 'boolean = false')
Use ArkEnvError for environment variable errors instead of generic Error types
For environment schema definition, use ArkType string literal syntax for enumerated values (e.g., "'deve...
Files:
packages/vite-plugin/src/index.tspackages/vite-plugin/src/index.test.ts
**/index.ts
📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)
Use barrel exports (
index.ts) for package entry points
Files:
packages/vite-plugin/src/index.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)
Co-locate tests with components:
Component.tsxnext toComponent.test.tsx
**/*.test.{ts,tsx}: Use Vitest for unit and integration tests
Test individual functions, components, and hooks in isolation with mocked dependencies in unit tests
Unit tests should focus on individual function logic and edge cases, component rendering and props, error handling and validation, and type checking
Unit tests should execute in less than 100ms per test
Mock external dependencies (clipboard, network, etc.) in unit tests
Co-locate unit test files with source files using naming convention: source file → test file (e.g., create-env.ts → create-env.test.ts)
Test component behavior, not aesthetics, and focus on what users can do and what the component guarantees through its API
Test component public API (props, events, and component contract), user behavior (clicks, typing, focus, keyboard, ARIA), state transitions, accessibility, and side effects in component tests
Do not test pure styling or CSS classes, library internals (Radix/shadcn), implementation details (hooks, setState, private variables), or visual variants in component tests
Use Testing Library with user-event for real user simulation in component tests
Query by role, name, label, and text (accessibility first) in component tests
Use beforeEach/afterEach for cleanup, not beforeAll/afterAll when possible
Keep tests fast, deterministic, and parallelizable
Mock at component boundaries (network, time, context)Create unit tests with
.test.tsor.test.tsxsuffix located alongside source files, testing individual functions and components in isolation with mocked dependencies
Files:
packages/vite-plugin/src/index.test.ts
packages/vite-plugin/src/**/*.test.ts
📄 CodeRabbit inference engine (.cursor/rules/test-patterns.mdc)
packages/vite-plugin/src/**/*.test.ts: Test the Vite plugin using the with-vite-react example as a fixture and validate that the plugin works with real Vite projects
Use fixture-based testing pattern for Vite plugin tests by placing fixtures in fixtures directory
Files:
packages/vite-plugin/src/index.test.ts
**/*.{test,integration.test}.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{test,integration.test}.{ts,tsx}: Use Vitest'sdescribe/itstructure for all test files
Test both success and failure cases in unit and integration tests
Mockprocess.envin unit tests to test different environment variable scenarios
Files:
packages/vite-plugin/src/index.test.ts
🧠 Learnings (18)
📓 Common learnings
Learnt from: danciudev
Repo: yamcodes/arkenv PR: 614
File: packages/vite-plugin/src/index.test.ts:641-654
Timestamp: 2025-12-26T19:27:11.710Z
Learning: In packages/vite-plugin/src/**/*.test.ts: The test suite uses `env.test` files (without leading dot) as test fixtures that are manually read by the `readTestConfig` helper function and stubbed into process.env with `vi.stubEnv`, not as files to be read by Vite's loadEnv during tests.
Learnt from: CR
Repo: yamcodes/arkenv PR: 0
File: .cursor/rules/test-patterns.mdc:0-0
Timestamp: 2025-12-12T13:20:01.954Z
Learning: Applies to packages/vite-plugin/src/**/*.test.ts : Use fixture-based testing pattern for Vite plugin tests by placing fixtures in __fixtures__ directory
Learnt from: CR
Repo: yamcodes/arkenv PR: 0
File: .cursor/rules/test-patterns.mdc:0-0
Timestamp: 2025-12-12T13:20:01.954Z
Learning: Applies to packages/vite-plugin/src/**/*.test.ts : Test the Vite plugin using the with-vite-react example as a fixture and validate that the plugin works with real Vite projects
📚 Learning: 2025-12-12T13:20:01.954Z
Learnt from: CR
Repo: yamcodes/arkenv PR: 0
File: .cursor/rules/test-patterns.mdc:0-0
Timestamp: 2025-12-12T13:20:01.954Z
Learning: Applies to packages/vite-plugin/src/**/*.test.ts : Use fixture-based testing pattern for Vite plugin tests by placing fixtures in __fixtures__ directory
Applied to files:
.changeset/cold-lemons-cut.mdpackages/vite-plugin/src/index.tspackages/vite-plugin/src/index.test.ts
📚 Learning: 2025-12-12T13:20:01.954Z
Learnt from: CR
Repo: yamcodes/arkenv PR: 0
File: .cursor/rules/test-patterns.mdc:0-0
Timestamp: 2025-12-12T13:20:01.954Z
Learning: Applies to packages/vite-plugin/src/**/*.test.ts : Test the Vite plugin using the with-vite-react example as a fixture and validate that the plugin works with real Vite projects
Applied to files:
.changeset/cold-lemons-cut.mdpackages/vite-plugin/src/index.tspackages/vite-plugin/src/index.test.ts
📚 Learning: 2025-12-12T13:20:01.954Z
Learnt from: CR
Repo: yamcodes/arkenv PR: 0
File: .cursor/rules/test-patterns.mdc:0-0
Timestamp: 2025-12-12T13:20:01.954Z
Learning: Applies to **/*.test.{ts,tsx} : Use Vitest for unit and integration tests
Applied to files:
.vscode/tasks.jsonpackages/vite-plugin/src/index.test.ts
📚 Learning: 2025-11-24T16:04:11.901Z
Learnt from: CR
Repo: yamcodes/arkenv PR: 0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-11-24T16:04:11.901Z
Learning: Applies to turbo.json : Turborepo tasks are defined in turbo.json with common tasks: build, dev, typecheck, test, and test:e2e
Applied to files:
.vscode/tasks.json
📚 Learning: 2025-12-23T07:09:57.130Z
Learnt from: CR
Repo: yamcodes/arkenv PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T07:09:57.130Z
Learning: Applies to **/*.test.{ts,tsx} : Create unit tests with `.test.ts` or `.test.tsx` suffix located alongside source files, testing individual functions and components in isolation with mocked dependencies
Applied to files:
.vscode/tasks.jsonpackages/vite-plugin/src/index.test.ts
📚 Learning: 2025-12-12T13:20:01.954Z
Learnt from: CR
Repo: yamcodes/arkenv PR: 0
File: .cursor/rules/test-patterns.mdc:0-0
Timestamp: 2025-12-12T13:20:01.954Z
Learning: Applies to **/*.test.{ts,tsx} : Co-locate unit test files with source files using naming convention: source file → test file (e.g., create-env.ts → create-env.test.ts)
Applied to files:
.vscode/tasks.json
📚 Learning: 2025-12-26T19:27:11.710Z
Learnt from: danciudev
Repo: yamcodes/arkenv PR: 614
File: packages/vite-plugin/src/index.test.ts:641-654
Timestamp: 2025-12-26T19:27:11.710Z
Learning: In packages/vite-plugin/src/**/*.test.ts: The test suite uses `env.test` files (without leading dot) as test fixtures that are manually read by the `readTestConfig` helper function and stubbed into process.env with `vi.stubEnv`, not as files to be read by Vite's loadEnv during tests.
Applied to files:
.vscode/tasks.jsonpackages/vite-plugin/src/index.ts
📚 Learning: 2025-12-23T07:09:57.130Z
Learnt from: CR
Repo: yamcodes/arkenv PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T07:09:57.130Z
Learning: Applies to **/*.{test,integration.test}.{ts,tsx} : Use Vitest's `describe`/`it` structure for all test files
Applied to files:
.vscode/tasks.jsonpackages/vite-plugin/src/index.test.ts
📚 Learning: 2025-12-26T19:27:06.951Z
Learnt from: danciudev
Repo: yamcodes/arkenv PR: 614
File: packages/vite-plugin/src/index.test.ts:641-654
Timestamp: 2025-12-26T19:27:06.951Z
Learning: In packages/vite-plugin/src/index.test.ts, tests that rely on env.test fixtures are read by the readTestConfig helper and injected via vi.stubEnv into process.env, not loaded by Vite's loadEnv during tests. Treat env.test files as fixtures used for environment setup, and ensure test code does not rely on Vite's loadEnv to read them. If adding new tests that use a similar fixture strategy, follow the same pattern: read fixtures through readTestConfig and stub into process.env, rather than permitting Vite to load them automatically.
Applied to files:
packages/vite-plugin/src/index.test.ts
📚 Learning: 2025-12-23T07:09:57.130Z
Learnt from: CR
Repo: yamcodes/arkenv PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T07:09:57.130Z
Learning: Applies to **/*.{test,integration.test}.{ts,tsx} : Mock `process.env` in unit tests to test different environment variable scenarios
Applied to files:
packages/vite-plugin/src/index.test.ts
📚 Learning: 2025-12-12T13:20:01.954Z
Learnt from: CR
Repo: yamcodes/arkenv PR: 0
File: .cursor/rules/test-patterns.mdc:0-0
Timestamp: 2025-12-12T13:20:01.954Z
Learning: Applies to **/*.test.{ts,tsx} : Test individual functions, components, and hooks in isolation with mocked dependencies in unit tests
Applied to files:
packages/vite-plugin/src/index.test.ts
📚 Learning: 2025-12-12T13:20:01.954Z
Learnt from: CR
Repo: yamcodes/arkenv PR: 0
File: .cursor/rules/test-patterns.mdc:0-0
Timestamp: 2025-12-12T13:20:01.954Z
Learning: Applies to **/*.integration.test.{ts,tsx} : Test how multiple units (components, hooks, functions) work together without mocking their interactions in integration tests
Applied to files:
packages/vite-plugin/src/index.test.ts
📚 Learning: 2025-12-12T13:20:01.954Z
Learnt from: CR
Repo: yamcodes/arkenv PR: 0
File: .cursor/rules/test-patterns.mdc:0-0
Timestamp: 2025-12-12T13:20:01.954Z
Learning: Applies to **/*.integration.test.{ts,tsx} : Integration tests should focus on component and hook interactions, function composition and data flow, real dependencies between units, and state synchronization across boundaries
Applied to files:
packages/vite-plugin/src/index.test.ts
📚 Learning: 2025-12-23T07:09:57.130Z
Learnt from: CR
Repo: yamcodes/arkenv PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T07:09:57.130Z
Learning: Applies to **/*.{ts,tsx} : Use `createEnv(schema)` function (or default import as `arkenv`) to create validated environment objects in TypeScript
Applied to files:
packages/vite-plugin/src/index.test.ts
📚 Learning: 2025-11-24T16:03:45.295Z
Learnt from: CR
Repo: yamcodes/arkenv PR: 0
File: .cursor/rules/arktype.mdc:0-0
Timestamp: 2025-11-24T16:03:45.295Z
Learning: Applies to packages/arkenv/**/*.ts : Keep environment variable schemas readable and TypeScript-like using ArkType syntax
Applied to files:
packages/vite-plugin/src/index.test.ts
📚 Learning: 2025-12-12T13:20:01.954Z
Learnt from: CR
Repo: yamcodes/arkenv PR: 0
File: .cursor/rules/test-patterns.mdc:0-0
Timestamp: 2025-12-12T13:20:01.954Z
Learning: Applies to **/*.test.{ts,tsx} : Mock at component boundaries (network, time, context)
Applied to files:
packages/vite-plugin/src/index.test.ts
📚 Learning: 2025-12-12T13:20:01.954Z
Learnt from: CR
Repo: yamcodes/arkenv PR: 0
File: .cursor/rules/test-patterns.mdc:0-0
Timestamp: 2025-12-12T13:20:01.954Z
Learning: Applies to **/*.test.{ts,tsx} : Mock external dependencies (clipboard, network, etc.) in unit tests
Applied to files:
packages/vite-plugin/src/index.test.ts
🧬 Code graph analysis (1)
packages/vite-plugin/src/index.ts (2)
packages/vite-plugin/src/__fixtures__/with-env-dir/index.ts (1)
config(2-5)packages/arkenv/src/create-env.ts (1)
createEnv(65-94)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: test-e2e (a11y)
- GitHub Check: test-e2e (e2e)
- GitHub Check: test-build (latest)
- GitHub Check: test-typesafety
- GitHub Check: test-build (lts/*)
- GitHub Check: Deploy-Preview
🔇 Additional comments (4)
.changeset/cold-lemons-cut.md (1)
1-7: LGTM! Clear documentation of the refactor.The changeset accurately describes the migration to
configResolvedhook and appropriately categorizes this as a patch-level change.packages/vite-plugin/src/index.test.ts (3)
104-139: LGTM! Tests properly updated forconfigResolvedhook.The tests correctly verify the plugin exposes the
configResolvedhook and that it callscreateEnvwith the expected parameters. The mock setup and invocation pattern align with the new hook-based approach.
141-502: Excellent test coverage of the define mutation behavior.The test suite comprehensively covers:
- Environment variable transformation and type coercion
- Edge cases (null, undefined, empty string, zero, false)
- Prefix filtering (default, custom, array of prefixes)
- envDir configuration (custom, default, missing)
- Error propagation
All tests properly verify that
mockConfig.defineis mutated as expected, aligning with the refactored plugin implementation.
505-605: Well-structured integration tests for envDir fixture.The integration tests effectively validate the real-world behavior of custom
envDirconfiguration using thewith-env-dirfixture. The reusablecreateBuildConfigfactory function and clear test scenarios (success, failure, prioritization) demonstrate thorough validation of the feature.Based on learnings, the fixture-based testing pattern is correctly applied here.
| "type": "npm", | ||
| "script": "test", | ||
| "group": "test", | ||
| "problemMatcher": ["$tsc-watch"], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n .vscode/tasks.jsonRepository: yamcodes/arkenv
Length of output: 367
🌐 Web query:
VSCode built-in problem matcher for vitest testing framework
💡 Result:
Short answer: VS Code has no built-in problem matcher specifically for Vitest. Use one of these approaches instead:
- Use the official Vitest VS Code extension / Test Explorer to run and view test results in the editor. [2]
- Configure Vitest to emit a machine-readable report (json or junit) and consume that (e.g. via a task that writes a report file, or a CI parser). Vitest supports json/junit reporters and writing output to a file. [1]
- Create a custom tasks.json problemMatcher (regex-based) that matches Vitest's terminal output if you want failures to appear in the Problems panel. See the VS Code problem matcher docs for pattern/loop/fileLocation details. [3]
Links:
- Vitest reporters (json/junit output): https://vitest.dev/guide/reporters. [1]
- Vitest VS Code extension (Test Explorer): https://github.com/vitest-dev/vscode. [2]
- VS Code tasks / problem matcher documentation (how to author a matcher): https://code.visualstudio.com/docs/editor/tasks (or the Tasks docs). [3]
Use a custom problem matcher or the Vitest VS Code extension for this task.
The $tsc-watch problem matcher is designed for TypeScript compiler output, not vitest, and will not properly capture test failures in the Problems panel. VSCode has no built-in problem matcher for vitest; instead, either:
- Install and use the Vitest VS Code extension to run tests directly in the editor
- Create a custom regex-based problem matcher in this task configuration that captures vitest's output format
🤖 Prompt for AI Agents
.vscode/tasks.json around line 8: the task currently uses the "$tsc-watch"
problemMatcher which is for the TypeScript compiler and will not capture vitest
failures; replace it with either nothing and run tests via the Vitest VS Code
extension, or define and reference a custom problemMatcher that uses regexes
matching vitest output (errors, file:line:col, and test failure summaries) so
test failures appear in the Problems panel; update the task to point to that
custom matcher (or remove the matcher and document using the Vitest extension).
|
I'm closing this. Since we need to mutate the config's |
Closes #670
Summary by CodeRabbit
Refactor
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.