Skip to content

Conversation

@ymc9
Copy link
Member

@ymc9 ymc9 commented Oct 14, 2025

fixes #2243

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 14, 2025

📝 Walkthrough

Walkthrough

Enhancer now detects which Prisma client generator is used to choose "client" vs "models", limits enhancement generation for the new client to the "node" target, and conditionally emits and re-exports json-types.ts only when model declarations include type definitions. The SDK no longer injects // @ts-nocheck into emitted files.

Changes

Cohort / File(s) Summary
Enhancer: conditional models/json-types & target reduction
packages/schema/src/plugins/enhancer/enhance/index.ts
For the new prisma-client generator, models.ts always re-exports from the Prisma base import and conditionally re-exports ./json-types only when any model declaration is a type; json-types.ts is generated only if needed. generateEnhance is invoked only for the "node" target for the new client (removes "edge").
Enhancer: generator detection & client path selection
packages/schema/src/plugins/enhancer/index.ts
Uses getPrismaClientGenerator(model) when a custom output is configured to choose "client" (new generator) vs "models" (legacy) for prisma client import path resolution; resolves prismaClientPath relative to schema as before.
SDK: emission/header behavior
packages/sdk/src/code-gen.ts
Compiler types include 'node'; removed automatic insertion of // @ts-nocheck into saved source files and per-file pre-emission bypass directives; original file header/content now preserved on save.
Type handling & transformer heuristics
packages/schema/src/plugins/zod/utils/schema-gen.ts, packages/sdk/src/typescript-expression-transformer.ts
Zod utils: when a field default is a string literal and the field is Json or has @json, return the raw string instead of JSON-stringifying it; imports adjusted. Transformer: added heuristic to fold literal-evaluable binary comparisons to 'true'/'false'.
SDK utils: expanded attribute typing
packages/sdk/src/utils.ts
hasAttribute signature expanded to accept TypeDefField in its decl union parameter.
TanStack Query: generator + tests
packages/plugins/tanstack-query/src/generator.ts, packages/plugins/tanstack-query/tests/plugin.test.ts
generateQueryHook adds angular to v5 infinite-query target list so getNextPageParam fallback applies for Angular. Tests: removed unused queryKey destructuring/logs and replaced Angular dependency alias with direct @tanstack/angular-query-experimental@5.84.x.
tRPC & integration tests: pinned deps / test deps
packages/plugins/trpc/tests/trpc.test.ts, tests/integration/tests/cli/generate.test.ts, tests/integration/tests/cli/plugins.test.ts, tests/integration/tests/enhancements/with-delegate/plugin-interaction.test.ts
Test fixtures and integration tests updated to pin TRPC packages to @10 where applicable and add @types/node@20 to installed/devDependencies.
Server adapters: skipped tests
packages/server/tests/adapter/hono.test.ts, packages/server/tests/adapter/sveltekit.test.ts
The "custom load path" test cases are marked it.skip (disabled) with TODO and eslint disable comments.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Build as Schema build
  participant Enhancer as Enhancer plugin
  participant GenDetect as getPrismaClientGenerator
  participant FS as File writer

  Build->>Enhancer: run enhance(model, options)
  Enhancer->>GenDetect: getPrismaClientGenerator(model)
  GenDetect-->>Enhancer: { isNewPrismaClientGenerator }

  alt new prisma-client
    Enhancer->>Enhancer: prismaClientPath := ".../client"
    Enhancer->>Enhancer: targets := ["node"]
  else legacy prisma-client
    Enhancer->>Enhancer: prismaClientPath := ".../models"
    Enhancer->>Enhancer: targets := ["node","edge"]
  end

  Enhancer->>Enhancer: hasTypeDefs? = any model is a type
  alt has type defs
    Enhancer->>FS: write `json-types.ts`
    Enhancer->>FS: write `models.ts` exporting base models + `./json-types`
  else no type defs
    Enhancer->>FS: write `models.ts` exporting base models only
  end

  loop per target in targets
    Enhancer->>FS: generateEnhance(target, prismaClientPath)
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Pre-merge checks and finishing touches

