[RFC] Consolidate connection URL API for adapters

[timleslie]

The way a developer currently specifies a connection string for their database adapter is either as:

{ mongoUri: 'mongodb://...` }

or

{ knexOptions: { connection: 'postgres://...' } }

Furthermore, for mongo there's a list of ~10 environment variables, and ~3 for knex, which are looked if there isn't an explicit connection string specified.

IMHO this is overly confusing and non-obvious. I would like to propose that both adapters (and any future adapters) allow the dev to specify either:

1. A config option called url, or
2. An environment variable called DATABASE_URL.

This will simplify the code, but more importantly simplify the docs, so that a user doesn't have to go digging deep through API docs to work out the exact config option to use.

This will obviously be a breaking change, and I'm not keen to push it through too soon on the heals of the Arcade release, but I want to put it out there as an idea.

The code in this PR is a five minute mock up of approximately what the changes would look like. I haven't actually run/tested it, so don't pick on it too hard :-)

Comments welcomed!

[changeset-bot]

⚠️ No Changeset found

Latest commit: f361cf5

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[timleslie]

This also removes the idea of a default database location. Now that we have the create-keystone-app which sets the dev up with an explicit connection string, there's never a good use case that I can think of where we want keystone to magically connect to a DB based purely on the project name. Hiding the existence of the database behind a slightly magic default doesn't do anyone any favours.

[jesstelford]

I seem to remember discussing this a while back, and one of the reasons we have so many Mongo env vars is because of various deployment environments which make them available automatically (netlify might have been one). We were reaching for maximum compatibility there.

I'm not concerned that each adapter have the exact same interface - they're going to have to be unique at some point for some options given they're very different databases.

I'd prefer to err on the side of making it consistent with the database rather than consistent at the Keystone adapter level. That way a user of keystone can read a random blog or tutorial about Mongo that says "Set your mongoUri to X", and know they can just pass in mongoUri as a config option / search the Keystone docs for mongoUri, etc.

[timleslie]

The choice of variable name (url) and env var name (DATABASE_URL) is partly inspired by Prisma, but also because they make the most sense :-)

[jesstelford]

Am happy to support hose as well, and even set them as the "default". Then accept other options as a "compatibility" layer (which is documented, but a little less obviously).

[timleslie]

Yep, we already support DATABASE_URL as an env var on both adapters 👍

[jesstelford]

We're forward thinking 😉

🤣

[timleslie]

re: supporting deployment environments, my concern is that it is all a bit magical. If a particular deployment provides the URL as an env var then I think it's not too much to ask the developer to explicitly write new Adapter({ url: proces.env.HOSTING_MAGIC_VAR }) rather than us trying to second guess all the different use cases which may or may not exist.

[MadeByMike]

The original intention was only to ever document DATABASE_URL - I think the others were added to docs via a community suggestion that we should have probably pushed back on. They were intended as a legacy since the original Keystone had them. And to make some hosting environments "just work", the need for that I think has diminished.

More important than the environmental variables though is that it can be configured in different ways, on different adapters - this definitely sucks :)

[Vultraz]

Speaking of parity, it's rather odd that the Mongoose adapter accepts Mongoose options directly in the top-level config, whereas the Knex adapter requires an additional knexOptions option. Perhaps the knexOptions option should also be removed and any config options simply be passed through as with the Mongoose adapter?

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/keystonejs/next-keystone-docs/ehj7uc9ml
✅ Preview: Failed

[timleslie]

I'm going to close this out as not-going-to-happen. Not enough benefit from the breaking changes it would require.