Fix/Relation model class and Self relation class missing imports#27
Conversation
…eRelationFields When using separateRelationFields=true with swagger=true, the combined class was missing class-validator and Swagger decorators for relation fields. Changes: - Add class-validator imports (IsDefined, IsOptional) for relation fields - Add Swagger imports (ApiProperty) when swagger option is enabled - Add decorators to relation field properties in combined class - Add comprehensive tests to verify decorator generation This ensures the combined class properly decorates relation fields, matching the behavior of RelationClass and single class generation modes. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
When using separateRelationFields=true with self-relations, the Relations class was missing the import for the model type it references. Changes: - Detect self-relations in generateRelationClass function - Import the combined model class for self-relations (e.g., import User from ./User.model) - Avoid empty import statements when no external relations exist - Add comprehensive test suite for self-relation scenarios This ensures self-relations work correctly without circular dependencies while maintaining proper type references.
WalkthroughImplements self-relation handling and enhanced decorator/Swagger import logic in the class generator. Adds a self-relation Prisma schema and new tests validating separation of relation fields, decorator emission for relations, Swagger metadata on relations, and correct imports in combined models. Optimizes conditional import emission for relation types. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CG as Code Generator
participant Schema as Prisma Schema
participant Files as Output Files
Schema->>CG: Provide models (including self-relations)
CG->>CG: Detect relation fields per model
alt Self-relation detected
CG->>CG: Mark hasSelfRelation
CG->>Files: Import combined model for self-type
end
CG->>CG: Collect relation type imports (non-self only)
opt Any non-self relations
CG->>Files: Emit relation type imports
end
CG->>CG: Gather validator imports for relation decorators
opt Swagger enabled
CG->>CG: Gather Swagger imports for relation decorators
end
CG->>Files: Generate Base, Relations, and Combined classes
CG->>Files: Apply decorators on relation fields (validator + Swagger)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/generate-class.ts (1)
75-77: Avoid emitting empty barrel imports in single-class path.Unconditionally calling generateRelationImportsImport can yield
import { } from "./";when there are only self-relations or no relations, contradicting the PR objective and causing lint noise.Apply this diff to gate the import like you did elsewhere:
const relationImports = new Set(); model.fields.forEach((field) => { if (field.relationName && model.name !== field.type) { relationImports.add(field.type); } }); - generateRelationImportsImport(sourceFile, [ - ...relationImports, - ] as Array<string>); + if (relationImports.size > 0) { + generateRelationImportsImport( + sourceFile, + [...relationImports] as Array<string>, + ); + }
🧹 Nitpick comments (8)
src/generate-class.ts (3)
283-307: Consider adding ValidateNested/Type for proper nested validation of relations.If you intend to validate nested relation objects (not just presence/optionality), add
ValidateNested()andType(() => ...)(class-transformer) alongsideIsDefined/IsOptional. Gate behind a config flag to avoid breaking existing consumers/tests.I can draft a minimal, opt-in implementation if desired.
44-51: DRY up import-collection logic.The
validatorImportsaggregation pattern repeats in three places. Factor into a small helper (e.g.,collectValidatorImports(fields)) to reduce duplication and future drift.Also applies to: 138-145, 283-291
316-320: Type-only imports could further reduce runtime coupling.Relation types are used purely in type positions; emitting
import type { Foo } from "./";would eliminate runtime edges and mitigate cycles in exotic TS configs. Optional, since current tests assert value imports.tests/self-relation.test.ts (2)
10-17: Harden exec calls: set cwd and increase buffer to avoid CI flakes.Running build/generate without an explicit cwd and with default buffers can intermittently fail on CI.
Apply this diff:
const execAsync = promisify(exec); describe('Self-Relation Generation', () => { beforeAll(async () => { // Build the generator first - await execAsync('npm run build'); + const projectRoot = path.resolve(__dirname, '..'); + await execAsync('npm run build', { cwd: projectRoot, maxBuffer: 20 * 1024 * 1024 }); // Generate models for self-relation schema const schemaPath = path.resolve(__dirname, 'schemas/self-relation.prisma'); - await execAsync(`npx prisma generate --schema="${schemaPath}"`); + await execAsync(`npx prisma generate --schema="${schemaPath}"`, { + cwd: projectRoot, + maxBuffer: 20 * 1024 * 1024, + }); }, 60000);
70-73: Assert no empty barrel imports are emitted.Add a negative assertion to guard against regressions like
import { } from "./";.Apply this diff:
// For self-relations in Relations class, should import from the combined model expect(userRelations).toContain('import { User } from "./User.model"'); + expect(userRelations).not.toContain('import { } from "./"');// Should NOT import User from itself (no circular import) expect(userModel).not.toContain('import { User } from "./"'); expect(userModel).not.toContain('import { User } from "./User.model"'); + expect(userModel).not.toContain('import { } from "./"');Also applies to: 112-115
tests/swagger-generation.test.ts (2)
10-17: Deduplicate build/generate setup and harden exec options.Running
npm run buildtwice and missing explicit cwd/buffer can slow or flake CI. Prefer a single setup or set consistent exec options in both blocks.Apply this minimal hardening in both beforeAll blocks:
- await execAsync('npm run build'); + const projectRoot = path.resolve(__dirname, '..'); + await execAsync('npm run build', { cwd: projectRoot, maxBuffer: 20 * 1024 * 1024 }); ... - await execAsync(`npx prisma generate --schema="${schemaPath}"`); + await execAsync(`npx prisma generate --schema="${schemaPath}"`, { + cwd: projectRoot, + maxBuffer: 20 * 1024 * 1024, + });Optionally, move build into a single top-level beforeAll.
Also applies to: 74-83
102-105: Add negative assertions against empty barrel imports.Guard for
import { } from "./";regressions in combined classes.Apply this diff:
// Check for relation type imports (from index file) expect(userModel).toContain('import { Post } from "./"'); + expect(userModel).not.toContain('import { } from "./"');// Check for relation type imports (from index file) expect(postModel).toContain('import { User } from "./"'); + expect(postModel).not.toContain('import { } from "./"');Also applies to: 134-136
tests/schemas/self-relation.prisma (1)
23-27: Optional: add an index for mentorId to mirror real-world schemas.Not required for generation, but a lightweight improvement for parity with typical self-relation setups.
For example:
@@index([mentorId])
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/generate-class.ts(3 hunks)tests/schemas/self-relation.prisma(1 hunks)tests/self-relation.test.ts(1 hunks)tests/swagger-generation.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/generate-class.ts (1)
src/helpers.ts (7)
generateRelationImportsImport(260-268)getDecoratorsImportsByType(213-241)generateClassValidatorImport(243-251)shouldImportSwagger(295-297)getSwaggerImportsByType(299-303)generateSwaggerImport(305-313)getDecoratorsByFieldType(80-145)
🔇 Additional comments (2)
src/generate-class.ts (2)
218-236: Self-relation handling in Relations class looks correct.Detects self-relations, imports the combined model, and avoids importing from the barrel when unnecessary. Nice.
Also applies to: 238-242
308-320: Gated relation-type imports in combined class are good.Prevents empty barrel imports; mirrors the Relations class behavior.
|
Thanks for your contribution! |
|
Released in 6.1.1 |
2 participants
This PR is a merge PR of PR #25 #26.
Changes:
Add class-validator imports (IsDefined, IsOptional) for relation fields
Add Swagger imports (ApiProperty) when swagger option is enabled
Add decorators to relation field properties in combined class
Add comprehensive tests to verify decorator generation
This ensures the combined class properly decorates relation fields, matching the behavior of RelationClass and single class generation modes.
Changes:
Detect self-relations in generateRelationClass function
Import the combined model class for self-relations (e.g., import User from ./User.model)
Avoid empty import statements when no external relations exist
Add comprehensive test suite for self-relation scenarios
This ensures self-relations work correctly without circular dependencies while maintaining proper type references.
Summary by CodeRabbit