Fix prismaExtension schema folder detection#3746
Conversation
|
|
Hi @duongynhi000005-oss, thanks for your interest in contributing! This project requires that pull request authors are vouched, and you are not in the list of vouched users. This PR will be closed automatically. See https://github.com/triggerdotdev/trigger.dev/blob/main/CONTRIBUTING.md for more details. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe PR modifies Prisma schema handling in the build extension. The schema folder detection now checks the schema directory for additional Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🚩 No test coverage for schema folder detection logic
There are no test files for the Prisma extension (glob for *prisma*test* and *prisma*spec* returned no results). Given the complexity of the three-way detection logic (isMultiFileSchema vs usingSchemaFolder vs single-file) and the various downstream effects (migrations, TypedSQL, generate commands), this code would benefit from unit tests covering each detection branch. The CLAUDE.md testing guidelines emphasize tests should live beside source files.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const usingSchemaFolder = | ||
| !isMultiFileSchema && dirname(this._resolvedSchemaPath).endsWith("schema"); | ||
| !isMultiFileSchema && | ||
| (await readdir(schemaDir)).some( | ||
| (file) => file.endsWith(".prisma") && file !== basename(this._resolvedSchemaPath) | ||
| ); |
There was a problem hiding this comment.
🔴 Widened usingSchemaFolder detection causes incorrect prismaDir when .prisma siblings exist in prisma/ directly
The new usingSchemaFolder detection on lines 547-551 checks for any sibling .prisma files in the same directory, but the downstream usingSchemaFolder branch at line 640 assumes schema files live in a subdirectory (e.g., prisma/schema/*.prisma) and computes prismaDir = dirname(schemaDir) — going up one level.
When a user has multiple .prisma files directly in prisma/ (e.g., prisma/schema.prisma + prisma/models.prisma, a valid Prisma 6.7+ setup) and specifies schema: "prisma/schema.prisma", the new detection triggers a false positive: schemaDir = <root>/prisma, so prismaDir = dirname(schemaDir) = <root> (the project root). This causes:
- Migrations silently skipped:
migrationsDiris computed asjoin(prismaDir, "migrations")=<root>/migrations(prisma.ts:722), but actual migrations are at<root>/prisma/migrations. TheexistsSynccheck at line 736 fails and logs a warning, but the deployment proceeds without running migrations. - TypedSQL fails:
prismaDirForSqlis computed asdirname(dirname(resolvedSchemaPath))=<root>(prisma.ts:579), so SQL files are looked for at<root>/sql/instead of<root>/prisma/sql/.
The old heuristic (dirname(...).endsWith("schema")) would not have matched this case and correctly fell through to the single-file else branch.
Prompt for agents
The usingSchemaFolder detection now matches any directory containing sibling .prisma files, but the downstream logic (prismaDir = dirname(schemaDir) at line 640, prismaDirForSql = dirname(dirname(...)) at line 579) assumes the schema files are in a subdirectory like prisma/schema/*.prisma — not directly in prisma/.
To fix this, you need to either:
1. Restrict the detection to only match when the schema file is in a subdirectory of the prisma root (i.e., the schema file's parent directory is NOT the typical prisma root). For example, check that schemaDir itself is not the directory containing package.json or prisma config, or that schemaDir's basename is a known schema folder name.
2. Or, adjust the downstream code in the usingSchemaFolder branch to correctly handle the case where schema files live directly in the prisma directory (not in a subdirectory). This would mean prismaDir should be set to schemaDir (not dirname(schemaDir)), and prismaDirForSql should go up only one level.
Option 1 is safer as it preserves the existing downstream assumptions. A possible approach: keep the new sibling-file check but also verify that the schema file is at least 2 levels deep from the project root (i.e., the parent of schemaDir should not be the working directory), or check that schemaDir is a subdirectory of a directory that itself contains other prisma-related artifacts (like migrations/).
Was this helpful? React with 👍 or 👎 to provide feedback.
| commands.push( | ||
| `${binaryForRuntime( | ||
| manifest.runtime | ||
| )} node_modules/prisma/build/index.js generate ${generatorFlags.join(" ")}` // Don't add the --schema flag or this will fail | ||
| )} node_modules/prisma/build/index.js generate --schema=./prisma/schema ${generatorFlags.join( | ||
| " " | ||
| )}` | ||
| ); |
There was a problem hiding this comment.
🚩 The --schema=./prisma/schema flag addition contradicts the removed comment
The old code at line 666 had a comment: // Don't add the --schema flag or this will fail. The PR removes this comment and adds --schema=./prisma/schema. This is likely correct for modern Prisma versions (6.7+ and 5.x with prismaSchemaFolder preview) since the files ARE copied to output/prisma/schema/. However, if any users are on older Prisma versions that don't support --schema with a directory path, this would break. Worth confirming that the minimum supported Prisma version handles this flag correctly.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Fixes #2654. Detect schema folders by contents, not folder name, and pass
--schema=./prisma/schemafor schema-folder builds.