-
-
Notifications
You must be signed in to change notification settings - Fork 127
merge dev to main (v2.22.0) #2308
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
…client" generator (#2306)
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (3)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the 📝 WalkthroughWalkthroughBumps JetBrains plugin version and pins Prisma in tests; makes DataSource Changes
Sequence Diagram(s)sequenceDiagram
participant AuthGen as Auth Type Generator
participant GenLookup as Prisma Generator Lookup
participant Out as Generated Types
AuthGen->>GenLookup: getPrismaClientGenerator(model)
alt new Prisma client generator
GenLookup-->>AuthGen: isNewGenerator = true
AuthGen->>AuthGen: mark types .isTypeDef where relevant
AuthGen->>Out: emit Partial<$TypeDefs.Type> merges and relation fields
else legacy generator
GenLookup-->>AuthGen: isNewGenerator = false
AuthGen->>Out: emit Partial<_P.Type> merges and relation fields
end
sequenceDiagram
participant Transformer as TS Expression Transformer
participant EnumRef as Enum Field Reference
Transformer->>EnumRef: transform(enumReference, useLiteralEnum?)
alt useLiteralEnum = true
EnumRef-->>Transformer: return JSON string literal ("VALUE")
else
EnumRef-->>Transformer: return dotted access (Container.Name.VALUE)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
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: 0
🧹 Nitpick comments (2)
packages/schema/src/plugins/enhancer/enhance/index.ts (2)
317-338: Version-aware runtime import and$TypeDefsimport look aligned; consider deduping version and TypeDef checksThe new
runtimeLibraryImportSubPathlogic and conditional$TypeDefsimport look consistent with supporting both old/new client generators and Prisma ≥7 runtime layout. To keep this maintainable, consider:
- Caching
getPrismaVersion()once on the class (it’s now called in multiple helpers) instead of re-reading package metadata in each method.- Reusing the existing
hasTypeDef(this.model)helper (or a single shared predicate) instead of re-derivinghasTypeDefhere and again elsewhere.Also, since the exact
/runtime/clientvs/runtime/librarylocations and$TypeDefsusage depend on Prisma’s evolving API surface, please double-check these paths and conditions against the Prisma version(s) you officially support in this release.
441-446: Updated--no-enginegating for Prisma ≥7 is sensible; revisit behavior when version is unknownBounding
--no-engineto>= 5.2.0 && < 7.0.0matches the intent to avoid passing a removed flag to Prisma 7+. However, the!prismaVersionbranch still appends--no-engineeven when the installed Prisma CLI might actually be 7+ but its version couldn’t be detected (e.g., only a globalprismais present). That could now cause avoidable failures in such environments.It’s worth considering:
- Treating “unknown version” as “don’t add
--no-engine” (safer default), or- At least logging/warning when
prismaVersionis falsy before appending the flag.Please confirm the current Prisma CLI behavior for
--no-engineacross your supported versions to ensure this condition matches reality.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/schema/src/plugins/enhancer/enhance/index.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/schema/src/plugins/enhancer/enhance/index.ts (1)
packages/sdk/src/prisma.ts (1)
getPrismaVersion(60-79)
⏰ 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). (4)
- GitHub Check: build-test (20.x)
- GitHub Check: dependency-review
- GitHub Check: build-test (20.x)
- GitHub Check: build-test (20.x)
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: 0
🧹 Nitpick comments (1)
packages/schema/src/cli/actions/generate.ts (1)
53-56: Tighten Prisma 7+ detection and make the warning more informativeThe warning is useful, but you might want to (a) also catch 7.x pre‑releases and (b) be defensive against odd version strings while including the actual version in the message.
A small refinement could look like:
- const prismaVersion = getPrismaVersion(); - if (prismaVersion && semver.gte(prismaVersion, '7.0.0')) { - console.warn(colors.yellow('Prisma 7 support is untested and not planned. Use with caution.')); - } + const prismaVersion = getPrismaVersion(); + const coerced = prismaVersion && semver.coerce(prismaVersion); + if (coerced && semver.major(coerced) >= 7) { + console.warn( + colors.yellow( + `Prisma ${prismaVersion} detected. Prisma 7 support is untested and not planned. Use with caution.` + ) + ); + }This way:
- Non‑standard but coercible versions don’t crash
semver.- Any 7.x (including pre‑releases) triggers the warning.
- Users see exactly which Prisma version was detected.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/schema/src/cli/actions/generate.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/schema/src/cli/actions/generate.ts (1)
packages/sdk/src/prisma.ts (1)
getPrismaVersion(60-79)
⏰ 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). (4)
- GitHub Check: build-test (20.x)
- GitHub Check: build-test (20.x)
- GitHub Check: build-test (20.x)
- GitHub Check: dependency-review
🔇 Additional comments (1)
packages/schema/src/cli/actions/generate.ts (1)
3-6: Imports for Prisma version awareness look goodBringing in
getPrismaVersionandsemverat the CLI layer aligns with the new runtime guard and keeps the rest of the flow unchanged. No issues here.
No description provided.