fix: polymorphic logical Prisma schema has identifiers with too long names #1482
Conversation
…names - Shorten long names with a threshold - Fixed issues with missing `@unique` attribute in generated aux fk fields
WalkthroughWalkthroughThe recent changes address the issue of long relation map arguments in Prisma schemas by truncating field names and refactoring relation and constraint logic. Additionally, new tests validate schema loading and data manipulation for models with long names using PostgreSQL databases. Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (4)
packages/schema/src/plugins/prisma/schema-generator.ts (4)
Line range hint
51-51: Import Node.js built-in modules using thenode:protocol to ensure compatibility and clarity.- import fs from 'fs'; - import { writeFile } from 'fs/promises'; - import path from 'path'; + import fs from 'node:fs'; + import { writeFile } from 'node:fs/promises'; + import path from 'node:path';Also applies to: 52-52, 54-54
Line range hint
283-285: Consider usingfor...ofloops instead offorEachfor better performance and readability in scenarios where asynchronous operations are not involved.- dataModel.fields.forEach((field) => { + for (const field of dataModel.fields) {Also applies to: 288-288, 318-321, 336-373, 385-470, 401-469, 581-629
Line range hint
158-158: Use template literals for string concatenation to improve readability and maintain consistency.- const auxName = DELEGATE_AUX_RELATION_PREFIX + '_' + this.truncate(lowerCaseFirst(concrete.name)); + const auxName = `${DELEGATE_AUX_RELATION_PREFIX}_${this.truncate(lowerCaseFirst(concrete.name))}`;Also applies to: 186-201, 195-195, 285-285
Line range hint
176-181: Remove unnecessaryelseclauses to simplify control flow and improve code readability.- if (condition) { - // code - } else { - return; - } + if (condition) { + // code + }Also applies to: 179-181, 562-571, 566-570, 680-682
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- packages/schema/src/plugins/prisma/schema-generator.ts (13 hunks)
- tests/regression/tests/issue-1466.test.ts (1 hunks)
Additional context used
Biome
tests/regression/tests/issue-1466.test.ts
[error] 6-6: Unexpected any. Specify a different type.
[error] 69-69: Unexpected any. Specify a different type.
packages/schema/src/plugins/prisma/schema-generator.ts
[error] 51-51: A Node.js builtin module should be imported with the node: protocol.
[error] 52-52: A Node.js builtin module should be imported with the node: protocol.
[error] 54-54: A Node.js builtin module should be imported with the node: protocol.
[error] 158-158: Do not use template literals if interpolation and special-character handling are not needed.
[error] 176-181: This else clause can be omitted because previous branches break early.
[error] 179-181: This else clause can be omitted because previous branches break early.
[error] 186-201: Template literals are preferred over string concatenation.
[error] 191-198: This else clause can be omitted because previous branches break early.
[error] 195-195: Template literals are preferred over string concatenation.
[error] 283-285: Prefer for...of instead of forEach.
[error] 285-285: Template literals are preferred over string concatenation.
[error] 288-288: Prefer for...of instead of forEach.
[error] 318-321: Prefer for...of instead of forEach.
[error] 336-373: Prefer for...of instead of forEach.
[error] 385-470: Prefer for...of instead of forEach.
[error] 401-469: Prefer for...of instead of forEach.
[error] 562-571: This else clause can be omitted because previous branches break early.
[error] 566-570: This else clause can be omitted because previous branches break early.
[error] 581-629: Prefer for...of instead of forEach.
[error] 680-682: This else clause can be omitted because previous branches break early.
Additional comments not posted (4)
tests/regression/tests/issue-1466.test.ts (2)
1-1: Import statements forcreatePostgresDb,dropPostgresDb, andloadSchemaare correctly added to support the new test cases.
3-236: The test suite for issue #1466 is well-structured and covers various scenarios involving models with long names. It tests the schema loading and data manipulation using Prisma with PostgreSQL databases, which aligns with the PR objectives.Tools
Biome
[error] 6-6: Unexpected any. Specify a different type.
[error] 69-69: Unexpected any. Specify a different type.
packages/schema/src/plugins/prisma/schema-generator.ts (2)
86-86: The constantIDENTIFIER_NAME_MAX_LENGTHis correctly set to accommodate the maximum identifier length for databases like PostgreSQL. This change addresses the issue of relation map arguments being too long.
104-104: TheshortNameMapproperty is a good addition for managing truncated names, enhancing the maintainability of the schema generation logic.
Fixes #1466
@uniqueattribute in generated aux fk fields