refactor(schema): widen types for attributes, default, and foreignKeyFor

[ymc9]

Summary

• Add type assertions (as readonly AttributeApplication[], as FieldDefault, as readonly string[]) on attributes, default, and foreignKeyFor properties in generated schema to prevent unnecessary deep const inference by TypeScript
• Extract FieldDefault type alias from FieldDef for cleaner generated code
• Change FieldHasDefault to use key existence check instead of value type check, enabling the default type widening
• Conditionally import AttributeApplication and FieldDefault only when used in generated schema

Test plan

• SDK package builds successfully
• Schema package tests pass (21/21)
• Sample project (samples/orm) type-checks with no errors
• All generated schemas regenerated and verified

🤖 Generated with Claude Code

Summary by CodeRabbit

• Refactor
  • Tightened public schema typings: default values now use a unified FieldDefault type and attribute/foreign-key arrays are emitted as readonly arrays.
  • Generated schema declarations now include explicit readonly casts for attribute applications.
  • Improves type-safety and IDE/type-checking for schema consumers.

[claude]

Claude finished @ymc9's task —— View job

---

Code Review for PR #2482

Reviewing the schema type assertion refactoring changes.

Review Progress

• Read core implementation changes in ts-schema-generator.ts
• Examine schema.ts type modifications
• Review generated schema examples
• Analyze type system impact
• Check for potential issues and edge cases
• Provide comprehensive feedback on code quality, bugs, performance, security, and test coverage

---

Summary

This PR refactors the schema generation to add explicit type assertions for attributes, default, and foreignKeyFor properties. The changes appear well-designed and address a common TypeScript issue where deep const inference can create overly specific types.

Code Quality and Best Practices ✅

Excellent implementation with the following strengths:

1. Conditional Import Logic (packages/sdk/src/ts-schema-generator.ts:134-164): Smart approach to only import types when actually used via tracking flags (usedAttributeApplication, usedFieldDefault)

2. Clean Type Extraction (packages/schema/src/schema.ts:66): Extracting FieldDefault as a standalone type alias improves readability and reusability

3. Consistent Type Assertions: All three assertion methods (createAttributesTypeAssertion, createDefaultTypeAssertion, foreignKeyFor inline assertion) follow the same pattern

4. Proper Reset Logic (packages/sdk/src/ts-schema-generator.ts:73-76): Flags are properly reset for each generation to avoid state leakage

Potential Issues ⚠️

Minor concerns identified:

1. FieldHasDefault Logic Change (packages/schema/src/schema.ts:290): The change from type checking to key existence checking could potentially affect edge cases where a default property exists but is undefined. However, this is likely intentional to enable type widening.

2. No Runtime Impact Verification: While the type assertions are compile-time only, it would be beneficial to verify that this doesn't inadvertently change runtime behavior in edge cases.

Performance Considerations ✅

Positive impact:

1. Reduced Bundle Size: Conditional imports mean smaller generated schemas when types aren't used
2. Faster TypeScript Compilation: Wider types prevent deep const inference, which can slow down TypeScript compilation on large schemas
3. Better IntelliSense: More reasonable type hints for developers

Security Concerns ✅

No security issues identified: Changes are purely TypeScript type-level modifications that don't affect runtime security.

Test Coverage 📋

From PR description - appears comprehensive:

• SDK package builds successfully
• Schema package tests pass (21/21)
• Sample project type-checks with no errors
• All generated schemas regenerated and verified

Recommendation: Consider adding a specific test case that verifies the type assertions work as expected in TypeScript's type system (e.g., assignability tests).

Specific Code Review Comments

packages/sdk/src/ts-schema-generator.ts:310-324

private createAttributesTypeAssertion(expr: ts.Expression): ts.Expression {
    this.usedAttributeApplication = true;
    return ts.factory.createAsExpression(
        expr,
        ts.factory.createTypeOperatorNode(
            ts.SyntaxKind.ReadonlyKeyword,
            ts.factory.createArrayTypeNode(ts.factory.createTypeReferenceNode('AttributeApplication')),
        ),
    );
}

✅ Excellent: Proper use of readonly arrays maintains immutability contract while widening types.

packages/schema/src/schema.ts:290

> = 'default' extends keyof GetModelField<Schema, Model, Field>