❌ Failed checks (3 warnings)
Check name Status Explanation Resolution
Linked Issues Check ⚠️ Warning The linked issue #2243 requires updating generated hooks and imports to pull Prisma and model types from the client directory instead of models, but while the enhancer logic now chooses “client” for custom output paths, there is no evident change in the TanStack Query hook generator or default import paths to satisfy this requirement fully. Ensure that the hook generator in packages/plugins/tanstack-query and the default code-generation paths are updated to import types from the client directory across all output scenarios, not only when a custom path is specified.
Out of Scope Changes Check ⚠️ Warning The pull request contains widespread modifications unrelated to the client-vs-model type export fix—such as compiler options changes, test skips, dependency pins, expression transformer heuristics, and utility signature updates—which are outside the scope of issue #2243. Split unrelated updates into separate pull requests or revert them in this branch to maintain focus on the type export fix and simplify review.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title references fixing compilation issues in the new Prisma client generator, which aligns with a real aspect of the changeset related to the generator, but it does not clearly summarize the primary objective of correcting type exports from models to client, making it partially related and somewhat broad.
Description Check ✅ Passed The description “fixes #2243” correctly references the linked issue and indicates that the pull request addresses that specific problem, so it is on-topic and satisfies the lenient description requirement.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/prisma-client-compile

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 75f5482 and 92760f3.

📒 Files selected for processing (3)
  • packages/schema/src/plugins/enhancer/enhance/index.ts (3 hunks)
  • packages/schema/src/plugins/enhancer/index.ts (2 hunks)
  • packages/sdk/src/code-gen.ts (0 hunks)
💤 Files with no reviewable changes (1)
  • packages/sdk/src/code-gen.ts
🧰 Additional context used
🧬 Code graph analysis (2)
packages/schema/src/plugins/enhancer/enhance/index.ts (2)
packages/language/src/generated/ast.ts (1)
  • isTypeDef (650-652)
packages/sdk/src/code-gen.ts (1)
  • saveSourceFile (25-37)
packages/schema/src/plugins/enhancer/index.ts (1)
packages/sdk/src/utils.ts (1)
  • getPrismaClientGenerator (700-723)
⏰ 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: dependency-review
  • GitHub Check: build-test (20.x)
  • GitHub Check: build-test (20.x)
  • GitHub Check: OSSAR-Scan
🔇 Additional comments (4)
packages/schema/src/plugins/enhancer/enhance/index.ts (3)

132-145: LGTM!

The conditional export of json-types is correct. It only exports when type definitions exist in the model, which prevents importing a non-existent file.


606-612: LGTM!

The conditional generation of json-types.ts is correct. The file is only created when type definitions exist, which prevents empty file generation and aligns with the conditional export in models.ts.


168-171: Verify removal of the ‘edge’ target for the new Prisma Client generator
No documentation (e.g. CHANGELOG, migration guide) or tests reference edge support; if this is a breaking change, document it and update or remove any edge-related tests.

packages/schema/src/plugins/enhancer/index.ts (1)

39-40: Confirm fallback to 'models' when Prisma client generator is missing
The use of optional chaining means that if getPrismaClientGenerator returns undefined, the path will default to 'models'. Ensure this fallback is intended.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8fbc8ad and 722e8fb.

📒 Files selected for processing (1)
  • packages/plugins/tanstack-query/tests/plugin.test.ts (2 hunks)
⏰ 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: OSSAR-Scan
  • GitHub Check: build-test (20.x)
  • GitHub Check: dependency-review
  • GitHub Check: build-test (20.x)
  • GitHub Check: build-test (20.x)
🔇 Additional comments (1)
packages/plugins/tanstack-query/tests/plugin.test.ts (1)

347-347: LGTM! Simplified dependency declaration.

Removing the npm alias indirection (@tanstack/angular-query-v5@npm:...) in favor of directly referencing @tanstack/angular-query-experimental@5.84.x makes the dependency clearer and easier to understand.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

♻️ Duplicate comments (1)
packages/server/tests/adapter/hono.test.ts (1)

85-87: Same issue as in sveltekit.test.ts—investigate systematically.

This is the same test skip pattern as in packages/server/tests/adapter/sveltekit.test.ts:84-86. Since both Hono and SvelteKit adapters are affected, this suggests a systemic issue rather than an adapter-specific problem. The investigation should address both test files together.

Verify whether other adapter test files have similar skipped tests:

#!/bin/bash
# Search for other skipped custom load path tests
echo "=== Finding all it.skip patterns in adapter tests ==="
fd -e ts -e js . packages/server/tests/adapter/ --exec rg -n "it\.skip.*custom.*load" {} \;

