From 215aed387d35e9d4c896fe76991b12b54789cc55 Mon Sep 17 00:00:00 2001 From: Mitchell Hamilton Date: Fri, 19 Mar 2021 08:18:24 +1000 Subject: [PATCH] Use Prisma's programmatic API for keystone-next generate (#5155) --- .changeset/pink-eggs-drop.md | 10 ++ .changeset/slow-penguins-juggle.md | 7 + docs-next/pages/guides/cli.mdx | 13 +- packages-next/keystone/src/scripts/index.ts | 26 +++- .../keystone/src/scripts/migrate/generate.ts | 33 +++-- packages-next/types/src/core.ts | 7 +- packages/adapter-prisma/package.json | 2 - .../adapter-prisma/src/adapter-prisma.d.ts | 6 + packages/adapter-prisma/src/adapter-prisma.js | 16 +- packages/adapter-prisma/src/index.js | 1 - packages/adapter-prisma/src/index.ts | 3 + packages/adapter-prisma/src/migrations.ts | 138 ++++++++++++++++++ yarn.lock | 7 - 13 files changed, 216 insertions(+), 53 deletions(-) create mode 100644 .changeset/pink-eggs-drop.md create mode 100644 .changeset/slow-penguins-juggle.md create mode 100644 packages/adapter-prisma/src/adapter-prisma.d.ts delete mode 100644 packages/adapter-prisma/src/index.js create mode 100644 packages/adapter-prisma/src/index.ts diff --git a/.changeset/pink-eggs-drop.md b/.changeset/pink-eggs-drop.md new file mode 100644 index 00000000000..e9add69919e --- /dev/null +++ b/.changeset/pink-eggs-drop.md @@ -0,0 +1,10 @@ +--- +'@keystone-next/keystone': minor +'@keystone-next/adapter-prisma-legacy': minor +--- + +Changed `keystone-next generate` so that it uses Prisma's programmatic APIs to generate migrations and it accepts the following options as command line arguments or as prompts: + +- `--name` to set the name of the migration +- `--accept-data-loss` to allow resetting the database when it is out of sync with the migrations +- `--allow-empty` to create an empty migration when there are no changes to the schema diff --git a/.changeset/slow-penguins-juggle.md b/.changeset/slow-penguins-juggle.md new file mode 100644 index 00000000000..d7d93dbd1c8 --- /dev/null +++ b/.changeset/slow-penguins-juggle.md @@ -0,0 +1,7 @@ +--- +'@keystone-next/keystone': major +'@keystone-next/types': major +'@keystone-next/adapter-prisma-legacy': major +--- + +Removed `createOnly` migration mode diff --git a/docs-next/pages/guides/cli.mdx b/docs-next/pages/guides/cli.mdx index 3770fc4d56d..4ff15f2f25c 100644 --- a/docs-next/pages/guides/cli.mdx +++ b/docs-next/pages/guides/cli.mdx @@ -65,12 +65,19 @@ The `build` command is used to generate a built version of Admin UI Next.js appl #### keystone-next reset This command invokes `prisma migrate reset` to reset your local database to a state consistent with the migrations directory. -Use this command before running `keystone-next generate` to ensure that a valid migration is created. +You generally shouldn't need to use `keystone-next reset` directly unless you want to have a fresh database because `keystone-next dev` and `keystone-next generate` will prompt you to reset your database when it determines your database is inconsistent with your migrations. #### keystone-next generate -This command will generate a migration schema based on the current state of your database and your Keystone schema. -This command should be run after running `keystone-next reset` and the generated migration artefact should be added to your repository so that it can be shared with other developers and deployed in production. +This command will generate a migration based on the current state of your database and your Keystone schema. +The generated migration artefact should be added to your repository so that it can be shared with other developers and deployed in production. +You should generally use `keystone-next dev` instead of `keystone-next generate` because it will prompt you to create a migration when you need to. +You only need to use `keystone-next generate` when you want to generate an empty migration to do something that isn't a result of a schema change or generate a migration in a non-interactive environment. +`keystone-next generate` accepts the following options as command line arguments or as prompts: + +- `--name` to set the name of the migration +- `--accept-data-loss` to allow resetting the database when it is out of sync with the migrations +- `--allow-empty` to create an empty migration when there are no changes to the schema #### keystone-next deploy diff --git a/packages-next/keystone/src/scripts/index.ts b/packages-next/keystone/src/scripts/index.ts index 522ffe29fb1..7460f20c039 100644 --- a/packages-next/keystone/src/scripts/index.ts +++ b/packages-next/keystone/src/scripts/index.ts @@ -10,9 +10,10 @@ import { reset } from './migrate/reset'; export type StaticPaths = { dotKeystonePath: string; projectAdminPath: string }; +const commands = { prototype, dev, start, build, deploy, generate, reset }; + function cli() { - const commands = { prototype, dev, start, build, deploy, generate, reset }; - const { input, help } = meow( + const { input, help, flags } = meow( ` Usage $ keystone-next [command] @@ -27,10 +28,17 @@ function cli() { reset reset the database (this will drop all data!) generate generate a migration deploy deploy all migrations - ` + `, + { + flags: { + allowEmpty: { default: false, type: 'boolean' }, + acceptDataLoss: { default: false, type: 'boolean' }, + name: { type: 'string' }, + }, + } ); const command = input[0] || 'prototype'; - if (!(command in commands)) { + if (!isCommand(command)) { console.log(`${command} is not a command that keystone-next accepts`); console.log(help); process.exit(1); @@ -42,7 +50,15 @@ function cli() { const projectAdminPath = path.join(dotKeystonePath, 'admin'); const staticPaths: StaticPaths = { dotKeystonePath, projectAdminPath }; - commands[command as keyof typeof commands](staticPaths); + if (command === 'generate') { + generate(staticPaths, flags); + } else { + commands[command](staticPaths); + } +} + +function isCommand(command: string): command is keyof typeof commands { + return command in commands; } cli(); diff --git a/packages-next/keystone/src/scripts/migrate/generate.ts b/packages-next/keystone/src/scripts/migrate/generate.ts index 2ba57d406ef..deb0932f21e 100644 --- a/packages-next/keystone/src/scripts/migrate/generate.ts +++ b/packages-next/keystone/src/scripts/migrate/generate.ts @@ -1,25 +1,26 @@ +import path from 'path'; +import { + CLIOptionsForCreateMigration, + createMigration, +} from '@keystone-next/adapter-prisma-legacy'; import { createSystem } from '../../lib/createSystem'; import { initConfig } from '../../lib/initConfig'; import { requireSource } from '../../lib/requireSource'; -import { saveSchemaAndTypes } from '../../lib/saveSchemaAndTypes'; import { CONFIG_PATH } from '../utils'; import type { StaticPaths } from '..'; -export const generate = async ({ dotKeystonePath }: StaticPaths) => { - console.log('🤞 Migrating Keystone'); - +export const generate = async ( + { dotKeystonePath }: StaticPaths, + args: CLIOptionsForCreateMigration +) => { const config = initConfig(requireSource(CONFIG_PATH).default); - const { keystone, graphQLSchema, createContext } = createSystem( - config, - dotKeystonePath, - 'createOnly' - ); - - console.log('✨ Generating graphQL schema'); - await saveSchemaAndTypes(graphQLSchema, keystone, dotKeystonePath); + const { keystone } = createSystem(config, dotKeystonePath, 'none'); - console.log('✨ Generating migration'); - await keystone.connect({ context: createContext().sudo() }); - - await keystone.disconnect(); + await keystone.adapter._generateClient(keystone._consolidateRelationships()); + await createMigration( + config.db.url, + keystone.adapter.prismaSchema, + path.resolve(keystone.adapter.schemaPath), + args + ); }; diff --git a/packages-next/types/src/core.ts b/packages-next/types/src/core.ts index dccc31ba892..b8d0a3a67c5 100644 --- a/packages-next/types/src/core.ts +++ b/packages-next/types/src/core.ts @@ -67,9 +67,4 @@ export function getGqlNames({ }; } -export type MigrationMode = - | 'none-skip-client-generation' - | 'none' - | 'createOnly' - | 'dev' - | 'prototype'; +export type MigrationMode = 'none-skip-client-generation' | 'none' | 'dev' | 'prototype'; diff --git a/packages/adapter-prisma/package.json b/packages/adapter-prisma/package.json index b97fbb0a7ca..711044c0176 100644 --- a/packages/adapter-prisma/package.json +++ b/packages/adapter-prisma/package.json @@ -19,8 +19,6 @@ "@sindresorhus/slugify": "^1.1.0", "@types/prompts": "^2.0.9", "chalk": "^4.1.0", - "cuid": "^2.1.8", - "prisma": "2.18.0", "prompts": "^2.4.0" }, "repository": "https://github.com/keystonejs/keystone/tree/master/packages/adapter-prisma" diff --git a/packages/adapter-prisma/src/adapter-prisma.d.ts b/packages/adapter-prisma/src/adapter-prisma.d.ts new file mode 100644 index 00000000000..3499701ae33 --- /dev/null +++ b/packages/adapter-prisma/src/adapter-prisma.d.ts @@ -0,0 +1,6 @@ +// preconstruct(kinda rightfully) gets annoying if you have an import from a ts file +// to a non-ts file in the same package with no declarations so we need this + +export const PrismaAdapter: any; +export const PrismaListAdapter: any; +export const PrismaFieldAdapter: any; diff --git a/packages/adapter-prisma/src/adapter-prisma.js b/packages/adapter-prisma/src/adapter-prisma.js index 11d077763ae..377b40e8656 100644 --- a/packages/adapter-prisma/src/adapter-prisma.js +++ b/packages/adapter-prisma/src/adapter-prisma.js @@ -1,7 +1,5 @@ import fs from 'fs'; import path from 'path'; -import { execSync } from 'child_process'; -import cuid from 'cuid'; import { getGenerator, formatSchema } from '@prisma/sdk'; import { BaseKeystoneAdapter, @@ -57,13 +55,6 @@ class PrismaAdapter extends BaseKeystoneAdapter { } } - _runPrismaCmd(cmd) { - return execSync(`yarn prisma ${cmd} --schema "${this.schemaPath}"`, { - env: { ...process.env, DATABASE_URL: this._url() }, - encoding: 'utf-8', - }); - } - async deploy(rels) { // Apply any migrations which haven't already been applied await this._prepareSchema(rels); @@ -112,16 +103,15 @@ class PrismaAdapter extends BaseKeystoneAdapter { if (this.migrationMode === 'prototype') { // Sync the database directly, without generating any migration await runPrototypeMigrations(this._url(), prismaSchema, path.resolve(this.schemaPath)); - } else if (this.migrationMode === 'createOnly') { - // Generate a migration, but do not apply it - this._runPrismaCmd(`migrate dev --create-only --name keystone-${cuid()} --preview-feature`); } else if (this.migrationMode === 'dev') { // Generate and apply a migration if required. await devMigrations(this._url(), prismaSchema, path.resolve(this.schemaPath)); } else if (this.migrationMode === 'none') { // Explicitly disable running any migrations } else { - throw new Error(`migrationMode must be one of 'dev', 'prototype', 'createOnly', or 'none`); + throw new Error( + `migrationMode must be one of 'dev', 'prototype', 'none-skip-client-generation', or 'none` + ); } } diff --git a/packages/adapter-prisma/src/index.js b/packages/adapter-prisma/src/index.js deleted file mode 100644 index ada3115c6e1..00000000000 --- a/packages/adapter-prisma/src/index.js +++ /dev/null @@ -1 +0,0 @@ -export { PrismaAdapter, PrismaListAdapter, PrismaFieldAdapter } from './adapter-prisma'; diff --git a/packages/adapter-prisma/src/index.ts b/packages/adapter-prisma/src/index.ts new file mode 100644 index 00000000000..80b3e352461 --- /dev/null +++ b/packages/adapter-prisma/src/index.ts @@ -0,0 +1,3 @@ +export { PrismaAdapter, PrismaListAdapter, PrismaFieldAdapter } from './adapter-prisma'; +export { createMigration } from './migrations'; +export type { CLIOptionsForCreateMigration } from './migrations'; diff --git a/packages/adapter-prisma/src/migrations.ts b/packages/adapter-prisma/src/migrations.ts index 51f740a2e86..4e7b86538ac 100644 --- a/packages/adapter-prisma/src/migrations.ts +++ b/packages/adapter-prisma/src/migrations.ts @@ -132,6 +132,144 @@ export async function resetDatabaseWithMigrations(dbUrl: string, schemaPath: str }); } +export type CLIOptionsForCreateMigration = { + allowEmpty: boolean; + acceptDataLoss: boolean; + name: string | undefined; +}; + +// TODO: don't have process.exit calls here +export async function createMigration( + dbUrl: string, + prismaSchema: string, + schemaPath: string, + cliOptions: CLIOptionsForCreateMigration +) { + withMigrate(dbUrl, schemaPath, async migrate => { + // see if we need to reset the database + // note that the other action devDiagnostic can return is createMigration + // that doesn't necessarily mean that we need to create a migration + // it only means that we don't need to reset the database + const devDiagnostic = await runMigrateWithDbUrl(dbUrl, () => migrate.devDiagnostic()); + // when the action is reset, the database is somehow inconsistent with the migrations so we need to reset it + // (not just some migrations need to be applied but there's some inconsistency) + if (devDiagnostic.action.tag === 'reset') { + const credentials = uriToCredentials(dbUrl); + if (cliOptions.acceptDataLoss === false) { + console.log(`${devDiagnostic.action.reason} + + We need to reset the ${credentials.type} database "${ + credentials.database + }" at ${getDbLocation(credentials)}.`); + + if (!process.stdout.isTTY) { + console.log( + "We've detected that you're in a non-interactive environment so you need to pass --accept-data-loss to reset the database" + ); + process.exit(1); + } + const confirmedReset = await confirmPrompt( + `Do you want to continue? ${chalk.red('All data will be lost')}.` + ); + console.info(); // empty line + + if (!confirmedReset) { + console.info('Reset cancelled.'); + process.exit(0); + } + } + + // Do the reset + await migrate.reset(); + } + + let { appliedMigrationNames } = await runMigrateWithDbUrl(dbUrl, () => + migrate.applyMigrations() + ); + // Inform user about applied migrations now + if (appliedMigrationNames.length) { + console.info( + `✨ The following migration(s) have been applied:\n\n${printFilesFromMigrationIds( + appliedMigrationNames + )}` + ); + } + // evaluateDataLoss basically means "try to create a migration but don't write it" + // so we can tell the user whether it can be executed and if there will be data loss + const evaluateDataLossResult = await runMigrateWithDbUrl(dbUrl, () => + migrate.evaluateDataLoss() + ); + + // there are no steps to the migration so we need to make sure the user wants to create an empty migration + if (!evaluateDataLossResult.migrationSteps.length && cliOptions.allowEmpty === false) { + console.log('There have been no changes to your schema that require a migration'); + if (process.stdout.isTTY) { + if (!(await confirmPrompt('Are you sure that you want to create an empty migration?'))) { + process.exit(0); + } + } else { + console.log( + "We've detected that you're in a non-interactive environment so you need to pass --allow-empty to create an empty migration" + ); + // note this is a failure even though the migrations are up to date + // since the user said "i want to create a migration" and we've said no + process.exit(1); + } + } + + let migrationCanBeApplied = !evaluateDataLossResult.unexecutableSteps.length; + // see the link below for what "unexecutable steps" are + // https://github.com/prisma/prisma-engines/blob/c65d20050f139a7917ef2efc47a977338070ea61/migration-engine/connectors/sql-migration-connector/src/sql_destructive_change_checker/unexecutable_step_check.rs + // the tl;dr is "making things non null when there are nulls in the db" + if (!migrationCanBeApplied) { + console.log(`${chalk.bold.red('\n⚠️ We found changes that cannot be executed:\n')}`); + for (const item of evaluateDataLossResult.unexecutableSteps) { + console.log(` • Step ${item.stepIndex} ${item.message}`); + } + } + // warnings mean "if the migration was applied to the database you're connected to, you will lose x data" + // note that if you have a field where all of the values are null on your local db and you've removed it, you won't get a warning here. + // there will be a warning in a comment in the generated migration though. + if (evaluateDataLossResult.warnings.length) { + console.log(chalk.bold(`\n⚠️ Warnings:\n`)); + for (const warning of evaluateDataLossResult.warnings) { + console.log(` • ${warning.message}`); + } + } + + console.log(); // for an empty line + + let migrationName = await (() => { + if (cliOptions.name) { + return cliOptions.name; + } + if (process.stdout.isTTY) { + return getMigrationName(); + } + console.log( + "We've detected that you're in a non-interactive environment so you need to pass --name to provide a migration name" + ); + process.exit(1); + })(); + + // note this only creates the migration, it does not apply it + let { generatedMigrationName } = await runMigrateWithDbUrl(dbUrl, () => + migrate.createMigration({ + migrationsDirectoryPath: migrate.migrationsDirectoryPath, + // https://github.com/prisma/prisma-engines/blob/11dfcc85d7f9b55235e31630cd87da7da3aed8cc/migration-engine/core/src/commands/create_migration.rs#L16-L17 + // draft means "create an empty migration even if there are no changes rather than exiting" + draft: true, + prismaSchema, + migrationName, + }) + ); + + console.log( + `✨ A migration has been created at .keystone/prisma/migrations/${generatedMigrationName}` + ); + }); +} + // TODO: don't have process.exit calls here export async function devMigrations(dbUrl: string, prismaSchema: string, schemaPath: string) { withMigrate(dbUrl, schemaPath, async migrate => { diff --git a/yarn.lock b/yarn.lock index 30db77c88da..7b3676ee50f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -12843,13 +12843,6 @@ prism-react-renderer@^1.2.0: resolved "https://registry.yarnpkg.com/prism-react-renderer/-/prism-react-renderer-1.2.0.tgz#5ad4f90c3e447069426c8a53a0eafde60909cdf4" integrity sha512-GHqzxLYImx1iKN1jJURcuRoA/0ygCcNhfGw1IT8nPIMzarmKQ3Nc+JcG0gi8JXQzuh0C5ShE4npMIoqNin40hg== -prisma@2.18.0: - version "2.18.0" - resolved "https://registry.yarnpkg.com/prisma/-/prisma-2.18.0.tgz#b19a2d4a487b8b62759777be55506567af41f6ae" - integrity sha512-03po/kFW3/oGHtnANgZiKYz22KEx6NpdaIP2r4eievmVam9f2+0PdP4x/KSFdMCT6B6VHh+3ILTi2z3bYosCgA== - dependencies: - "@prisma/engines" "2.18.0-34.da6fafb57b24e0b61ca20960c64e2d41f9e8cff1" - process-nextick-args@~2.0.0: version "2.0.1" resolved "https://registry.yarnpkg.com/process-nextick-args/-/process-nextick-args-2.0.1.tgz#7820d9b16120cc55ca9ae7792680ae7dba6d7fe2"