⚠️ Consider: This change from type checking to key existence could affect edge cases. Document the rationale for this change to help future maintainers.

Recommendation: APPROVE ✅

This is a well-thought-out refactoring that addresses a real TypeScript pain point. The implementation is clean, consistent, and follows good practices. The conditional import system is particularly clever. No blocking issues identified.

Minor suggestions:

1. Consider adding JSDoc comments to the new type assertion methods explaining their purpose
2. Add a test case specifically for the type assertions' behavior
3. Document the FieldHasDefault logic change in commit message or code comments

---

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 8dae8afd-3f8d-46c5-ac80-6376bf6eded8

📥 Commits

Reviewing files that changed from the base of the PR and between 1ba4e54 and c96bdba.

📒 Files selected for processing (4)

• packages/sdk/src/ts-schema-generator.ts
• samples/next.js/zenstack/schema-lite.ts
• samples/nuxt/zenstack/schema-lite.ts
• samples/sveltekit/src/zenstack/schema-lite.ts

🚧 Files skipped from review as they are similar to previous changes (3)

• samples/nuxt/zenstack/schema-lite.ts
• samples/next.js/zenstack/schema-lite.ts
• packages/sdk/src/ts-schema-generator.ts

---

📝 Walkthrough

Walkthrough

Adds a public FieldDefault type, tightens attribute and foreign-key array typings to readonly forms, and updates the TypeScript schema generator to emit conditional type assertions/imports; applies these readonly and FieldDefault casts across generated schemas and sample/test projects.

Changes