echo ""
echo "=== Checking for TODO comments in adapter tests ==="
fd -e ts -e js . packages/server/tests/adapter/ --exec rg -n "TODO.*CI|TODO.*investigate" {} \;
🧹 Nitpick comments (1)
packages/sdk/src/typescript-expression-transformer.ts (1)

513-532: Clarify the comment about compile-time evaluation.

The compile-time evaluation logic is sound and correctly identifies expressions that resolve to compile-time constants (literals, currentModel(), currentOperation()). By comparing the transformed string representations, it accurately determines if the comparison will always yield the same result and short-circuits to a boolean literal.

However, the comment on line 518 says "to avoid TS compiler errors," which is slightly misleading. The expressions like ('User' == 'User') are valid TypeScript; this optimization is more about:

  1. Producing cleaner generated code
  2. Avoiding constant-condition warnings (if strict linting is enabled)
  3. Runtime performance (eliminating redundant comparisons)

Consider clarifying the comment:

-                    // resolve trivial comparisons to avoid TS compiler errors
+                    // resolve constant comparisons at compile time for cleaner generated code

Additionally, consider adding test coverage for this optimization to ensure it correctly handles:

  • Literal-to-literal comparisons: 'foo' == 'foo''true'
  • currentModel comparisons: currentModel() == 'User''true' (in User model)
  • currentOperation comparisons: currentOperation() != 'read' → depends on context
  • Mixed cases: currentModel('upper') == 'USER'
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 722e8fb and 92826df.

⛔ Files ignored due to path filters (2)
  • packages/schema/package.json is excluded by !**/*.json
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml, !**/*.yaml
📒 Files selected for processing (6)
  • packages/sdk/src/typescript-expression-transformer.ts (2 hunks)
  • packages/server/tests/adapter/hono.test.ts (1 hunks)
  • packages/server/tests/adapter/sveltekit.test.ts (1 hunks)
  • tests/integration/tests/cli/generate.test.ts (1 hunks)
  • tests/integration/tests/cli/plugins.test.ts (2 hunks)
  • tests/integration/tests/enhancements/with-delegate/plugin-interaction.test.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/sdk/src/typescript-expression-transformer.ts (1)
packages/language/src/generated/ast.ts (2)
  • isLiteralExpr (69-71)
  • isInvocationExpr (486-488)
tests/integration/tests/cli/generate.test.ts (1)
packages/testtools/src/schema.ts (1)
  • installPackage (68-70)
⏰ 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: build-test (20.x)
  • GitHub Check: OSSAR-Scan
  • GitHub Check: dependency-review
🔇 Additional comments (6)
tests/integration/tests/cli/plugins.test.ts (2)

89-89: LGTM!

Adding @types/node@20 is appropriate for ensuring type compatibility with Node.js LTS version 20.


77-77: Ignore tRPC v10 pinning for CLI plugin tests. CLI integration tests are explicitly targeting tRPC v10; v11 compatibility is covered in dedicated plugin tests.

Likely an incorrect or invalid review comment.

tests/integration/tests/cli/generate.test.ts (1)

48-48: LGTM!

The addition of @types/node@20 is consistent with similar changes across the test suite and ensures proper TypeScript type definitions for Node.js 20 APIs.

tests/integration/tests/enhancements/with-delegate/plugin-interaction.test.ts (1)

55-55: Verify tRPC v10 pinning across all packages.

All three tRPC packages are now pinned to major version 10 (@trpc/client@10, @trpc/server@10, @trpc/react-query@10). While this ensures version consistency across the tRPC ecosystem, verify that v10 is intentionally used instead of the current stable v11 release.

Based on learnings.

This verification can use the same script suggested for tests/integration/tests/cli/plugins.test.ts at line 77.

packages/server/tests/adapter/sveltekit.test.ts (1)

84-86: Investigate and track CI failure for SvelteKit custom load path test
Confirm whether the new generator structure broke loadSchema({ output: './zen' }) for SvelteKit (and Hono), update the implementation if needed, or open a GitHub issue and reference it in the TODO.

packages/sdk/src/typescript-expression-transformer.ts (1)

19-19: LGTM!

The import is necessary for the new compile-time evaluation logic added below.

@ymc9 ymc9 merged commit a93a2db into dev Oct 15, 2025
12 checks passed
@ymc9 ymc9 deleted the fix/prisma-client-compile branch October 15, 2025 00:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant