fix(internal): fall back to schema parsing for Prisma 7 ModelName#1752
fix(internal): fall back to schema parsing for Prisma 7 ModelName#1752bellcoTech wants to merge 3 commits into
Conversation
Prisma 7's new `prisma-client` provider no longer exports `Prisma.ModelName`, which `getPrismaClient()` relies on to populate the codegen mappers config. When both import attempts succeed but yield no ModelName record, codegen ends up with an empty mappers map and emits broken type definitions (e.g. `type X = MakeRelationsOptional<Y, never>`), producing hundreds of downstream type errors in user projects. Add a third fallback that reads model declarations directly from the Prisma schema file (or schema directory, supported in Prisma 7). Model declarations have stable syntax across all providers, so a small regex recovers the names without depending on any generated artefacts. Wired in after both `importGeneratedPrismaClient()` attempts; behaviour is unchanged for projects on the legacy `prisma-client-js` provider because the first attempt still resolves `Prisma.ModelName` normally.
👷 Deploy request for cedarjs pending review.Visit the deploys page to approve it
|
Greptile SummaryThis PR fixes a silent codegen regression for projects using Prisma 7's new
Confidence Score: 4/5Safe to merge with one latency issue for Prisma 7 users that was flagged in a prior review. The fix is functionally correct and well-tested. The only outstanding concern — already documented in a prior outside-diff comment — is that a Prisma 7 project with an already-generated client hits the unnecessary The control flow in Important Files Changed
Reviews (3): Last reviewed commit: "test(internal): end-to-end codegen test ..." | Re-trigger Greptile |
Addresses feedback on cedarjs#1752: - `readdirSync` without `withFileTypes` doesn't distinguish files from subdirectories, so a directory named e.g. `views.prisma/` would slip through the `.endsWith('.prisma')` filter and cause `readFileSync(directoryPath)` to throw `EISDIR`. The outer try/catch would swallow that and the fallback would silently return `null`. Switch to `withFileTypes: true` + `entry.isFile()`. - Add `packages/internal/src/__tests__/readModelNamesFromSchema.test.ts` with dedicated coverage for the fallback. The function silently swallows all errors and returns `null` on failure, so a regression would reproduce the very symptom the PR fixes (empty mappers, type errors) without any visible failure signal — explicit unit tests prevent that. Cases covered: - single-file schema parsing - multi-file directory schema (only `.prisma` files merged) - subdirectories ending in `.prisma` are ignored (greptile-bot case) - schema with no model declarations returns null - getSchemaPath resolving to null returns null - non-existent schema path returns null - getSchemaPath throwing returns null - regex matches indented, tight, and padded model declarations and ignores commented-out ones Tests use a real temp directory rather than memfs so the `isFile()` filter exercises real filesystem semantics, and to avoid adding memfs as a new devDependency. `readModelNamesFromSchema` is exported solely to make these tests possible — the public-API surface of `@cedarjs/internal` is unchanged in practice because consumers import from the package root, not from this submodule.
|
Ughh :( It'd be great with a unit tests to catch regressions. Can you help with that? EDIT: Lol. I see you did just add unit tests! Thank you! 🙏 |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run-many -t build:pack --exclude create-ceda... |
✅ Succeeded | 2s | View ↗ |
nx run-many -t build |
✅ Succeeded | 4s | View ↗ |
nx run-many -t test --minWorkers=1 --maxWorkers=4 |
✅ Succeeded | 1m 46s | View ↗ |
nx run-many -t test:types |
✅ Succeeded | 11s | View ↗ |
☁️ Nx Cloud last updated this comment at 2026-05-11 14:12:14 UTC
|
@bellcoTech It bugs me I didn't catch this even once for all the hundreds of times I've spun up our local testing project(s). Also, why isn't Could you please take a look at adding something to |
Addresses follow-up review on cedarjs#1752 asking for a regression test "at the codegen-output level" — one that would have caught the broken-mappers symptom before it shipped, not just the helper in isolation. Adds `graphqlCodeGen.prismaClientProvider.test.ts`: - Reuses the `example-todo-main` fixture's real `schema.prisma` (model `Todo`). - Mocks the Prisma module to deliberately omit `Prisma.ModelName` — the exact shape Prisma 7's `prisma-client` provider produces at runtime, which is what triggered the original silent-empty-mappers bug. - Stubs `execa.sync` so the intermediate `cedar prisma generate` shell-out in `getPrismaClient` is a no-op. - Stubs `getSchemaPath` to point at the fixture's schema (the fixture predates Prisma's `prisma.config.{ts,cjs}` style, so the real `loadPrismaConfig` would throw on the missing config file — that's a separate piece of plumbing not under test here). - Runs `generateTypeDefGraphQLApi` end-to-end and asserts on the generated `graphql.d.ts` content: - `import { Todo as PrismaTodo } from 'src/lib/db'` is present — proves the schema-parsing fallback ran and recovered the model name. - `type AllMappedModels = MaybeOrArrayOfMaybe<Todo>` is present — `printMappedModelsPlugin`'s output, the strongest single signal that mappers came through. - `type AllMappedModels = MaybeOrArrayOfMaybe<>` is _not_ present — negative assertion against regressing back to empty mappers. Together with the existing helper-level tests in `readModelNamesFromSchema.test.ts`, this gives two layers of cover for the regression class: the unit tests pin the helper's behaviour, and this test pins the codegen pipeline's behaviour when the helper is the only thing providing model names.
|
@Tobbe I've added some tests I'll keep working on it.
The new tests should pick this up as it does an end to end with assertion |
|
@bellcoTech We do run |
|
I also did consider suggesting adding vitest typechecks (https://v3.vitest.dev/guide/testing-types.html), but only the ESM fixture would be able to run them. So it's a bit more complicated to set up. But definitely doable |
|
@Tobbe what does the top of |
import { Prisma } from "src/lib/db"
import { MergePrismaWithSdlTypes, MakeRelationsOptional } from '@cedarjs/api'
import { UserExample as PrismaUserExample } from 'src/lib/db'
import { GraphQLResolveInfo, GraphQLScalarType, GraphQLScalarTypeConfig } from 'graphql';
import { CedarGraphQLContext } from '@cedarjs/graphql-server/dist/types';
export type Maybe<T> = T | null;
export type InputMaybe<T> = Maybe<T>;
export type Exact<T extends { [key: string]: unknown }> = { [K in keyof T]: T[K] };
export type MakeOptional<T, K extends keyof T> = Omit<T, K> & { [SubKey in K]?: Maybe<T[SubKey]> };
export type MakeMaybe<T, K extends keyof T> = Omit<T, K> & { [SubKey in K]: Maybe<T[SubKey]> };
export type MakeEmpty<T extends { [key: string]: unknown }, K extends keyof T> = { [_ in K]?: never };
export type Incremental<T> = T | { [P in keyof T]?: P extends ' $fragmentName' | '__typename' ? T[P] : never };
export type ResolverFn<TResult, TParent, TContext, TArgs> = (
args?: TArgs,
obj?: { root: TParent; context: TContext; info: GraphQLResolveInfo }
) => TResult | Promise<TResult>
export type RequireFields<T, K extends keyof T> = Omit<T, K> & { [P in K]-?: NonNullable<T[P]> };
export type OptArgsResolverFn<TResult, TParent = {}, TContext = {}, TArgs = {}> = (
args?: TArgs,
obj?: { root: TParent; context: TContext; info: GraphQLResolveInfo }
) => TResult | Promise<TResult>
export type RequiredResolverFn<TResult, TParent = {}, TContext = {}, TArgs = {}> = (
args: TArgs,
obj: { root: TParent; context: TContext; info: GraphQLResolveInfo }
) => TResult | Promise<TResult>
/** All built-in and custom scalars, mapped to their actual values */
export type Scalars = {
ID: { input: string; output: string; }If I look at the export const ModelName = {
UserExample: 'UserExample'
} as const
export type ModelName = (typeof ModelName)[keyof typeof ModelName] |
|
Ah ok I can reproduce it in a new project now. When I did the upgrade I kept the api as cjs, so I changed
|
|
@bellcoTech Ahh, yeah, I had to play around a lot with TS config settings and prisma settings to get it all to play nice together for both CJS and ESM. I agree it's a bit weird to have TS files like that mixed with how the rest of your app looks, but it's the only way I could get it to work. So what Cedar generates for a new app by default is CJS, and it works with the prisma settings you get by default. |
|
Yeah I think a comment in the schema file is fine. That way at least you're not playing around with this when trying to get it all to behave. Let me make changes on my project to make sure there's nothing else lingering when I change it too |
|
@Tobbe Confirming now that I have everything else working together, changing back from ts was not breaking. I think a comment in schema is fine. Happy for me to leave that with you so you can do in your commenting style? This PR can be closed |
|
Awesome. Thanks for confirming. So the two real issues you found were the lock race condition in the CLI + the auth headers stuff for request handlers? |
|
I'll follow up with a separate PR with a schema file comment. Leaving this PR open as a reminder in the meantime |
|
Closing this in favor of #1761 @bellcoTech I'm still interested in your answers to the question I posted above regarding the two "real" issues :) |
Had this comment first, but it felt like it was too long ```js // For CJS Cedar apps: // It might feel a little out of place to have .mts files in the codebase, and // especially importing them like we do in `src/lib/db`. // For ESM Cedar apps: // It might feel a little weird to ask Prisma to generate CJS compatible format. // // Setting `moduleFormat = "cjs"` and using `.mts` extensions is however the // only combination that makes Prisma 7, which is ESM-only, work in CJS Cedar // apps while also having the same configuration work for ESM Cedar apps. // // The only reason this works is thanks to Node 24's support for require(esm). // https://www.prisma.io/docs/orm/prisma-schema/overview/generators ``` So I shortened it to what we have now, and then pointing here for more info. This all came out of the discussion on PR #1752
|
@Tobbe Apologies I've been off. Correct re the other two. I'm still fighting local when I change back my prisma file. All works on prod though so I assume I've stuffed something up. I'll look at it more when I'm back tomorrow |
|
@bellcoTech No need to apologize. Thanks for your PRs and collaboration 🙏 |
Had this comment first, but it felt like it was too long ```js // For CJS Cedar apps: // It might feel a little out of place to have .mts files in the codebase, and // especially importing them like we do in `src/lib/db`. // For ESM Cedar apps: // It might feel a little weird to ask Prisma to generate CJS compatible format. // // Setting `moduleFormat = "cjs"` and using `.mts` extensions is however the // only combination that makes Prisma 7, which is ESM-only, work in CJS Cedar // apps while also having the same configuration work for ESM Cedar apps. // // The only reason this works is thanks to Node 24's support for require(esm). // https://www.prisma.io/docs/orm/prisma-schema/overview/generators ``` So I shortened it to what we have now, and then pointing here for more info. This all came out of the discussion on PR #1752



Problem
After migrating a project to Prisma 7's new
prisma-clientgenerator provider,yarn cedar generate types(or any command that triggers it, e.g.yarn cedar build,yarn cedar dev) runs without an explicit error but emits broken GraphQL resolver types.yarn cedar type-checkthen surfaces hundreds of errors like:Inspecting the generated
api/types/graphql.d.tsshows that themappersblock is empty for every model — every resolver is typed againstunknown, andMakeRelationsOptional<Model, never>collapses every relation field tonever.For a real project this can mean 300+ type errors appearing in one go, all on a working codebase, simply because the Prisma generator was switched to the recommended new provider.
Root cause
@cedarjs/internal's codegen builds its GraphQLmappersconfig by readingPrisma.ModelNamefrom the generated client:Prisma 7's new
prisma-clientprovider — the recommended one going forward, see https://www.prisma.io/docs/orm/prisma-schema/overview/generators — deliberately stops exportingPrisma.ModelNameas part of the runtime client. The legacyprisma-client-jsprovider still does, which is why projects on the old provider are unaffected.With
ModelNamemissing,getPrismaClient()falls through to:…codegen then runs with an empty mappers map and emits the broken resolver types the user sees. There's no error or warning — the broken output looks superficially the same as a working one until type-check is run downstream.
Fix
Add a third fallback that reads model declarations straight from the Prisma schema file (or schema directory — Prisma 7 supports multiple
*.prismafiles underprisma/schema/). Model declarations have stable syntax across every provider, so a small regex recovers the names without depending on any generated artefact:…wired in after both
importGeneratedPrismaClient()attempts:The fallback only runs when both client-import attempts succeed-but-without-
ModelName(or both throw). Projects on the legacyprisma-client-jsprovider take exactly the same path they always have —getModelName(localPrisma)returns the real record on the first attempt and we return early — so there is no behavioural change for existing setups.The regex is the same one Prisma's own parser uses for the model declaration head; it doesn't try to understand model bodies (we only need names), and a malformed schema simply yields an empty set, returning
nulland letting the existing empty-mapper path run.Checks
Per
CONTRIBUTING.md:yarn check— clean (no constraint or dedupe issues)yarn testinpackages/internal— 164 passing, including the existinggraphqlCodeGen.test.tssuiteyarn buildinpackages/internal— clean (ESM + CJS)yarn lint— clean (via pre-commit hook)No new dependencies (
getSchemaPathis already exported from@cedarjs/project-config, which@cedarjs/internalalready depends on). Verified end-to-end on a real Prisma 7 project: codegen now emits correct mappers andyarn cedar type-checkpasses against the same codebase that was previously producing 300+ errors.