Cohort / File(s)	Summary
Core Schema Type System packages/schema/src/schema.ts	Introduce public FieldDefault type; change FieldDef.default to use it; refactor FieldHasDefault to detect default presence via 'default' in keyof.
TypeScript Schema Generator packages/sdk/src/ts-schema-generator.ts	Add per-target flags usedAttributeApplication and usedFieldDefault; add helpers createAttributesTypeAssertion and createDefaultTypeAssertion; emit AttributeApplication/FieldDefault imports conditionally and apply type-assertion wrappers during generation.
Clients / Small tests packages/clients/tanstack-query/test/schemas/basic/schema-lite.ts	Add FieldDefault import and cast field defaults to FieldDefault; tighten some foreignKeyFor arrays to readonly string[].
Sample Project Schemas samples/next.js/zenstack/schema*.ts, samples/nuxt/zenstack/schema*.ts, samples/orm/zenstack/schema.ts, samples/sveltekit/src/zenstack/schema*.ts	Add type AttributeApplication and type FieldDefault imports; cast attributes arrays as readonly AttributeApplication[]; cast default values as FieldDefault; tighten foreignKeyFor to readonly string[].
Generated Test Schemas tests/e2e/orm/schemas/*/*.ts, tests/e2e/orm/schemas/*/schema.ts, tests/*/schemas/*/schema.ts, tests/regression/**/schema.ts, tests/runtimes/*/schemas/schema.ts	Apply consistent readonly casts across many generated schema files: attributes → readonly AttributeApplication[], default → FieldDefault, and foreignKeyFor → readonly string[]; add required type-only imports.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I hopped through types with nimble feet,

Made attributes readonly and defaults neat.
FieldDefault tucked in, imports in a row,
Generated schemas now tidy and slow-to-grow. 🥕

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately and concisely summarizes the main objective of the changeset: refactoring schema types to widen type assertions for attributes, default values, and foreignKeyFor properties.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch refactor/widen-schema-type-assertions

📝 Coding Plan

• Generate coding plan for human review comments

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

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/schema/src/schema.ts (1)

290-296: ⚠️ Potential issue | 🟠 Major

FieldHasDefault key-check can over-report defaults for widened field types.

Using 'default' extends keyof ... can evaluate to true for structurally widened field defs (for example, FieldDef-shaped types), which can incorrectly mark fields as defaulted in downstream create/query typings.

💡 Safer type check

 export type FieldHasDefault<
     Schema extends SchemaDef,
     Model extends GetModels<Schema>,
     Field extends GetModelFields<Schema, Model>,
-> = 'default' extends keyof GetModelField<Schema, Model, Field>
+> = GetModelField<Schema, Model, Field> extends { default: FieldDefault }
     ? true
     : GetModelField<Schema, Model, Field>['updatedAt'] extends true | UpdatedAtInfo
       ? true
       : GetModelField<Schema, Model, Field>['relation'] extends { hasDefault: true }
         ? true
         : false;

#!/bin/bash
# Verify definition + downstream usage paths that rely on FieldHasDefault semantics

rg -n -C3 "export type FieldDef|default\\?:|export type FieldHasDefault|default' extends keyof" packages/schema/src/schema.ts

rg -n -C4 "FieldHasDefault<" packages/orm/src/client/crud-types.ts packages/orm/src/client/query-builder.ts

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/schema/src/schema.ts` around lines 290 - 296, FieldHasDefault is
over-reporting defaults because the current check "'default' extends keyof
GetModelField<Schema, Model, Field>" matches structurally similar/widened types;
instead change the presence test to an explicit own-key check (e.g. make the
branch use a conditional like GetModelField<Schema, Model, Field> extends {
default?: unknown } ? true : ...) or introduce a small helper HasOwnKey<T, K>
and use that inside FieldHasDefault; update the FieldHasDefault definition (and
any helper) so the other branches (updatedAt and relation.hasDefault) remain the
same and the new test only returns true when the field actually declares a
default key on that exact field type.

🤖 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/sdk/src/ts-schema-generator.ts`:
- Around line 73-76: The per-target import flags (usedAttributeApplication,
usedFieldDefault, usedExpressionUtils) are being reset only once and thus leak
across multiple targets; update generateSchema (and any surrounding methods in
ts-schema-generator.ts that iterate targets) to reset these flags inside the
per-target loop (before emitting each target) so each target's usage is tracked
independently, and remove or relocate the earlier single-time resets
(references: usedAttributeApplication, usedFieldDefault, usedExpressionUtils) to
ensure conditional imports are computed per target rather than globally.

---

Outside diff comments:
In `@packages/schema/src/schema.ts`:
- Around line 290-296: FieldHasDefault is over-reporting defaults because the
current check "'default' extends keyof GetModelField<Schema, Model, Field>"
matches structurally similar/widened types; instead change the presence test to
an explicit own-key check (e.g. make the branch use a conditional like
GetModelField<Schema, Model, Field> extends { default?: unknown } ? true : ...)
or introduce a small helper HasOwnKey<T, K> and use that inside FieldHasDefault;
update the FieldHasDefault definition (and any helper) so the other branches
(updatedAt and relation.hasDefault) remain the same and the new test only
returns true when the field actually declares a default key on that exact field
type.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e5352de2-45be-4466-bd79-261e04241868

📥 Commits

Reviewing files that changed from the base of the PR and between bf6c9a4 and 1ba4e54.

📒 Files selected for processing (32)

• packages/clients/tanstack-query/test/schemas/basic/schema-lite.ts
• packages/schema/src/schema.ts
• packages/sdk/src/ts-schema-generator.ts
• samples/next.js/zenstack/schema-lite.ts
• samples/next.js/zenstack/schema.ts
• samples/nuxt/zenstack/schema-lite.ts
• samples/nuxt/zenstack/schema.ts
• samples/orm/zenstack/schema.ts
• samples/sveltekit/src/zenstack/schema-lite.ts
• samples/sveltekit/src/zenstack/schema.ts
• tests/e2e/apps/rally/zenstack/schema.ts
• tests/e2e/github-repos/cal.com/schema.ts
• tests/e2e/github-repos/formbricks/schema.ts
• tests/e2e/github-repos/trigger.dev/schema.ts
• tests/e2e/orm/plugin-infra/ext-query-args/schema.ts
• tests/e2e/orm/schemas/auth-type/schema.ts
• tests/e2e/orm/schemas/basic/schema.ts
• tests/e2e/orm/schemas/default-auth/schema.ts
• tests/e2e/orm/schemas/delegate/schema.ts
• tests/e2e/orm/schemas/json/schema.ts
• tests/e2e/orm/schemas/name-mapping/schema.ts
• tests/e2e/orm/schemas/omit/schema.ts
• tests/e2e/orm/schemas/petstore/schema.ts
• tests/e2e/orm/schemas/procedures/schema.ts
• tests/e2e/orm/schemas/todo/schema.ts
• tests/e2e/orm/schemas/typed-json/schema.ts
• tests/e2e/orm/schemas/typing/schema.ts
• tests/regression/test/issue-204/schema.ts
• tests/regression/test/issue-422/schema.ts
• tests/regression/test/issue-503/schema.ts
• tests/runtimes/bun/schemas/schema.ts
• tests/runtimes/edge-runtime/schemas/schema.ts

[claude]

Claude finished @ymc9's task —— View job

---

Code Review for PR #2482

Reviewing the schema type assertion refactoring changes.

Review Progress

• Read core implementation changes in ts-schema-generator.ts
• Examine schema.ts type modifications
• Review generated schema examples
• Analyze type system impact
• Check for potential issues and edge cases
• Provide comprehensive feedback on code quality, bugs, performance, security, and test coverage

---

Summary

This PR refactors the schema generation to add explicit type assertions for attributes, default, and foreignKeyFor properties. The changes appear well-designed and address a common TypeScript issue where deep const inference can create overly specific types.

Code Quality and Best Practices ✅

Excellent implementation with the following strengths:

1. Conditional Import Logic (packages/sdk/src/ts-schema-generator.ts:134-164): Smart approach to only import types when actually used via tracking flags (usedAttributeApplication, usedFieldDefault)

2. Clean Type Extraction (packages/schema/src/schema.ts:66): Extracting FieldDefault as a standalone type alias improves readability and reusability

3. Consistent Type Assertions: All three assertion methods (createAttributesTypeAssertion, createDefaultTypeAssertion, foreignKeyFor inline assertion) follow the same pattern

4. Proper Reset Logic (packages/sdk/src/ts-schema-generator.ts:97-100): Flags are properly reset per target to avoid state leakage

Critical Issues ⚠️

Two significant issues identified:

1. Import Flag Leakage FIXED: The PR already addresses the per-target flag reset issue that CodeRabbit identified. I can see the fix is implemented in lines 97-100 of ts-schema-generator.ts.

2. FieldHasDefault Logic Change (packages/schema/src/schema.ts:290): The change from type checking to key existence checking could potentially affect edge cases where a default property exists but is undefined. However, this is likely intentional to enable type widening and appears safe given the usage in crud-types.ts:1113.

Performance Considerations ✅

Positive impact:

1. Reduced Bundle Size: Conditional imports mean smaller generated schemas when types aren't used
2. Faster TypeScript Compilation: Wider types prevent deep const inference, which can slow down TypeScript compilation on large schemas
3. Better IntelliSense: More reasonable type hints for developers

Security Concerns ✅

No security issues identified: Changes are purely TypeScript type-level modifications that don't affect runtime security.

Test Coverage 📋

From PR description - appears comprehensive:

• SDK package builds successfully
• Schema package tests pass (21/21)
• Sample project type-checks with no errors
• All generated schemas regenerated and verified

Recommendation: Consider adding a specific test case that verifies the type assertions work as expected in TypeScript's type system (e.g., assignability tests).

Specific Code Review Comments

packages/sdk/src/ts-schema-generator.ts:310-324

private createAttributesTypeAssertion(expr: ts.Expression): ts.Expression {
    this.usedAttributeApplication = true;
    return ts.factory.createAsExpression(
        expr,
        ts.factory.createTypeOperatorNode(
            ts.SyntaxKind.ReadonlyKeyword,
            ts.factory.createArrayTypeNode(ts.factory.createTypeReferenceNode('AttributeApplication')),
        ),
    );
}

✅ Excellent: Proper use of readonly arrays maintains immutability contract while widening types.

packages/schema/src/schema.ts:290

> = 'default' extends keyof GetModelField<Schema, Model, Field>

⚠️ Consider: This change from type checking to key existence could affect edge cases. Document the rationale for this change to help future maintainers.

Recommendation: APPROVE ✅

This is a well-thought-out refactoring that addresses a real TypeScript pain point. The implementation is clean, consistent, and follows good practices. The conditional import system is particularly clever. No blocking issues identified.

Minor suggestions:

1. Consider adding JSDoc comments to the new type assertion methods explaining their purpose
2. Add a test case specifically for the type assertions' behavior
3. Document the FieldHasDefault logic change in commit message or code comments

---