-
Notifications
You must be signed in to change notification settings - Fork 2.1k
feat(config): allow undefined urls in e.g. prisma generate
#28895
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
feat(config): allow undefined urls in e.g. prisma generate
#28895
Conversation
WalkthroughThis PR makes the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
size-limit report 📦
|
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: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/client/tests/e2e/prisma-config-generate-doesnt-need-valid-url/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (26)
packages/cli/src/__tests__/commands/Generate.test.ts(1 hunks)packages/cli/src/__tests__/incomplete-schemas.test.ts(13 hunks)packages/cli/src/utils/loadConfig.ts(2 hunks)packages/client/tests/e2e/prisma-config-generate-doesnt-need-valid-url/README.md(1 hunks)packages/client/tests/e2e/prisma-config-generate-doesnt-need-valid-url/_steps.ts(1 hunks)packages/client/tests/e2e/prisma-config-generate-doesnt-need-valid-url/package.json(1 hunks)packages/client/tests/e2e/prisma-config-generate-doesnt-need-valid-url/prisma.config.ts(1 hunks)packages/client/tests/e2e/prisma-config-generate-doesnt-need-valid-url/src/prisma.config.process-env.ts(1 hunks)packages/client/tests/e2e/prisma-config-generate-doesnt-need-valid-url/src/prisma.config.using-env-helper.ts(1 hunks)packages/client/tests/e2e/prisma-config-generate-doesnt-need-valid-url/src/prisma/schema.prisma(1 hunks)packages/client/tests/e2e/prisma-config-generate-doesnt-need-valid-url/tsconfig.json(1 hunks)packages/config/src/PrismaConfig.ts(1 hunks)packages/config/src/__tests__/env.test.ts(1 hunks)packages/config/src/__tests__/fixtures/loadConfigFromFile/datasource-url-undefined-env/prisma.config.ts(1 hunks)packages/config/src/__tests__/fixtures/loadConfigFromFile/datasource-url-undefined/prisma.config.ts(1 hunks)packages/config/src/__tests__/loadConfigFromFile.test.ts(1 hunks)packages/config/src/env.ts(1 hunks)packages/internals/src/utils/validatePrismaConfigWithDatasource.ts(3 hunks)packages/migrate/src/__tests__/DbDrop.test.ts(1 hunks)packages/migrate/src/__tests__/DbExecute.test.ts(1 hunks)packages/migrate/src/__tests__/DbPush.test.ts(1 hunks)packages/migrate/src/__tests__/MigrateDeploy.test.ts(1 hunks)packages/migrate/src/__tests__/MigrateDev.test.ts(1 hunks)packages/migrate/src/__tests__/MigrateReset.test.ts(1 hunks)packages/migrate/src/__tests__/MigrateResolve.test.ts(1 hunks)packages/migrate/src/__tests__/MigrateStatus.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for new file names (e.g.,query-utils.ts,filter-operators.test.ts)
Avoid creating barrel files (index.tsthat re-export from other modules). Import directly from the source file (e.g.,import { foo } from './utils/query-utils'notimport { foo } from './utils'), unless./utils/index.tsfile already exists
Files:
packages/migrate/src/__tests__/DbDrop.test.tspackages/migrate/src/__tests__/MigrateDev.test.tspackages/cli/src/__tests__/incomplete-schemas.test.tspackages/config/src/env.tspackages/config/src/__tests__/loadConfigFromFile.test.tspackages/config/src/PrismaConfig.tspackages/cli/src/utils/loadConfig.tspackages/config/src/__tests__/env.test.tspackages/migrate/src/__tests__/MigrateDeploy.test.tspackages/migrate/src/__tests__/MigrateResolve.test.tspackages/migrate/src/__tests__/DbPush.test.tspackages/cli/src/__tests__/commands/Generate.test.tspackages/migrate/src/__tests__/DbExecute.test.tspackages/client/tests/e2e/prisma-config-generate-doesnt-need-valid-url/_steps.tspackages/migrate/src/__tests__/MigrateStatus.test.tspackages/client/tests/e2e/prisma-config-generate-doesnt-need-valid-url/src/prisma.config.process-env.tspackages/internals/src/utils/validatePrismaConfigWithDatasource.tspackages/config/src/__tests__/fixtures/loadConfigFromFile/datasource-url-undefined/prisma.config.tspackages/client/tests/e2e/prisma-config-generate-doesnt-need-valid-url/src/prisma.config.using-env-helper.tspackages/config/src/__tests__/fixtures/loadConfigFromFile/datasource-url-undefined-env/prisma.config.tspackages/migrate/src/__tests__/MigrateReset.test.tspackages/client/tests/e2e/prisma-config-generate-doesnt-need-valid-url/prisma.config.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Test files named
*.test.tsare excluded from build output via esbuild config; place tests alongside source files
Files:
packages/migrate/src/__tests__/DbDrop.test.tspackages/migrate/src/__tests__/MigrateDev.test.tspackages/cli/src/__tests__/incomplete-schemas.test.tspackages/config/src/__tests__/loadConfigFromFile.test.tspackages/config/src/__tests__/env.test.tspackages/migrate/src/__tests__/MigrateDeploy.test.tspackages/migrate/src/__tests__/MigrateResolve.test.tspackages/migrate/src/__tests__/DbPush.test.tspackages/cli/src/__tests__/commands/Generate.test.tspackages/migrate/src/__tests__/DbExecute.test.tspackages/migrate/src/__tests__/MigrateStatus.test.tspackages/migrate/src/__tests__/MigrateReset.test.ts
🧬 Code graph analysis (4)
packages/config/src/__tests__/loadConfigFromFile.test.ts (3)
packages/config/src/loadConfigFromFile.ts (1)
loadConfigFromFile(92-168)packages/config/src/index.ts (1)
loadConfigFromFile(5-5)packages/config/src/__tests__/fixtures/loadConfigFromFile/invalid/no-default-export/prisma.config.ts (1)
config(3-5)
packages/cli/src/utils/loadConfig.ts (2)
packages/config/src/index.ts (1)
PrismaConfigEnvError(3-3)packages/internals/src/index.ts (1)
HelpError(15-15)
packages/config/src/__tests__/env.test.ts (2)
packages/config/src/env.ts (2)
env(14-20)PrismaConfigEnvError(1-6)packages/config/src/index.ts (2)
env(3-3)PrismaConfigEnvError(3-3)
packages/internals/src/utils/validatePrismaConfigWithDatasource.ts (1)
packages/config/src/PrismaConfig.ts (3)
PrismaConfig(198-231)Datasource(26-29)SchemaEngineConfigInternal(31-33)
🔇 Additional comments (25)
packages/config/src/PrismaConfig.ts (1)
21-29: LGTM! Type change correctly makesdatasource.urloptional.The change from required to optional for both the Shape definition and TypeScript type is consistent and aligns with the PR objective to allow
prisma generatewithout a valid datasource URL. Command-level validation (as evidenced by updated test expectations across migrate commands) ensures operations requiring database access still enforce the URL requirement.packages/migrate/src/__tests__/MigrateDeploy.test.ts (1)
12-19: LGTM! Test expectation correctly updated to referencedatasource.urlproperty.The updated error message is more specific about the required configuration property, consistent with the broader validation refinements in this PR.
packages/migrate/src/__tests__/DbPush.test.ts (1)
17-24: LGTM! Test expectation correctly updated to referencedatasource.urlproperty.Consistent with validation message updates across other migrate commands.
packages/migrate/src/__tests__/MigrateResolve.test.ts (1)
10-19: LGTM! Test expectation correctly updated to referencedatasource.urlproperty.Aligns with validation refinements across migrate commands.
packages/migrate/src/__tests__/DbExecute.test.ts (1)
33-44: LGTM! Test expectation correctly updated to referencedatasource.urlproperty.Maintains consistency with validation message refinements across migrate commands.
packages/config/src/env.ts (1)
1-6: LGTM! Improved error message clarity.The updated message "Cannot resolve environment variable" is more accurate than "Missing required environment variable" as it covers both missing and empty-string scenarios. The change also improves consistency by adding a trailing period.
packages/config/src/__tests__/env.test.ts (2)
26-31: LGTM! Test expectation correctly updated.The inline snapshot matches the refined error message from
PrismaConfigEnvError.
33-39: LGTM! Test expectation correctly updated.Consistent with the updated error message for unresolvable environment variables.
packages/migrate/src/__tests__/MigrateReset.test.ts (1)
8-17: LGTM! Test expectation correctly updated to referencedatasource.urlproperty.Completes the consistent validation message refinement across all migrate commands.
packages/client/tests/e2e/prisma-config-generate-doesnt-need-valid-url/src/prisma.config.process-env.ts (1)
1-7: LGTM!The test configuration correctly uses
process.env['UNDEFINED_VARIABLE']to test the scenario wheredatasource.urlis undefined. According to the PR objectives,prisma generateshould succeed in this case.packages/client/tests/e2e/prisma-config-generate-doesnt-need-valid-url/src/prisma.config.using-env-helper.ts (1)
1-7: LGTM!The test configuration correctly uses
env('UNDEFINED_VARIABLE')to test the scenario where theenvhelper is used with an undefined variable. According to the PR objectives, this approach should fail with a PrismaConfigEnvError, which provides the expected behavior for required environment variables.packages/client/tests/e2e/prisma-config-generate-doesnt-need-valid-url/package.json (1)
1-16: LGTM!The package.json configuration is appropriate for an e2e test, with all necessary dependencies included. The use of local tarball paths for Prisma packages is standard practice for testing.
packages/client/tests/e2e/prisma-config-generate-doesnt-need-valid-url/src/prisma/schema.prisma (1)
1-35: LGTM!The Prisma schema is well-structured with proper model definitions and relations. The datasource intentionally omits the
urlproperty as it will be provided by the Prisma config file, which aligns with the test's purpose.packages/client/tests/e2e/prisma-config-generate-doesnt-need-valid-url/tsconfig.json (1)
1-4: LGTM!The TypeScript configuration correctly extends the base config and excludes the
_steps.tsfile, which is standard practice for e2e test suites.packages/migrate/src/__tests__/DbDrop.test.ts (1)
14-21: LGTM!The updated error message correctly specifies that
datasource.urlis required rather than the genericdatasource, providing clearer guidance to users. This aligns with the PR's goal of making datasource validation more specific.packages/migrate/src/__tests__/MigrateDev.test.ts (1)
57-65: LGTM!The updated error message correctly specifies that
datasource.urlis required forprisma migrate dev, providing clearer and more actionable feedback. This change is consistent with the broader PR effort to make datasource validation more specific.packages/cli/src/__tests__/commands/Generate.test.ts (1)
19-25: Error message expectation correctly updated todatasource.url.The snapshot now matches the more precise config validation (“The datasource.url property is required…”), which aligns with the new behavior for
prisma generate --sql.packages/cli/src/utils/loadConfig.ts (1)
1-6: Warning diagnostic forPrismaConfigEnvErroris well-scoped and non-breaking.Importing
PrismaConfigEnvErrorand pushing awarndiagnostic only when the inner error matches cleanly augments CLI feedback, while still returning the sameHelpErrorfor the load failure. This keeps behavior stable while exposing the env-resolution detail.Also applies to: 26-38
packages/migrate/src/__tests__/MigrateStatus.test.ts (1)
7-15: Status error snapshot now correctly targetsdatasource.url.The expectation string for
prisma migrate statusis consistent with the new requirement thatdatasource.url(not justdatasource) be present in the Prisma config.packages/config/src/__tests__/fixtures/loadConfigFromFile/datasource-url-undefined/prisma.config.ts (1)
1-7: Fixture correctly modelsdatasource.urlcoming fromprocess.envasundefined.Using
process.env['UNDEFINED_VARIABLE']here is appropriate for exercising the “raw env yields undefined but is accepted” path inloadConfigFromFile.packages/cli/src/__tests__/incomplete-schemas.test.ts (2)
101-197: Env-unset snapshots now matchPrismaConfigEnvErrorwording.All expectations switched from “Missing required environment variable” to “Cannot resolve environment variable: SOME_UNDEFINED_DB.”, which aligns with the new
PrismaConfigEnvErrormessage and the wrapped HelpError text.
213-274: Config-validation snapshots consistently requiredatasource.url.The updated messages for
db push/pull/executeandmigrate reset/devnow explicitly mentionThe datasource.url property is required…, matching the stricter config validation and improving guidance for users.packages/config/src/__tests__/fixtures/loadConfigFromFile/datasource-url-undefined-env/prisma.config.ts (1)
1-7: Fixture correctly targets theenv()-helper failure path.Importing
envand settingdatasource.url: env('UNDEFINED_VARIABLE')is the right setup for tests that expect aConfigLoadErrorwrappingPrismaConfigEnvError.packages/client/tests/e2e/prisma-config-generate-doesnt-need-valid-url/prisma.config.ts (1)
1-8: E2E Prisma config usesdefineConfiganddatasource.urlcorrectly.The config object (file-based SQLite URL plus explicit
schemapath) is well-formed and matches the newprisma/configentrypoint usage expected in the e2e suite.packages/internals/src/utils/validatePrismaConfigWithDatasource.ts (1)
1-1: LGTM: Validation now correctly enforces datasource.url requirement.The changes improve specificity by:
- Requiring
datasource.urlto be a string (not justdatasourceto exist)- Updating the type definition to reflect this requirement
- Providing a clearer error message that references
datasource.urlspecificallyThis aligns with the PR objectives and ensures commands that need database access (e.g.,
prisma db push) properly validate the URL presence, while allowingprisma generateto work without it.Also applies to: 18-20, 23-23, 37-37
packages/client/tests/e2e/prisma-config-generate-doesnt-need-valid-url/_steps.ts
Show resolved
Hide resolved
packages/client/tests/e2e/prisma-config-generate-doesnt-need-valid-url/_steps.ts
Show resolved
Hide resolved
packages/client/tests/e2e/prisma-config-generate-doesnt-need-valid-url/README.md
Show resolved
Hide resolved
|
Hey 👋 This still failing for If a command doesn't require a config thing, it should work when the thing is not available - regardless how the thing is provided (e.g. the The responsibility of deciding what should throw should be the command's. The config loader should do best effort and report (not throw) missing things. |
That's by design. Now, I've already shared with the docs team (in the past 10 days or so) that What this PR does, is to allow the |
But that doesn't matter for commands that don't require or use the config property it was invoked for. Nobody's expecting Sometimes people run these commands without providing the envars they normally would for others. e.g. a GitHub workflow that generates the client and then runs |
This PR:
prisma-config-generate-doesnt-need-valid-urlimport { env } from 'prisma/config''s error message"Missing required environment variable: UNDEFINED_VARIABLE""Cannot resolve environment variable: UNDEFINED_VARIABLE."After this PR:
prisma generateworks when thedatasource.urlvalue in the Prisma config file is undefined:prisma generatestill fails when thedatasource.urlvalue in the Prisma config file uses theenvutility with an undefined variable:The error looks like:
Failed to load config file "[path]/prisma.config.ts" as a TypeScript/JavaScript module. Error: PrismaConfigEnvError: Missing required environment variable: UNDEFINED_VARIABLE.prisma db pushstill fails when thedatasource.urlvalue in the Prisma config file is undefined.The error looks like:
Error: The datasource.url property is required in your Prisma config file when using prisma db push..Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.