chore: Improve performance for prisma-client generation #2177
Conversation
📝 Walkthrough""" WalkthroughThe change refactors the process for transforming and saving TypeScript source files generated for Prisma models. Instead of incrementally building new files and moving them, it now accumulates transformed statements, creates new files in one step, and then synchronously renames them to overwrite the originals. It also adds logic to remove Changes
Sequence Diagram(s)sequenceDiagram
participant Enhancer
participant FileSystem
Enhancer->>Enhancer: Process Prisma client namespace file
Enhancer->>Enhancer: Remove delegate_aux_* fields from variable statements
Enhancer->>FileSystem: Write fixed namespace file and rename over original
Enhancer->>Enhancer: For each Prisma model file:
Enhancer->>Enhancer: Accumulate transformed statements
Enhancer->>FileSystem: Create new source file with all statements
Enhancer->>FileSystem: After all models processed, rename fixed files over originals
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration 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
📜 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 (2)
packages/schema/src/plugins/zod/generator.ts (1)
project(165-167)packages/plugins/tanstack-query/scripts/postbuild.js (1)
fs(3-3)
🔇 Additional comments (6)
packages/schema/src/plugins/enhancer/enhance/index.ts (6)
512-512: LGTM! Good optimization approach.Accumulating statements in an array instead of incremental AST additions is a smart performance optimization that aligns with the PR objectives.
525-525: LGTM! Consistent with the accumulation pattern.The change correctly pushes import statements to the accumulated array instead of adding them incrementally to the source file.
531-531: LGTM! Proper transformation accumulation.The interface transformation is correctly accumulated in the statements array.
533-533: LGTM! Proper transformation accumulation.The type alias transformation is correctly accumulated in the statements array.
535-535: LGTM! Comprehensive statement handling.All other nodes are correctly handled by getting their text and adding to the statements array.
539-544: LGTM! Efficient file creation approach.The optimization of getting the original file structure and replacing statements before creating the new file is a good approach that should improve performance significantly.
|
@ymc9 Any thoughts here? I'm still tracking down some other issues related to how Zenstack re-exports the logical-prisma-client, but wondering if these changes make sense to you. |
|
I'll finish reviewing it today @mhodgson . Thank you for making the PR! |
- Unnecessary regex escape - Type assertion - Removed logic that updates `prismaNamespace.ts` which seems to be unnecessary
|
Hi @mhodgson I guess you've been busy. I've made several changes based on my judgment and will merge the PR. Please take a look when you have time. About to make a new patch release soon. |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/integration/tests/cli/generate.test.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/integration/tests/cli/generate.test.ts (1)
packages/testtools/src/schema.ts (1)
installPackage(69-71)
| @@ -45,7 +45,7 @@ model Post { | |||
| // set up project | |||
| fs.writeFileSync('package.json', JSON.stringify({ name: 'my app', version: '1.0.0' })); | |||
| createNpmrc(); | |||
| installPackage('prisma @prisma/client zod'); | |||
| installPackage('prisma @prisma/client zod@3'); | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Test dependency version pinning looks good, but verify necessity.
The explicit pinning to zod@3 ensures test stability and compatibility with the enhanced schema processing logic mentioned in the PR. However, please verify if this version constraint is actually required or if it's possible to support newer zod versions.
Consider running the following to check if the latest zod version would work:
🏁 Script executed:
#!/bin/bash
# Check latest zod version and any breaking changes
curl -s https://api.github.com/repos/colinhacks/zod/releases/latest | jq '.tag_name'
# Check if there are any major version differences that would justify pinning
npm view zod versions --json | jq '.[-10:]'Length of output: 436
Update test to use the latest Zod major version
Latest stable Zod release is v4.0.5, so pinning to v3 may be unnecessarily restrictive and could prevent us from catching breaking changes or benefiting from fixes in v4. Unless there’s a specific incompatibility with Zod 4, please update the test setup.
• Location: tests/integration/tests/cli/generate.test.ts:48
• Change:
- installPackage('prisma @prisma/client zod@3');
+ installPackage('prisma @prisma/client zod@4');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| installPackage('prisma @prisma/client zod@3'); | |
| installPackage('prisma @prisma/client zod@4'); |
🤖 Prompt for AI Agents
In tests/integration/tests/cli/generate.test.ts at line 48, the test installs
Zod version 3 which is outdated. Update the installPackage call to use the
latest stable Zod major version 4 instead of pinning to v3, unless there is a
known incompatibility. This will ensure the test uses the current Zod release
and can catch any breaking changes or benefit from fixes.
|
@ymc9 yes, sorry, was about to get back to this in the morning. I have some broader issues with the In the current
Thoughts? I'm happy to provide another PR, but need your thoughts and guidance here as it is really up to you. As it currently stands, the Zenstack exports are incompatible with any framework trying to use Prisma model or enum types in a client side bundle. |
Hi @mhodgson , thanks for following up. I think this is not specific to the new "prisma-client" generator, right? With the old generator, the For using the exported model types, does it work if you do a type-only import? I assume bundlers will refrain from bundling js code if everything it seems is type import. But I haven't tried. If so, is it specifically about importing enums (which have to be value imports)? I agree adding a new |
|
@ymc9 you are correct. This worked with the You are right that just importing types works fine, but in this case there are also javascript values (namely the enums) that must be loaded, so just importing types leaves a gap. In our Remix (react-router) codebase I didn't discover this issue until running into actual front-end errors where the code was trying to load files from disk. I'm hopeful that having a clear separation of what files are browser safe vs. meant for the server will help. I thought about raising this issue with the Prisma folks, but I think loading anything out of Prisma called I like the idea of just mirroring the structure of the Prisma Client as much as possible with the Zenstack files/exports. It should make onboarding of new users clean and clear, and adheres to the principal of least surprise. Then the only new/extra part of using Zenstack is the |
Thanks for the clarification. We're on the same page then. A PR is greatly appreciated if you have time! |
In our fairly large schema with delegate types we saw our
zenstack generatetime balloon up to almost 10 minutes when switching to the new 'prisma-client' generator. I was able to track down the performance issue to the use of ts-morph in overwriting the model/*.ts files.This change does two things:
It is likely these same changes would speed up the existing "legacy" implementation as well, but probably not as noticeably since there is only one file being moved instead of one for each model.