feat(contract-ts): [DRAFT] design new contract.ts#261
Open
Conversation
📝 WalkthroughWalkthroughThis change introduces a new "refined object-literal" contract authoring style for SQL-based Prisma contracts alongside the existing fluent builder pattern. It adds comprehensive DSL machinery including field, model, and rel helpers with extensive type inference support, updates contract type definitions with new capability flags and metadata, and includes refactored examples and integration tests demonstrating the new authoring surface. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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/runtime-executor
@prisma-next/sql-runtime
@prisma-next/extension-paradedb
@prisma-next/extension-pgvector
@prisma-next/postgres
@prisma-next/sql-orm-client
@prisma-next/contract-authoring
@prisma-next/contract-ts
@prisma-next/ids
@prisma-next/psl-parser
@prisma-next/cli
@prisma-next/emitter
@prisma-next/eslint-plugin
@prisma-next/migration-tools
@prisma-next/vite-plugin-contract-emit
@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/family-sql
@prisma-next/sql-kysely-lane
@prisma-next/sql-lane-query-builder
@prisma-next/sql-relational-core
@prisma-next/sql-lane-sql-builder-new
@prisma-next/sql-lane
@prisma-next/target-postgres
@prisma-next/adapter-postgres
@prisma-next/driver-postgres
@prisma-next/core-control-plane
@prisma-next/core-execution-plane
@prisma-next/config
@prisma-next/contract
@prisma-next/operations
@prisma-next/plan
@prisma-next/utils
commit: |
jkomyno
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
packages/2-sql/2-authoring/contract-ts/README.md (1)
18-20: Keep the README focused on the supported API, not rollout language.Calling one surface “legacy” here and the refined DSL the “long-term direction” / “first slice” below makes a durable package README read like migration notes. Please describe the currently supported surfaces directly and move roadmap phrasing out of this file.
As per coding guidelines,
packages/**/*.md: Update design docs to reflect the current state only, not the transition history or legacy approaches.Also applies to: 248-249
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/2-authoring/contract-ts/README.md` around lines 18 - 20, The README currently labels one API as “legacy” and frames the other as a “long-term direction”; update the document to describe only the currently supported surfaces (e.g., the two forms of the SQL contract builder: defineContract and the object-literal DSL using model('User', { fields, relations }).attributes(...).sql(...)) without rollout/migration language, remove or relocate any roadmap/transition phrasing (including the text around lines referencing the same issue such as the content at locations 248-249) to a separate design/roadmap doc, and ensure the README concisely documents usage and examples for the supported APIs only.test/integration/test/contract-builder.types.test-d.ts (1)
106-190: Keep this.test-d.tscase type-only instead of building the contract at runtime.This block is asserting refined token/ref preservation through
defineContract(...), which means the test is going through the builder’s widened runtime return type instead of a stable type-only fixture. That can hide exactly the regression this file is supposed to catch. Please move this scenario to a.test.tsfile or rewrite it around a type-only fixture/emitted contract type.Based on learnings, in Prisma-next, for type-level tests (files ending in
.test-d.ts), avoid using runtime builders likedefineContractsince they can widen types during type-level evaluation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/integration/test/contract-builder.types.test-d.ts` around lines 106 - 190, This test file is type-only (.test-d.ts) but uses runtime builders (defineContract, validateContract, createTestContext, createStubAdapter, schema, sql) which widens types and defeats the purpose of the test; replace the runtime contract construction with a purely type-level fixture (e.g., declare a type/const type-only shape that mirrors the intended contract and use that type in ResultType/type assertions) or move the scenario into a runtime test (.test.ts) if you need actual builder behavior—in short, stop calling defineContract/validateContract/createTestContext in this .test-d.ts and instead reference a static emitted contract type (or move the code to a .test.ts) so the refined token/ref preservation assertions remain type-only.test/integration/test/contract-builder.test.ts (1)
276-340: Split the refined-surface coverage into its own test file.
test/integration/test/contract-builder.test.tsis already well past the 500-line cap, and this adds another distinct concern to an oversized integration file. Moving this case into something likecontract-builder.refined.test.tswould keep failures easier to localize and align with the repo’s test-file limits.As per coding guidelines,
**/*.test.ts: Keep test files under 500 lines to maintain readability and navigability. Split test files when they exceed 500 lines or contain multiple distinct concerns that can be logically separated.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/integration/test/contract-builder.test.ts` around lines 276 - 340, Extract the "refined object contract works with schema() and sql()" it-block into a new test file (e.g., contract-builder.refined.test.ts): copy the entire it(...) including setup of User, Post, contract (defineContract), adapter/context creation (createStubAdapter, createTestContext), and assertions using schema and sql; update imports at the top of the new file to include model, field, rel, int4Column, textColumn, timestamptzColumn, postgresPack, defineContract, createStubAdapter, createTestContext, schema, sql, SelectAst, ResultType, and expectTypeOf; remove the copied it-block from contract-builder.test.ts so the original file stays under 500 lines; run tests to ensure the new file is detected by the test runner and that no shared test fixtures are left behind.packages/2-sql/2-authoring/contract-ts/src/refined-option-a.ts (1)
372-462: UseifDefined()for these conditional properties.This helper block repeats the inline
...(cond ? { ... } : {})pattern forname,using,config, and FK options. The repo already standardizes onifDefined()for this case, so it would be good to switch these helpers over while the DSL surface is still settling. As per coding guidelines, "UseifDefined()from@prisma-next/utils/definedfor conditional object spreads instead of inline conditional spread patterns."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/2-sql/2-authoring/contract-ts/src/refined-option-a.ts` around lines 372 - 462, Replace the inline conditional spread patterns in the constraint helper factories with the ifDefined() helper: in id, unique, index and foreignKey (functions named id, unique, index, foreignKey) remove occurrences of ...(options?.name ? { name: options.name } : {}) and the other inline spreads for using, config, onDelete, onUpdate, constraint, index and instead use ifDefined(...) from `@prisma-next/utils/defined` to conditionally add those properties (e.g., pass the option value and a mapper to produce the property object); keep normalizeFieldRefInput and normalizeTargetFieldRefInput usage intact and only change the conditional spreads to ifDefined calls.
🤖 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/2-sql/2-authoring/contract-ts/src/contract-builder.ts`:
- Around line 1186-1192: The code calls resolveModelIdConstraint(...) then
unconditionally passes the fields to next.primaryKey(...), which can produce
nullable PK columns if any participating model fields are optional; update the
logic in contract-builder.ts to validate the resolved idConstraint.fields
against the model field metadata (using spec.fields or whatever field
descriptors are available on spec) before calling primaryKey: detect any
optional/nullable fields referenced by idConstraint.fields and either (a) throw
or emit a clear error mentioning spec.modelName and the offending field names,
or (b) strip/convert them according to project rules so that primaryKey is only
invoked with non-nullable fields; ensure you reference resolveModelIdConstraint,
mapFieldNamesToColumnNames, primaryKey, spec.modelName and spec.fieldToColumn
when implementing the check so the fix is applied at the correct spot.
- Around line 1122-1139: The code builds tableName via applyNaming and
fieldToColumn for each model but never rejects duplicate storage names; add
checks before modelSpecs.set to validate (1) tableName is unique across
already-processed models in modelSpecs (reject with an Error referencing
modelName and the colliding tableName), and (2) the computed fieldToColumn
values are unique within this model (detect duplicate column names produced by
modelDefinition.stageOne.fields and throw an Error naming the model and
duplicate column). Use the existing symbols (applyNaming, tableName,
fieldToColumn, modelDefinition.stageOne.fields, modelSpecs) to locate the logic
and return/throw on collision so later lowering cannot silently overwrite
storage names.
In `@packages/2-sql/2-authoring/contract-ts/src/refined-option-a.ts`:
- Around line 981-983: The current isRefinedContractInput guard only checks for
a 'target' key; tighten it by verifying that value is an object with a non-null
object "target" and that target has the expected pack-ref shape (e.g.,
properties "kind", "familyId", and "targetId" exist and are strings); update
isRefinedContractInput to return true only when these checks pass so malformed
inputs like { target: 'postgres' } or incomplete contracts are rejected
immediately before dispatching.
In
`@packages/2-sql/2-authoring/contract-ts/test/contract-builder.refined-option-a.test.ts`:
- Around line 379-388: The test currently wraps ts-expect-error checks inside an
if (false) block which triggers lint errors; remove that constant-condition
block and move the failing assertions into a non-executed TypeScript-only
context: either place the three offending calls (rel.belongsTo(User, { from:
'userId', to: 'posts' }); rel.hasMany(Post, { by: 'posts' }); and the two valid
calls for context if needed) inside an uncalled helper function (e.g., function
_tsExpectErrors() { /* keep `@ts-expect-error` lines here */ } ) or move them into
a companion .test-d.ts file so the `@ts-expect-error` comments are checked by TS
without producing runtime/lintrule failures; target the calls to rel.belongsTo
and rel.hasMany and the types User and Post when relocating the lines.
---
Nitpick comments:
In `@packages/2-sql/2-authoring/contract-ts/README.md`:
- Around line 18-20: The README currently labels one API as “legacy” and frames
the other as a “long-term direction”; update the document to describe only the
currently supported surfaces (e.g., the two forms of the SQL contract builder:
defineContract and the object-literal DSL using model('User', { fields,
relations }).attributes(...).sql(...)) without rollout/migration language,
remove or relocate any roadmap/transition phrasing (including the text around
lines referencing the same issue such as the content at locations 248-249) to a
separate design/roadmap doc, and ensure the README concisely documents usage and
examples for the supported APIs only.
In `@packages/2-sql/2-authoring/contract-ts/src/refined-option-a.ts`:
- Around line 372-462: Replace the inline conditional spread patterns in the
constraint helper factories with the ifDefined() helper: in id, unique, index
and foreignKey (functions named id, unique, index, foreignKey) remove
occurrences of ...(options?.name ? { name: options.name } : {}) and the other
inline spreads for using, config, onDelete, onUpdate, constraint, index and
instead use ifDefined(...) from `@prisma-next/utils/defined` to conditionally add
those properties (e.g., pass the option value and a mapper to produce the
property object); keep normalizeFieldRefInput and normalizeTargetFieldRefInput
usage intact and only change the conditional spreads to ifDefined calls.
In `@test/integration/test/contract-builder.test.ts`:
- Around line 276-340: Extract the "refined object contract works with schema()
and sql()" it-block into a new test file (e.g.,
contract-builder.refined.test.ts): copy the entire it(...) including setup of
User, Post, contract (defineContract), adapter/context creation
(createStubAdapter, createTestContext), and assertions using schema and sql;
update imports at the top of the new file to include model, field, rel,
int4Column, textColumn, timestamptzColumn, postgresPack, defineContract,
createStubAdapter, createTestContext, schema, sql, SelectAst, ResultType, and
expectTypeOf; remove the copied it-block from contract-builder.test.ts so the
original file stays under 500 lines; run tests to ensure the new file is
detected by the test runner and that no shared test fixtures are left behind.
In `@test/integration/test/contract-builder.types.test-d.ts`:
- Around line 106-190: This test file is type-only (.test-d.ts) but uses runtime
builders (defineContract, validateContract, createTestContext,
createStubAdapter, schema, sql) which widens types and defeats the purpose of
the test; replace the runtime contract construction with a purely type-level
fixture (e.g., declare a type/const type-only shape that mirrors the intended
contract and use that type in ResultType/type assertions) or move the scenario
into a runtime test (.test.ts) if you need actual builder behavior—in short,
stop calling defineContract/validateContract/createTestContext in this
.test-d.ts and instead reference a static emitted contract type (or move the
code to a .test.ts) so the refined token/ref preservation assertions remain
type-only.
🪄 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: 02933a6b-59b4-4e30-9940-57b835b604e0
⛔ Files ignored due to path filters (4)
projects/ts-contract-authoring-redesign/authoring-api-options-recommendation.mdis excluded by!projects/**projects/ts-contract-authoring-redesign/plan.mdis excluded by!projects/**projects/ts-contract-authoring-redesign/spec.mdis excluded by!projects/**projects/ts-contract-authoring-redesign/typed-cross-model-refs-before-and-after.mdis excluded by!projects/**
📒 Files selected for processing (10)
examples/prisma-next-demo/prisma/contract.tsexamples/prisma-next-demo/src/prisma/contract.d.tsexamples/prisma-next-demo/src/prisma/contract.jsonpackages/2-sql/2-authoring/contract-ts/README.mdpackages/2-sql/2-authoring/contract-ts/src/contract-builder.tspackages/2-sql/2-authoring/contract-ts/src/exports/contract-builder.tspackages/2-sql/2-authoring/contract-ts/src/refined-option-a.tspackages/2-sql/2-authoring/contract-ts/test/contract-builder.refined-option-a.test.tstest/integration/test/contract-builder.test.tstest/integration/test/contract-builder.types.test-d.ts
Comment on lines
+1122
to
+1139
| const tableName = sqlSpec?.table ?? applyNaming(modelName, definition.naming?.tables); | ||
| const fieldToColumn: Record<string, string> = {}; | ||
|
|
||
| for (const [fieldName, fieldBuilder] of Object.entries(modelDefinition.stageOne.fields)) { | ||
| const fieldState = fieldBuilder.build(); | ||
| fieldToColumn[fieldName] = | ||
| fieldState.columnName ?? applyNaming(fieldName, definition.naming?.columns); | ||
| } | ||
|
|
||
| modelSpecs.set(modelName, { | ||
| modelName, | ||
| tableName, | ||
| fieldBuilders: modelDefinition.stageOne.fields, | ||
| fieldToColumn, | ||
| relations: modelDefinition.stageOne.relations, | ||
| attributesSpec, | ||
| sqlSpec, | ||
| }); |
There was a problem hiding this comment.
Reject post-naming storage collisions before lowering.
applyNaming() and explicit table / column overrides can make two models or fields resolve to the same storage name. This loop never checks that invariant, so the later definition silently wins once table() and computeMappings() run.
Diff suggestion
+ const tableOwners = new Map<string, string>();
for (const [modelName, modelDefinition] of Object.entries(models)) {
const tokenModelName = modelDefinition.stageOne.modelName;
if (tokenModelName && tokenModelName !== modelName) {
throw new Error(
`Model token "${tokenModelName}" must be assigned to models.${tokenModelName}. Received models.${modelName}.`,
);
}
const attributesSpec = modelDefinition.buildAttributesSpec();
const sqlSpec = modelDefinition.buildSqlSpec();
const tableName = sqlSpec?.table ?? applyNaming(modelName, definition.naming?.tables);
+ const existingModel = tableOwners.get(tableName);
+ if (existingModel) {
+ throw new Error(
+ `Models "${existingModel}" and "${modelName}" both map to table "${tableName}".`,
+ );
+ }
+ tableOwners.set(tableName, modelName);
const fieldToColumn: Record<string, string> = {};
+ const columnOwners = new Map<string, string>();
for (const [fieldName, fieldBuilder] of Object.entries(modelDefinition.stageOne.fields)) {
const fieldState = fieldBuilder.build();
- fieldToColumn[fieldName] =
- fieldState.columnName ?? applyNaming(fieldName, definition.naming?.columns);
+ const columnName =
+ fieldState.columnName ?? applyNaming(fieldName, definition.naming?.columns);
+ const existingField = columnOwners.get(columnName);
+ if (existingField) {
+ throw new Error(
+ `Model "${modelName}" maps both "${existingField}" and "${fieldName}" to column "${columnName}".`,
+ );
+ }
+ columnOwners.set(columnName, fieldName);
+ fieldToColumn[fieldName] = columnName;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/2-sql/2-authoring/contract-ts/src/contract-builder.ts` around lines
1122 - 1139, The code builds tableName via applyNaming and fieldToColumn for
each model but never rejects duplicate storage names; add checks before
modelSpecs.set to validate (1) tableName is unique across already-processed
models in modelSpecs (reject with an Error referencing modelName and the
colliding tableName), and (2) the computed fieldToColumn values are unique
within this model (detect duplicate column names produced by
modelDefinition.stageOne.fields and throw an Error naming the model and
duplicate column). Use the existing symbols (applyNaming, tableName,
fieldToColumn, modelDefinition.stageOne.fields, modelSpecs) to locate the logic
and return/throw on collision so later lowering cannot silently overwrite
storage names.
Comment on lines
+1163
to
+1168
| if (fieldState.executionDefault) { | ||
| next = next.generated(columnName, { | ||
| type: descriptor, | ||
| generated: fieldState.executionDefault, | ||
| }); | ||
| continue; |
There was a problem hiding this comment.
Don't silently drop modifiers on generated fields.
field.generated(...).optional() and field.generated(...).default(...) are both currently expressible, but this branch ignores those flags and emits only the execution default. That leaves the refined builder state out of sync with the contract that gets built.
Diff suggestion
if (fieldState.executionDefault) {
+ if (fieldState.nullable || fieldState.default) {
+ throw new Error(
+ `Generated field "${spec.modelName}.${fieldName}" cannot also be optional or define a default.`,
+ );
+ }
next = next.generated(columnName, {
type: descriptor,
generated: fieldState.executionDefault,
});
continue;
}
Comment on lines
+1186
to
+1192
| const idConstraint = resolveModelIdConstraint(spec); | ||
| if (idConstraint) { | ||
| next = next.primaryKey( | ||
| mapFieldNamesToColumnNames(spec.modelName, idConstraint.fields, spec.fieldToColumn), | ||
| idConstraint.name, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Block nullable fields from becoming the primary key.
Nothing here re-checks the participating fields before primaryKey(). An author can define .optional().id() or an attribute-level id over optional fields, and the storage IR will describe a nullable primary-key column.
Diff suggestion
const idConstraint = resolveModelIdConstraint(spec);
if (idConstraint) {
+ for (const fieldName of idConstraint.fields) {
+ if (spec.fieldBuilders[fieldName]!.build().nullable) {
+ throw new Error(
+ `Model "${spec.modelName}" uses nullable field "${fieldName}" in its identity.`,
+ );
+ }
+ }
next = next.primaryKey(
mapFieldNamesToColumnNames(spec.modelName, idConstraint.fields, spec.fieldToColumn),
idConstraint.name,
);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const idConstraint = resolveModelIdConstraint(spec); | |
| if (idConstraint) { | |
| next = next.primaryKey( | |
| mapFieldNamesToColumnNames(spec.modelName, idConstraint.fields, spec.fieldToColumn), | |
| idConstraint.name, | |
| ); | |
| } | |
| const idConstraint = resolveModelIdConstraint(spec); | |
| if (idConstraint) { | |
| for (const fieldName of idConstraint.fields) { | |
| if (spec.fieldBuilders[fieldName]!.build().nullable) { | |
| throw new Error( | |
| `Model "${spec.modelName}" uses nullable field "${fieldName}" in its identity.`, | |
| ); | |
| } | |
| } | |
| next = next.primaryKey( | |
| mapFieldNamesToColumnNames(spec.modelName, idConstraint.fields, spec.fieldToColumn), | |
| idConstraint.name, | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/2-sql/2-authoring/contract-ts/src/contract-builder.ts` around lines
1186 - 1192, The code calls resolveModelIdConstraint(...) then unconditionally
passes the fields to next.primaryKey(...), which can produce nullable PK columns
if any participating model fields are optional; update the logic in
contract-builder.ts to validate the resolved idConstraint.fields against the
model field metadata (using spec.fields or whatever field descriptors are
available on spec) before calling primaryKey: detect any optional/nullable
fields referenced by idConstraint.fields and either (a) throw or emit a clear
error mentioning spec.modelName and the offending field names, or (b)
strip/convert them according to project rules so that primaryKey is only invoked
with non-nullable fields; ensure you reference resolveModelIdConstraint,
mapFieldNamesToColumnNames, primaryKey, spec.modelName and spec.fieldToColumn
when implementing the check so the fix is applied at the correct spot.
Comment on lines
+981
to
+983
| export function isRefinedContractInput(value: unknown): value is RefinedContractInput { | ||
| return typeof value === 'object' && value !== null && 'target' in value; | ||
| } |
There was a problem hiding this comment.
Tighten the refined-input guard before dispatching.
Any object with a target key passes this check. In JS or untyped callers, { target: 'postgres' } or even an already-built contract will go down the refined path and only fail later with unrelated errors. Please validate the target pack-ref shape here (kind, familyId, targetId) so bad inputs fail immediately.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/2-sql/2-authoring/contract-ts/src/refined-option-a.ts` around lines
981 - 983, The current isRefinedContractInput guard only checks for a 'target'
key; tighten it by verifying that value is an object with a non-null object
"target" and that target has the expected pack-ref shape (e.g., properties
"kind", "familyId", and "targetId" exist and are strings); update
isRefinedContractInput to return true only when these checks pass so malformed
inputs like { target: 'postgres' } or incomplete contracts are rejected
immediately before dispatching.
Comment on lines
+379
to
+388
| if (false) { | ||
| rel.belongsTo(User, { from: 'userId', to: 'id' }); | ||
| rel.hasMany(Post, { by: 'userId' }); | ||
|
|
||
| // @ts-expect-error relation targets must expose real scalar fields | ||
| rel.belongsTo(User, { from: 'userId', to: 'posts' }); | ||
|
|
||
| // @ts-expect-error relation targets must expose real scalar fields | ||
| rel.hasMany(Post, { by: 'posts' }); | ||
| } |
There was a problem hiding this comment.
Replace the if (false) compile-only blocks.
Biome already reports both branches as lint/correctness/noConstantCondition, so this new test file won’t pass lint as written. Move these @ts-expect-error checks into an uncalled helper or a .test-d.ts companion instead.
Possible fix
- if (false) {
- rel.belongsTo(User, { from: 'userId', to: 'id' });
- rel.hasMany(Post, { by: 'userId' });
-
- // `@ts-expect-error` relation targets must expose real scalar fields
- rel.belongsTo(User, { from: 'userId', to: 'posts' });
-
- // `@ts-expect-error` relation targets must expose real scalar fields
- rel.hasMany(Post, { by: 'posts' });
- }
+ const assertRelationTargets = () => {
+ rel.belongsTo(User, { from: 'userId', to: 'id' });
+ rel.hasMany(Post, { by: 'userId' });
+
+ // `@ts-expect-error` relation targets must expose real scalar fields
+ rel.belongsTo(User, { from: 'userId', to: 'posts' });
+
+ // `@ts-expect-error` relation targets must expose real scalar fields
+ rel.hasMany(Post, { by: 'posts' });
+ };
+ void assertRelationTargets;
...
- if (false) {
- // `@ts-expect-error` unnamed models must not expose token-based cross-model refs
- Anonymous.ref('id');
-
- // `@ts-expect-error` unnamed models must not expose token-based cross-model refs
- Anonymous.refs.id;
-
- // `@ts-expect-error` unnamed models must not compile as relation targets
- rel.belongsTo(Anonymous, { from: 'id', to: 'id' });
-
- // `@ts-expect-error` unnamed models must not compile through lazy relation targets
- rel.hasMany(() => Anonymous, { by: 'id' });
- }
+ const assertAnonymousModelErrors = () => {
+ // `@ts-expect-error` unnamed models must not expose token-based cross-model refs
+ Anonymous.ref('id');
+
+ // `@ts-expect-error` unnamed models must not expose token-based cross-model refs
+ Anonymous.refs.id;
+
+ // `@ts-expect-error` unnamed models must not compile as relation targets
+ rel.belongsTo(Anonymous, { from: 'id', to: 'id' });
+
+ // `@ts-expect-error` unnamed models must not compile through lazy relation targets
+ rel.hasMany(() => Anonymous, { by: 'id' });
+ };
+ void assertAnonymousModelErrors;Also applies to: 400-412
🧰 Tools
🪛 Biome (2.4.9)
[error] 379-379: Unexpected constant condition.
(lint/correctness/noConstantCondition)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/2-sql/2-authoring/contract-ts/test/contract-builder.refined-option-a.test.ts`
around lines 379 - 388, The test currently wraps ts-expect-error checks inside
an if (false) block which triggers lint errors; remove that constant-condition
block and move the failing assertions into a non-executed TypeScript-only
context: either place the three offending calls (rel.belongsTo(User, { from:
'userId', to: 'posts' }); rel.hasMany(Post, { by: 'posts' }); and the two valid
calls for context if needed) inside an uncalled helper function (e.g., function
_tsExpectErrors() { /* keep `@ts-expect-error` lines here */ } ) or move them into
a companion .test-d.ts file so the `@ts-expect-error` comments are checked by TS
without producing runtime/lintrule failures; target the calls to rel.belongsTo
and rel.hasMany and the types User and Post when relocating the lines.
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
1 participant
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
model(...).relations(...).attributes(...).sql(...)Testing
Linear
Summary by CodeRabbit
New Features
defineContract({ ... })instead of fluent builder chains.field,model,rel) for intuitive field and relation definitions.Documentation