-
-
Notifications
You must be signed in to change notification settings - Fork 126
fix: type with @@auth cannot be properly compiled with new "prisma-client" generator
#2306
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 fixes #2294
📝 WalkthroughWalkthroughAdds TypeDef-aware handling to the enhancer: auth type generation now detects TypeDef vs DataModel, tracks per-type isTypeDef and required relations, renders type references conditionally as $TypeDefs. or _P. based on Prisma generator, injects json-types import when needed, and adds tests including a regression for Changes
Sequence Diagram(s)sequenceDiagram
participant Schema
participant Enhancer
participant AuthGen
participant Output
Schema->>Enhancer: load schema (models + TypeDefs)
Enhancer->>Enhancer: detect hasTypeDef flag
Enhancer->>Enhancer: getPrismaClientGenerator(model) -> isNewGenerator
alt hasTypeDef && isNewGenerator
Enhancer->>Output: include "import type * as $TypeDefs from './json-types'"
else
Enhancer->>Output: include legacy `_P` imports
end
Enhancer->>AuthGen: request auth namespace generation
AuthGen->>AuthGen: findType(name) -> DataModel | TypeDef
AuthGen->>AuthGen: record typeInfo { isTypeDef, requiredRelations }
AuthGen->>Output: render auth types
alt isNewGenerator && typeInfo.isTypeDef
Output->>Schema: reference $TypeDefs.<Type>
else
Output->>Schema: reference _P.<Type>
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)packages/schema/src/plugins/enhancer/enhance/auth-type-generator.ts (1)
⏰ 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)
🔇 Additional comments (4)
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 (1)
packages/schema/src/plugins/enhancer/enhance/auth-type-generator.ts (1)
44-54: Consider defensive check for undefined declaration.At line 48,
isTypeDef(decl)is called wheredeclmight beundefinediffindTypereturns no match. While type guards typically handle undefined gracefully, adding an explicit check would make the code more robust and prevent adding non-existent types to the map.const addTypeField = (typeName: string, fieldName: string, fieldType: string, array: boolean) => { let typeInfo = types.get(typeName); if (!typeInfo) { const decl = findType(typeName); + if (!decl) { + return; + } typeInfo = { isTypeDef: isTypeDef(decl), requiredRelations: [] }; types.set(typeName, typeInfo); } if (!typeInfo.requiredRelations.find((f) => f.name === fieldName)) { typeInfo.requiredRelations.push({ name: fieldName, type: array ? `${fieldType}[]` : fieldType }); } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/schema/src/plugins/enhancer/enhance/auth-type-generator.ts(5 hunks)packages/schema/src/plugins/enhancer/enhance/index.ts(1 hunks)tests/integration/tests/enhancements/with-policy/auth.test.ts(2 hunks)tests/regression/tests/issue-2294.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
tests/integration/tests/enhancements/with-policy/auth.test.ts (1)
packages/testtools/src/schema.ts (1)
loadSchema(173-249)
tests/regression/tests/issue-2294.test.ts (1)
packages/testtools/src/schema.ts (1)
loadSchema(173-249)
packages/schema/src/plugins/enhancer/enhance/auth-type-generator.ts (1)
packages/sdk/src/utils.ts (2)
getPrismaClientGenerator(709-732)getIdFields(521-541)
⏰ 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 (6)
packages/schema/src/plugins/enhancer/enhance/auth-type-generator.ts (2)
1-8: LGTM! Imports correctly added.The new imports
getPrismaClientGeneratorandisTypeDefare properly added to support TypeDef tracking and conditional type reference generation.
88-125: Excellent solution for TypeDef type references!The conditional type reference logic correctly addresses the core issue from #2294. By detecting the new Prisma client generator and using
$TypeDefsfor TypeDefs (which are generated in json-types.ts) versus_Pfor the legacy path, this ensures auth types work correctly across both generator versions.tests/regression/tests/issue-2294.test.ts (1)
1-48: LGTM! Effective regression test for issue #2294.This test correctly validates the fix by:
- Using the new "prisma-client" generator
- Declaring
AuthUseras atype(notmodel) with@@auth- Verifying the generated code compiles and can be imported
This directly addresses the reported issue where TypeDef auth types were undefined in generated enhancement files.
packages/schema/src/plugins/enhancer/enhance/index.ts (1)
325-331: LGTM! Conditional import enables TypeDef references.The
hasTypeDefcheck correctly detects TypeDef declarations and conditionally imports$TypeDefsfrom json-types when using the new generator. This import is essential for the$TypeDefs.${type}references generated by auth-type-generator.ts to resolve correctly.tests/integration/tests/enhancements/with-policy/auth.test.ts (2)
868-922: LGTM! Backwards compatibility preserved.Renaming the existing test to clarify it targets the legacy generator path ensures clear test coverage and maintains validation of the existing functionality.
924-993: LGTM! Comprehensive test coverage for new generator.The new test effectively validates TypeDef auth types with the new "prisma-client" generator by:
- Explicitly configuring the new generator in the schema
- Using custom output paths to test the new code generation paths
- Testing the same policy scenarios as the legacy test to ensure functional parity
- Verifying compilation succeeds with generated types
This provides strong confidence that the fix works correctly for the new generator.
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/schema/src/plugins/enhancer/enhance/auth-type-generator.ts(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/schema/src/plugins/enhancer/enhance/auth-type-generator.ts (1)
packages/sdk/src/utils.ts (2)
getPrismaClientGenerator(709-732)getIdFields(521-541)
⏰ 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: dependency-review
- GitHub Check: build-test (20.x)
🔇 Additional comments (2)
packages/schema/src/plugins/enhancer/enhance/auth-type-generator.ts (2)
60-86: Traversal correctly reusesaddTypeFieldand preserves relation semanticsThe updated traversal now calls
addTypeFieldwith the container type (exprType.name/fieldDecl.$container.name) and the related type name, while ensuring the related type itself is present viaensureType. This keepsrequiredRelationskeyed by the owning auth/user type and still ensures the relation target gets its own entry intypes.Given the guards (
isDataModel(exprType)andisDataModel(fieldType)), this remains constrained to real model relations, and the newisTypeDefflag will be set correctly for TypeDef containers when they appear here.Looks good to me.
88-106: Dynamic$TypeDefsvs_Pselection for TypeDefs vs models looks soundThe
isNewGeneratorcheck combined withtypeInfo.isTypeDef:const typeRef = isNewGenerator && typeInfo.isTypeDef ? `$TypeDefs.${type}` : `_P.${type}`;cleanly addresses the reported issue: for the new
prisma-clientgenerator, auth types backed bytype/TypeDefnow resolve to JSON types ($TypeDefs.X), while DataModels and all legacy generators continue to use_P.X. ReusinggetIdFields(authDecl)for bothDataModelandTypeDefkeeps the “id fields are required” behavior consistent.Assuming the enhancer’s import logic brings in
$TypeDefsonly when needed (as described in the PR), this rendering strategy looks correct and backward‑compatible.Also applies to: 115-118, 122-122
fixes #2294