fix(vite): watch resolved inputs and avoid self-trigger loops#362
Merged
fix(vite): watch resolved inputs and avoid self-trigger loops#362
Conversation
The SQL provider was updated to use context.configDir but the mongo provider was missed, causing watcher/loader path divergence when cwd differs from the config directory.
…onfigDir - Switch config-loader from node:path to pathe for consistency with the rest of the tooling layer. - Resolve .ts contract paths inside load() via context.configDir instead of capturing process.cwd() at config-definition time, so the watcher and loader always reference the same file.
The paths branch omitted the config file from the watch set, so edits to prisma-next.config.ts (e.g. changing paths or swapping provider) would not trigger re-emit. Also surface a partial-coverage warning when loadConfig fails instead of silently falling back.
Close D1 from the PR #356 review by driving a real Vite dev server through both scenarios that were previously only unit-mocked: modifying `contract.prisma` re-emits the contract, and editing the config to swap the declared PSL input re-emits against the new file.
Align with the Mongo PSL provider and with the parser step's sourceId. The absolute form stays accessible via meta.absoluteSchemaPath for downstream consumers that need it.
Collapse the ~20-line inline provider that postgres and mongo facades each build around a dynamic import of the user's `contract.ts`. The logic (path resolution, pathToFileURL, default-export extraction) is family-agnostic, so each contract-ts package exposes it as a sibling to typescriptContract.
- handleTrackedFileChange now runs ctx.file through pathe.resolve before comparing against watchedFiles / ignoredOutputFiles, so symlinked or case-folded paths line up with the resolved entries. - resolveContractOutputFiles no longer falls back to the hardcoded 'src/prisma/contract.json' when contract.output is missing. The filter simply skips; defineConfig() guarantees output is present in the happy path, and raw-object configs get a clean no-op instead of a path the CLI never actually writes.
…ceContext Left over from the split that moved configDir out of ContractSourceContext into ContractSourceEnvironment — the context builder no longer needs it.
…rdening Updates all `prisma-next.config.ts` fixtures under test/integration to the new `contract.source.inputs` shape. Adds the vite-plugin PSL fixture used by the HMR e2e test and makes that test wait until the rewritten `contract.json` is readable before parsing. Raises timeout budgets on DB-backed CLI and sql-builder integration tests that were flaky under full- repo load.
Updates ADR 163 to describe provider-invoked source interpretation with the resolvedInputs I/O boundary, and aligns the config / cli / vite-plugin READMEs with the current `load()` signature and the one-call-site path derivation.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughDelegates contract normalization to Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
@prisma-next/mongo-runtime
@prisma-next/family-mongo
@prisma-next/sql-runtime
@prisma-next/family-sql
@prisma-next/middleware-telemetry
@prisma-next/mongo
@prisma-next/extension-paradedb
@prisma-next/extension-pgvector
@prisma-next/postgres
@prisma-next/sql-orm-client
@prisma-next/sqlite
@prisma-next/target-mongo
@prisma-next/adapter-mongo
@prisma-next/driver-mongo
@prisma-next/contract
@prisma-next/utils
@prisma-next/config
@prisma-next/errors
@prisma-next/framework-components
@prisma-next/operations
@prisma-next/ts-render
@prisma-next/contract-authoring
@prisma-next/ids
@prisma-next/psl-parser
@prisma-next/psl-printer
@prisma-next/cli
@prisma-next/emitter
@prisma-next/migration-tools
prisma-next
@prisma-next/vite-plugin-contract-emit
@prisma-next/runtime-executor
@prisma-next/mongo-codec
@prisma-next/mongo-contract
@prisma-next/mongo-value
@prisma-next/mongo-contract-psl
@prisma-next/mongo-contract-ts
@prisma-next/mongo-emitter
@prisma-next/mongo-schema-ir
@prisma-next/mongo-query-ast
@prisma-next/mongo-orm
@prisma-next/mongo-query-builder
@prisma-next/mongo-lowering
@prisma-next/mongo-wire
@prisma-next/sql-contract
@prisma-next/sql-errors
@prisma-next/sql-operations
@prisma-next/sql-schema-ir
@prisma-next/sql-contract-psl
@prisma-next/sql-contract-ts
@prisma-next/sql-contract-emitter
@prisma-next/sql-lane-query-builder
@prisma-next/sql-relational-core
@prisma-next/sql-builder
@prisma-next/target-postgres
@prisma-next/target-sqlite
@prisma-next/adapter-postgres
@prisma-next/adapter-sqlite
@prisma-next/driver-postgres
@prisma-next/driver-sqlite
commit: |
jkomyno
changed the title
…ma/prisma-next into tml-2276-authoritative-watch-inputs
…uts' into tml-2277-vite-plugin-watch-behavior
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (6)
test/integration/test/sql-builder/extension-functions.test.ts (1)
4-29: Optional: apply the suite timeout to theilikesuite too for consistency.Only the
extension functionssuite got{ timeout: timeouts.databaseOperation }. Theilikesuite above still relies on Vitest's default. If the intent is uniform suite-level timeouts across integration tests (as done in sibling files), consider applying it here as well.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/integration/test/sql-builder/extension-functions.test.ts` around lines 4 - 29, The "integration: ilike (adapter operation)" test suite currently lacks the extended timeout used elsewhere; update the describe call that defines this suite (describe('integration: ilike (adapter operation)')) to include the same suite option object used in the other suite, e.g. add { timeout: timeouts.databaseOperation } so the ilike tests use the longer databaseOperation timeout during execution.packages/1-framework/3-tooling/cli/test/control-api/contract-emit.test.ts (1)
72-87: Rename the stale negative-test description.The assertion now verifies a valid source provider object, but the test still says “not callable,” which describes the old function API.
Suggested rename
- it('throws when contract source is not callable', async () => { + it('throws when contract source is not a valid provider object', async () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/1-framework/3-tooling/cli/test/control-api/contract-emit.test.ts` around lines 72 - 87, The test description "throws when contract source is not callable" is outdated; update the it(...) title to reflect the current assertion that the source must be a valid source provider object. Change the test name in the it call (surrounding the assertion that uses mockConfigWithContract and executeContractEmit) to something like "throws when contract source is not a valid source provider object" so it matches the expectation checking error.why includes 'valid source provider object'.test/integration/test/cli.emit-command.test.ts (1)
628-658: Consider moving this new coverage into a split emit-command test file.This file is already 865 lines, so adding the default-output case here makes the oversized test harder to navigate. A focused split such as
cli.emit-command.defaults.test.tswould keep this behavior covered while following the test-size rule.As per coding guidelines,
**/*.test.ts: “Keep test files under 500 lines to maintain readability and navigability. If a test file exceeds this limit, it should be split into multiple files.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/integration/test/cli.emit-command.test.ts` around lines 628 - 658, The new "uses default output path for plain-object configs without defineConfig" test should be moved out of the large cli.emit-command.test.ts into a dedicated smaller test file (suggested name: cli.emit-command.defaults.test.ts) to keep files under 500 lines; locate the spec by its it(...) block and the helper usage (createContractEmitCommand, setupIntegrationTestDirectoryFromFixtures, executeCommand) and copy that entire test case (including its timeout, process.chdir handling, assertions on src/prisma/contract.json and contract.d.ts) into the new file, update any imports/fixture references to match the new file context, and remove the original block from cli.emit-command.test.ts so both suites run but the large file stays below the size guideline.packages/1-framework/3-tooling/cli/test/control-api/client.test.ts (1)
1-18: UseifDefined()for the optionalinputsproperty.This keeps the new helper aligned with the repo’s conditional-spread convention.
♻️ Proposed refactor
import type { ContractSourceProvider } from '@prisma-next/config/config-types'; import { createContract } from '@prisma-next/contract/testing'; import type { Contract } from '@prisma-next/contract/types'; import type { EmitResult } from '@prisma-next/emitter'; @@ import type { EmissionSpi } from '@prisma-next/framework-components/emission'; +import { ifDefined } from '@prisma-next/utils/defined'; import { notOk, ok } from '@prisma-next/utils/result'; import { describe, expect, it, vi } from 'vitest'; @@ ): ContractSourceProvider { return { - ...(inputs ? { inputs } : {}), + ...ifDefined('inputs', inputs), load, }; }As per coding guidelines, use
ifDefined()from@prisma-next/utils/definedfor conditional object spreads instead of inline conditional spread patterns.Also applies to: 37-44
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/1-framework/3-tooling/cli/test/control-api/client.test.ts` around lines 1 - 18, Replace inline conditional object-spread for the optional inputs property with the repository helper ifDefined from '@prisma-next/utils/defined': import ifDefined and, where the test builds the object containing inputs (the object passed to createContract / the helper that currently does ... (inputs ? { inputs } : {})), change that spread to ...ifDefined(inputs) so the conditional-spread follows the repo convention; update all occurrences mentioned (including lines around the createContract usage where inputs is optional).packages/1-framework/1-core/config/src/config-types.ts (1)
89-92: UseSchema.array()for this Arktype array field.The provider schema currently uses the string DSL array form for
inputs; switch to a string schema plus.array()to match the repo’s Arktype conventions.Proposed refactor
+const ContractSourceInputSchema = type('string'); + export const ContractSourceProviderSchema = type({ - 'inputs?': 'string[]', + 'inputs?': ContractSourceInputSchema.array(), load: 'Function', });As per coding guidelines,
Use 'Schema.array()' for array definitions in Arktype schemas (avoid tuple mistakes).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/1-framework/1-core/config/src/config-types.ts` around lines 89 - 92, ContractSourceProviderSchema currently uses the DSL string array form ('inputs?': 'string[]') which can be a tuple pitfall; update the schema to use the Arktype Schema API by replacing that field with a string schema plus .array() and mark it optional (e.g., use Schema.string().array().optional()). Modify the ContractSourceProviderSchema definition (where 'inputs?' and load are declared) to use Schema.string().array().optional() for inputs and keep load as the existing Function type, and ensure the Schema symbol is imported if not already.packages/1-framework/3-tooling/vite-plugin-contract-emit/src/plugin.ts (1)
378-414: Redundant config load on server startup.
configureServercallsresolveWatchedFiles(viteServer)directly at Line 378 and thenawait emitContract()at Line 414, which internally callsupdateWatchedFiles(viteServer)→resolveWatchedFiles(viteServer)again. That meansloadConfig(absoluteConfigPath)andssrLoadModule(...)for every module-graph root run twice on cold start. Functionally correct (the diff betweenwatchedFilesandnewWatchedFilesis empty on the second pass, so no duplicatewatcher.add/unwatch), but wasteful.Consider either passing a flag to
emitContractto skip the inner refresh on the initial call, or removing the explicit call at Line 378 (moving the “watchedFiles is empty → overlay error” check to run after the firstemitContract).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/1-framework/1-core/config/src/config-validation.ts`:
- Around line 21-48: The current validation reads prototype-inherited properties
(e.g., source.load, source.inputs, contract.output); change the checks to
require own properties by using Object.prototype.hasOwnProperty.call(...) before
validating each nested field: for inputs use hasOwnProperty on sourceConfig with
key 'inputs' before checking Array.isArray and element types, for load use
hasOwnProperty on sourceConfig with key 'load' before verifying typeof ===
'function', and for output use hasOwnProperty on contract with key 'output'
before validating it is a string; keep using throwValidation (e.g.,
throwValidation('contract.source.inputs', ...),
throwValidation('contract.source.load', ...), throwValidation('contract.output',
...)) for failures.
In `@packages/1-framework/3-tooling/cli/test/commands/migration-e2e.test.ts`:
- Around line 98-190: The test creates a temp dir via createTempDir() and never
removes it; wrap the main test body (the code that creates root, migrationsDir,
writes migrations, attests and verifies) inside a try/finally so that in the
finally block you call rm(root, { recursive: true, force: true }) to clean up;
locate usage of createTempDir, migrationsDir, writeMigrationPackage,
attestMigration, readMigrationsDir, verifyMigration and ensure the finally
always executes even if assertions fail.
In
`@packages/1-framework/3-tooling/vite-plugin-contract-emit/test/plugin.test.ts`:
- Around line 415-417: The test contains a tautological negative assertion using
consoleErrorSpy.not.toHaveBeenCalledWith(expect.stringContaining('Failed to
resolve watched files:')) which will always pass because that string never
appears in plugin.ts; replace this with a meaningful check—either remove the
assertion entirely or change it to assert that consoleErrorSpy was not called
with a real fallback/emit error string such as 'Contract emit failed'
(referencing consoleErrorSpy in plugin.test.ts and the actual error messages in
plugin.ts like 'Failed to load config module graph root:' / 'Failed to collect
watched files:' when making the replacement).
In `@packages/2-mongo-family/2-authoring/contract-ts/src/config-types.ts`:
- Around line 22-32: The loader currently imports the module in the load
function and sets const contract: Contract = mod.default ?? mod.contract, which
can silently become undefined; change this to explicitly validate that either
mod.default or mod.contract exists and throw a descriptive Error if neither is
present (e.g., "typescriptContractFromPath: module must export default or named
'contract'"). Update the block around the import in load to check the presence
of mod.default and mod.contract before returning ok(contract) and only call
ok(contract) with a properly-typed Contract.
In `@packages/2-sql/2-authoring/contract-ts/src/config-types.ts`:
- Around line 20-29: The loader in load: async (context) currently accepts a
module that may export neither default nor contract, assigning undefined to
const contract and returning ok(contract); change this to validate that
mod.default or mod.contract is present: after const mod = await import(...),
compute const contract = mod.default ?? mod.contract and if contract is
undefined throw a clear Error (including the absolutePath and a message saying
the module must export either default or named export "contract") instead of
returning ok(undefined); only return ok(contract) once the contract value is
confirmed.
In `@test/integration/test/authoring/parity/callback-mode-scalars/contract.ts`:
- Around line 15-22: The contract uses defaultSql('autoincrement()') and
defaultSql('now()') in fields (see uses of defaultSql on id and createdAt), but
the defineContract call is missing matching capabilities; update the
defineContract options to include 'defaults.autoincrement': true and
'defaults.now': true under capabilities so the emitter knows to include full
default payloads, then re-run the emitter and verify the emitted JSON contains
the complete default payloads for those fields.
---
Nitpick comments:
In `@packages/1-framework/1-core/config/src/config-types.ts`:
- Around line 89-92: ContractSourceProviderSchema currently uses the DSL string
array form ('inputs?': 'string[]') which can be a tuple pitfall; update the
schema to use the Arktype Schema API by replacing that field with a string
schema plus .array() and mark it optional (e.g., use
Schema.string().array().optional()). Modify the ContractSourceProviderSchema
definition (where 'inputs?' and load are declared) to use
Schema.string().array().optional() for inputs and keep load as the existing
Function type, and ensure the Schema symbol is imported if not already.
In `@packages/1-framework/3-tooling/cli/test/control-api/client.test.ts`:
- Around line 1-18: Replace inline conditional object-spread for the optional
inputs property with the repository helper ifDefined from
'@prisma-next/utils/defined': import ifDefined and, where the test builds the
object containing inputs (the object passed to createContract / the helper that
currently does ... (inputs ? { inputs } : {})), change that spread to
...ifDefined(inputs) so the conditional-spread follows the repo convention;
update all occurrences mentioned (including lines around the createContract
usage where inputs is optional).
In `@packages/1-framework/3-tooling/cli/test/control-api/contract-emit.test.ts`:
- Around line 72-87: The test description "throws when contract source is not
callable" is outdated; update the it(...) title to reflect the current assertion
that the source must be a valid source provider object. Change the test name in
the it call (surrounding the assertion that uses mockConfigWithContract and
executeContractEmit) to something like "throws when contract source is not a
valid source provider object" so it matches the expectation checking error.why
includes 'valid source provider object'.
In `@test/integration/test/cli.emit-command.test.ts`:
- Around line 628-658: The new "uses default output path for plain-object
configs without defineConfig" test should be moved out of the large
cli.emit-command.test.ts into a dedicated smaller test file (suggested name:
cli.emit-command.defaults.test.ts) to keep files under 500 lines; locate the
spec by its it(...) block and the helper usage (createContractEmitCommand,
setupIntegrationTestDirectoryFromFixtures, executeCommand) and copy that entire
test case (including its timeout, process.chdir handling, assertions on
src/prisma/contract.json and contract.d.ts) into the new file, update any
imports/fixture references to match the new file context, and remove the
original block from cli.emit-command.test.ts so both suites run but the large
file stays below the size guideline.
In `@test/integration/test/sql-builder/extension-functions.test.ts`:
- Around line 4-29: The "integration: ilike (adapter operation)" test suite
currently lacks the extended timeout used elsewhere; update the describe call
that defines this suite (describe('integration: ilike (adapter operation)')) to
include the same suite option object used in the other suite, e.g. add {
timeout: timeouts.databaseOperation } so the ilike tests use the longer
databaseOperation timeout during execution.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 2887f2b6-4ca7-4522-bf8a-0b091c565bc5
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (97)
docs/architecture docs/adrs/ADR 163 - Provider-invoked source interpretation packages.mdpackages/1-framework/1-core/config/README.mdpackages/1-framework/1-core/config/src/config-types.tspackages/1-framework/1-core/config/src/config-validation.tspackages/1-framework/1-core/config/src/contract-source-types.tspackages/1-framework/1-core/config/test/config-types.test-d.tspackages/1-framework/1-core/config/test/config-validation.test.tspackages/1-framework/3-tooling/cli/README.mdpackages/1-framework/3-tooling/cli/recordings/fixtures/prisma-next.config.tspackages/1-framework/3-tooling/cli/src/commands/contract-emit.tspackages/1-framework/3-tooling/cli/src/config-loader.tspackages/1-framework/3-tooling/cli/src/config-path-validation.tspackages/1-framework/3-tooling/cli/src/control-api/client.tspackages/1-framework/3-tooling/cli/src/control-api/operations/contract-emit.tspackages/1-framework/3-tooling/cli/src/control-api/types.tspackages/1-framework/3-tooling/cli/test/commands/migration-e2e.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-plan.test.tspackages/1-framework/3-tooling/cli/test/config-loader.test.tspackages/1-framework/3-tooling/cli/test/config-types.test.tspackages/1-framework/3-tooling/cli/test/control-api/client.test.tspackages/1-framework/3-tooling/cli/test/control-api/contract-emit.test.tspackages/1-framework/3-tooling/emitter/src/artifact-paths.tspackages/1-framework/3-tooling/emitter/src/emit-types.tspackages/1-framework/3-tooling/emitter/src/emit.tspackages/1-framework/3-tooling/emitter/src/exports/index.tspackages/1-framework/3-tooling/emitter/test/emitter.test.tspackages/1-framework/3-tooling/vite-plugin-contract-emit/README.mdpackages/1-framework/3-tooling/vite-plugin-contract-emit/package.jsonpackages/1-framework/3-tooling/vite-plugin-contract-emit/src/plugin.tspackages/1-framework/3-tooling/vite-plugin-contract-emit/test/plugin.test.tspackages/2-mongo-family/2-authoring/contract-psl/src/provider.tspackages/2-mongo-family/2-authoring/contract-psl/test/provider.test.tspackages/2-mongo-family/2-authoring/contract-ts/package.jsonpackages/2-mongo-family/2-authoring/contract-ts/src/config-types.tspackages/2-mongo-family/2-authoring/contract-ts/src/exports/config-types.tspackages/2-mongo-family/2-authoring/contract-ts/test/config-types.test.tspackages/2-sql/2-authoring/contract-psl/src/provider.tspackages/2-sql/2-authoring/contract-psl/test/fixtures.tspackages/2-sql/2-authoring/contract-psl/test/provider.test.tspackages/2-sql/2-authoring/contract-ts/package.jsonpackages/2-sql/2-authoring/contract-ts/src/config-types.tspackages/2-sql/2-authoring/contract-ts/src/exports/config-types.tspackages/2-sql/2-authoring/contract-ts/test/config-types.test.tspackages/3-extensions/mongo/src/config/define-config.tspackages/3-extensions/mongo/test/config/define-config.test.tspackages/3-extensions/postgres/package.jsonpackages/3-extensions/postgres/src/config/define-config.tspackages/3-extensions/postgres/test/config/define-config.test.tspackages/3-extensions/sql-orm-client/test/fixtures/prisma-next.config.tspackages/3-targets/6-adapters/postgres/test/codec-render-output-type.test.tstest/integration/test/authoring/cli.emit-parity-fixtures.test.tstest/integration/test/authoring/parity/callback-mode-scalars/contract.tstest/integration/test/authoring/psl.pgvector-dbinit.test.tstest/integration/test/authoring/side-by-side-contracts.test.tstest/integration/test/cli.emit-command.test.tstest/integration/test/cli.emit-contract.test.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/cli-journeys/prisma-next.config.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/cli-journeys/prisma-next.config.with-db.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-init/prisma-next.config.with-db.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-introspect/prisma-next.config.no-db.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-introspect/prisma-next.config.no-driver.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-introspect/prisma-next.config.with-db.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-sign/prisma-next.config.with-db.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-update-scenarios/prisma-next.config.with-db.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-verify/prisma-next.config.no-driver.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-verify/prisma-next.config.no-verify.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-verify/prisma-next.config.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-verify/prisma-next.config.with-db.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/emit/prisma-next.config.emit.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/migration-apply/prisma-next.config.with-db.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/migration-plan/prisma-next.config.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/mongo-cli-journeys/prisma-next.config.with-db.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/mongo-db-commands/prisma-next.config.with-db.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/vite-plugin-psl/contract-alt.prismatest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/vite-plugin-psl/contract.prismatest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/vite-plugin-psl/prisma-next.config.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/vite-plugin-psl/vite.config.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/vite-plugin/prisma-next.config.tstest/integration/test/fixtures/cli/cli-integration-test-app/fixtures/emit-command/prisma-next.config.async-source.tstest/integration/test/fixtures/cli/cli-integration-test-app/fixtures/emit-command/prisma-next.config.missing-output.tstest/integration/test/fixtures/cli/cli-integration-test-app/fixtures/emit-command/prisma-next.config.mongo-contract-ts.tstest/integration/test/fixtures/prisma-next.config.tstest/integration/test/mongo/fixtures/prisma-next.config.tstest/integration/test/sql-builder/distinct.test.tstest/integration/test/sql-builder/execution.test.tstest/integration/test/sql-builder/extension-functions.test.tstest/integration/test/sql-builder/fixtures/prisma-next.config.tstest/integration/test/sql-builder/group-by.test.tstest/integration/test/sql-builder/join.test.tstest/integration/test/sql-builder/mutation-defaults.test.tstest/integration/test/sql-builder/mutation.test.tstest/integration/test/sql-builder/order-by.test.tstest/integration/test/sql-builder/pagination.test.tstest/integration/test/sql-builder/select.test.tstest/integration/test/sql-builder/subquery.test.tstest/integration/test/sql-builder/where.test.tstest/integration/test/vite-plugin.hmr.e2e.test.ts
…watch-behavior # Conflicts: # packages/1-framework/3-tooling/cli/src/config-path-validation.ts # packages/1-framework/3-tooling/cli/test/config-loader.test.ts # packages/1-framework/3-tooling/vite-plugin-contract-emit/README.md # packages/1-framework/3-tooling/vite-plugin-contract-emit/src/plugin.ts # packages/1-framework/3-tooling/vite-plugin-contract-emit/test/plugin.test.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/1-framework/3-tooling/vite-plugin-contract-emit/test/plugin.test.ts (1)
354-367: Assert the startup ordering, not just the load count.This test name says
loadConfighappens before the initial emit, but the assertion only checks it was called once. Add an invocation-order check so the test covers the contract it documents.Suggested assertion tightening
await configureServer(mockServer); expect(mockedLoadConfig).toHaveBeenCalledTimes(1); + expect(mockedExecuteContractEmit).toHaveBeenCalledTimes(1); + expect(mockedLoadConfig.mock.invocationCallOrder[0]!).toBeLessThan( + mockedExecuteContractEmit.mock.invocationCallOrder[0]!, + ); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/1-framework/3-tooling/vite-plugin-contract-emit/test/plugin.test.ts` around lines 354 - 367, The test currently only asserts mockedLoadConfig was called once but must verify it was called before the plugin's initial emit; after invoking configResolved and awaiting plugin.configureServer(mockServer) add an invocation-order assertion comparing mockedLoadConfig's call order to the initial emit on the mock server (e.g., compare mockedLoadConfig.mock.invocationCallOrder[0] < mockServer.ws.send.mock.invocationCallOrder[0] or use a Jest call-order matcher) so that mockedLoadConfig is confirmed to run before the initial emit triggered by configureServer.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/1-framework/3-tooling/vite-plugin-contract-emit/src/plugin.ts`:
- Around line 287-296: The warning text is misleading when
previousWatchedFiles.size > 0 because the code continues watching the prior
dependency set but the message says "Watching only {absoluteConfigPath}"; update
the log emitted in the block that checks didWarnConfigWatchFallback (and that
may follow the branch where previousWatchedFiles is used) to conditionally
include the preserved watch state: use didWarnConfigWatchFallback and
previousWatchedFiles to choose a message that either says "Watching only
{absoluteConfigPath}" (when previousWatchedFiles is empty) or "Watching previous
dependency set plus {absoluteConfigPath}; contract watch coverage is partial"
(when previousWatchedFiles.size > 0), and keep using logWarning and the existing
reason variable to retain error.message details.
In
`@packages/1-framework/3-tooling/vite-plugin-contract-emit/test/plugin.test.ts`:
- Around line 407-432: The test currently rejects mockedLoadConfig but leaves
executeContractEmit using the default resolved mock from beforeEach, so the
assertion that console.error was not called for "Contract emit failed" doesn't
exercise the emit fallback; update the test to explicitly set
executeContractEmit to the intended behavior for the fallback path (e.g.,
mockedExecuteContractEmit.mockRejectedValue(new Error('emit failed')) or
.mockResolvedValue(...) depending on desired outcome) before invoking
plugin.configureServer, or remove the console.error assertion; reference
mockedLoadConfig and mockedExecuteContractEmit (or executeContractEmit mock set
up in beforeEach) and adjust the test case "falls back to watching the config
file and warns when loadConfig fails" accordingly.
---
Nitpick comments:
In
`@packages/1-framework/3-tooling/vite-plugin-contract-emit/test/plugin.test.ts`:
- Around line 354-367: The test currently only asserts mockedLoadConfig was
called once but must verify it was called before the plugin's initial emit;
after invoking configResolved and awaiting plugin.configureServer(mockServer)
add an invocation-order assertion comparing mockedLoadConfig's call order to the
initial emit on the mock server (e.g., compare
mockedLoadConfig.mock.invocationCallOrder[0] <
mockServer.ws.send.mock.invocationCallOrder[0] or use a Jest call-order matcher)
so that mockedLoadConfig is confirmed to run before the initial emit triggered
by configureServer.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: ed8ca4bf-826f-4642-8bc8-b5d3e3402e2a
📒 Files selected for processing (15)
packages/1-framework/1-core/config/src/config-types.tspackages/1-framework/1-core/config/src/config-validation.tspackages/1-framework/1-core/config/test/config-validation.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-e2e.test.tspackages/1-framework/3-tooling/cli/test/control-api/client.test.tspackages/1-framework/3-tooling/cli/test/control-api/contract-emit.test.tspackages/1-framework/3-tooling/vite-plugin-contract-emit/src/plugin.tspackages/1-framework/3-tooling/vite-plugin-contract-emit/test/plugin.test.tspackages/2-mongo-family/2-authoring/contract-ts/src/config-types.tspackages/2-mongo-family/2-authoring/contract-ts/test/config-types.test.tspackages/2-sql/2-authoring/contract-ts/src/config-types.tspackages/2-sql/2-authoring/contract-ts/test/config-types.test.tstest/integration/test/cli.emit-command.additional.test.tstest/integration/test/cli.emit-command.test.tstest/integration/test/sql-builder/extension-functions.test.ts
✅ Files skipped from review due to trivial changes (3)
- packages/1-framework/1-core/config/test/config-validation.test.ts
- packages/1-framework/1-core/config/src/config-types.ts
- test/integration/test/cli.emit-command.additional.test.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- test/integration/test/sql-builder/extension-functions.test.ts
- packages/2-mongo-family/2-authoring/contract-ts/test/config-types.test.ts
- packages/1-framework/3-tooling/cli/test/control-api/contract-emit.test.ts
- packages/1-framework/3-tooling/cli/test/control-api/client.test.ts
- packages/2-sql/2-authoring/contract-ts/src/config-types.ts
- packages/1-framework/1-core/config/src/config-validation.ts
- test/integration/test/cli.emit-command.test.ts
- packages/1-framework/3-tooling/cli/test/commands/migration-e2e.test.ts
jkomyno
force-pushed
the
tml-2277-vite-plugin-watch-behavior
branch
from
f3292fb to
e74d02e
Compare
SevInf
reviewed
Apr 22, 2026
SevInf
reviewed
Apr 22, 2026
SevInf
approved these changes
Apr 22, 2026
Contributor
SevInf
left a comment
SevInf
left a comment
There was a problem hiding this comment.
Tentatively approving, with one minor issue:
Default output handling is now duplicated between defineConfig and finalizeConfig, i think we should consolidate that.
jkomyno
added a commit
that referenced
this pull request
Apr 22, 2026
## Summary - add a PSL-first Vite plugin HMR integration test that exercises startup emit, bad edit, and recovery - assert invalid schema edits leave the last good contract.json and contract.d.ts on disk - assert the next valid edit re-emits both artifacts ## Testing - pnpm --filter @prisma-next/integration-tests exec vitest run test/vite-plugin.hmr.e2e.test.ts - pnpm --filter @prisma-next/integration-tests typecheck - pnpm exec biome check test/integration/test/vite-plugin.hmr.e2e.test.ts ## Notes - Stacked on #362 - Refs TML-2293 --------- Co-authored-by: jkomyno <12381818+jkomyno@users.noreply.github.com>
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
2 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.
Summary
contract.outputin the CLI loader for plain-object configs so the Vite plugin always receives resolved absolute pathsTesting
Notes
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
Tests