Skip to content
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

[breaking] upgrade Prisma v2.16.0 and replace rw db commands with rw prisma #1720

Merged
merged 19 commits into from
Feb 6, 2021

Conversation

thedavidprice
Copy link
Contributor

@thedavidprice thedavidprice commented Feb 3, 2021

  • Upgrades Prisma v2.16.0
  • Deprecates Redwood DB commands
  • Add new Redwood Prisma commands
    • Add tests
  • Renames template/api/db/seeds.js --> seed.js
  • Updated and passing E2E tests

Breaking

This upgrade introduces the new Prisma Migrate:

  • breaks yarn rw db commands
  • breaks schema.prisma Dyanmic Provider (fully deprecated)
  • breaks default deployment command set

Replace @prisma/cli package

This PR also replaces the package @prisma/cli with prisma. See the related Prisma Release Note.

Prisma Release Notes

@thedavidprice thedavidprice added next release test release:breaking This PR is a breaking change labels Feb 3, 2021
@github-actions
Copy link

github-actions bot commented Feb 3, 2021

@thedavidprice
Copy link
Contributor Author

OK, this is most likely going to require upgrading commands at the same time we upgrade Prisma version. I'll coordinate with @peterp directly.

@peterp
Copy link
Contributor

peterp commented Feb 4, 2021

@thedavidprice I've now added the pass through command and the deprecation notices; you can test this by:

  1. making sure build:watch is running.
  2. running this command:
pwd
~/gh/redwoodjs/redwood/packages/cli
__REDWOOD__CONFIG_PATH=../../__fixtures__/example-todo-main yarn node dist/index.js db up

Deprecation notice
The rw db up command is now deprecated, please try rw prisma up instead.

Migrations are now available via the rw prisma migrate command.

Please view the migration docs

The copy probably needs some work, but it seems to be working nicely. :)

@thedavidprice thedavidprice changed the title Upgrade Prisma to v2.16.0 [breaking] [breaking] upgrade Prisma v2.16.0 and replace rw db commands with rw prisma Feb 5, 2021
@@ -1,24 +1,15 @@
// helper used in Dev and Build commands
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

db generate had specific logic for use with Build and Dev commands. I converted to a helper and moved to commands/prisma/generate.js. Perhaps best to move into prisma.js? Suggestions otherwise?

@thedavidprice
Copy link
Contributor Author

thedavidprice commented Feb 5, 2021

@peterp I have all the commands working now. (Some are more elegant than others.) Input and changes are 100% welcome.

Problems

  1. Cannot pass through flags for specific Prisma commands:
    • E.g. yarn rw prisma migrate dev --name migration-name fails because Yargs tries to understand the --name flag
    • Note: I've added an initial hacky workaround for only migrate dev to handle --name automatically and accept --create-only when passed by the user.

Questions

  1. Should we add tests to Prisma commands? ((can use previous DB Command tests as reference)
  2. We need to figure out the DX for migrate resolve, which potentially is interactive and requests the connection string when run. The other two commands like this are introspect and migrate status:
    • thoughts about how to handle these, either with interactivity or custom options?
    • I don't think it's a priority we need to have custom, interactive support for introspect
  3. Should we pass a default Prisma migration name with the option to customize like we did with rw db save? That would be my preference. Had to add this as initial workaround to Problem 1 above.
  4. How can we improve --help output? Looking at this again, I'd really like:
    • rw prisma --help to output yarn prisma --help and
    • rw prisma db push --help (for example) to output yarn prisma db push --help

Improvements:

  • [priority] currently yargs --help always displays for the top-level command and all other commands; ideally yargs default would only display for the top-level and otherwise display would be Prisma CLI help for specific commands E.g. yarn rw prisma db seed --help
  • aliases, E.g. dev for migrate dev or push for db push (probably would need some switchin')

@thedavidprice
Copy link
Contributor Author

thedavidprice commented Feb 5, 2021

@peterp @cannikin I think I'm over my head regarding the failing tests. My best guess is that Prisma has changed DMMF behavior under the hood without any heads up. Here's a screenshot from the first test that fails:
Screen Shot 2021-02-05 at 9 05 01 AM

All roads (that I've traveled) lead back to packages/cli/src/lib/index.js

...
export const getSchemaDefinitions = async () => {
  const schemaPath = path.join(getPaths().api.db, 'schema.prisma')
  const metadata = await getDMMF({
    datamodel: readFile(schemaPath.toString()),
  })

  return metadata
}
...

So, ahem, what is the Prisma Schema DMMF? 🤔

Note: The Scaffold generator does work correctly when manually tested

@thedavidprice
Copy link
Contributor Author

@cannikin is my HERO 😍

Screen Shot 2021-02-05 at 2 37 54 PM

@thedavidprice thedavidprice merged commit c705c57 into main Feb 6, 2021
@thedavidprice thedavidprice deleted the dsp-upgrade-prisma-v2.16 branch February 6, 2021 00:21
@thedavidprice thedavidprice added this to the v0.25.0 milestone Feb 6, 2021
dac09 added a commit to dac09/redwood that referenced this pull request Feb 9, 2021
…ender-p1

* 'main' of github.com:redwoodjs/redwood:
  Exclude --help, leave everything else.
  Fix yargs command.
  Specify node_env before running build web
  Prisma migrate dev handles schema path with spaces
  Dynamically add args as options.
  This is not a TS package.
  Move generate command into lib.
  check if flags exist (redwoodjs#1746)
  use new prisma command (redwoodjs#1745)
  remove dynamic provider and related copy (redwoodjs#1743)
  [breaking] upgrade Prisma v2.16.0 and replace `rw db` commands with `rw prisma` (redwoodjs#1720)
  MockProvider: Wrap children in LocationProvider
  Adds Facebook to list of supported Supabase OAuth providers (redwoodjs#1739)
  Specify types for the router package
  bugfix: don't pass cell children to useQuery
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:breaking This PR is a breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants