-
-
Couldn't load subscription status.
- Fork 126
fix: issue with zod generation for self-relation involving delegate types #2238
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughAdjusts Zod transformer’s delegate input type resolution to pick from candidate names based on existing DMMF input types (handles certain Prisma naming variants). Adds an inheritance-aware field comparison in SDK backlink detection. Adds a regression test validating delegated subtype update input schema for a self-relation. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant T as Zod Transformer
participant D as DMMF.inputObjectTypes
participant R as Result
T->>T: Derive mappedInputTypeName
T->>T: Build candidate list (mapped, UpdateOne→Update, NestedInput→Input)
T->>D: Search for first matching input type name among candidates
alt Match found
D-->>T: MatchedName
T->>R: Use MatchedName for processedInputType
else No match
D-->>T: None
T->>R: Use original mappedInputTypeName
end
sequenceDiagram
autonumber
participant U as getRelationBackLink
participant F as Field
participant O as OtherField
U->>U: sameField(O, F) compares name + base ($inheritedFrom | $container)
alt same field
U-->>U: Skip backlink candidate
else different field
U-->>U: Consider as backlink
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Pre-merge checks (3 passed, 1 warning, 1 inconclusive)❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (3)
✨ Finishing Touches
🧪 Generate unit tests
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/sdk/src/utils.ts (1)
662-669: Correctly skipping self/clone in backlink detection for delegated inheritance.Replacing strict identity with a structural "same field" check fixes the self-relation case where inherited fields are cloned. This is the right place to apply it and the comparison logic (name + base model via
$inheritedFrom ?? $container) is sound.Minor: consider extracting
sameFieldas a shared utility if other places compare fields across inheritance.Also applies to: 671-672
tests/regression/tests/issue-2226.test.ts (1)
24-31: Solid regression; consider one more assertion to harden failures.Add an explicit check that
zodSchemasand the target object schema both exist before parsing, so failures surface with clearer messages.- const schema = zodSchemas.objects.RegistrationFrameworkUpdateInputObjectSchema; - expect(schema).toBeDefined(); + expect(zodSchemas).toBeDefined(); + const schema = zodSchemas.objects?.RegistrationFrameworkUpdateInputObjectSchema; + expect(schema).toBeDefined();Optional follow-ups (future PR): add a second case where the to-many side appears before the to-one side in the model to cover ordering permutations requested in the issue.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/schema/src/plugins/zod/transformer.ts(1 hunks)packages/sdk/src/utils.ts(1 hunks)tests/regression/tests/issue-2226.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/regression/tests/issue-2226.test.ts (1)
packages/testtools/src/schema.ts (1)
loadSchema(172-248)
⏰ 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). (5)
- GitHub Check: build-test (20.x)
- GitHub Check: build-test (20.x)
- GitHub Check: OSSAR-Scan
- GitHub Check: dependency-review
- GitHub Check: build-test (20.x)
🔇 Additional comments (1)
packages/schema/src/plugins/zod/transformer.ts (1)
260-268: Import safety still OK after remapping.Good call to only add schema import when the mapped type differs from the current object’s
originalName, avoiding self-import cycles.
fixes #2226