sqlmancer
sqlmancer copied to clipboard
Add support for multiple clients
The @sqlmancer
directive could accept a list configuration objects instead of just one, allowing the generation of multiple database clients. This would be useful for projects that utilize more than one database. A name
option would be necessary to distinguish between clients and identify which client (or clients) a particular model belongs to.
The function createSqlmancerClient
currently only receives a single knex
instance and return it augmented with the models. This should be the other way around, having the models returned augmented with their respective knex
instances. This would allow passing multiple, scoped, knex
instances. For example :
const pgKnex = Knex({
client: 'pg',
connection: process.env.PG_CONNECTION_STRING,
});
const myKnex = Knex({
client: 'mysql',
connection: process.env.MY_CONNECTION_STRING,
});
const models = createSqlmancerClient('./src/**/*.graphql', {
pg: pgKnex
mysql: myKnex
});
In order to specify each model's connection, the @model
directive should allow specifying the named knex
instance. For example :
type Actor @model(db: "pg", table: "actor", pk: "id") {
id: ID!
firstName: String!
lastName: String!
}
type Movie @model(db: "mysql", table: "movies", pk: "id") {
id: ID!
name: String!
year: Number!
genre: MovieGenreEnum!
}
Therefore :
models.Actor.knex === pgKnex; // true
models.Movie.knex === myKnex; // true
@yanickrochon I think the above suggested changes to the API make perfect sense. However, I don't think consumers of the API should have to go through a particular model to access the Knex instance. In other words, I should be able to do:
client.mysql.transaction(trx => { ... })
instead of
models.Actor.knex.transaction(trx => { ... })
So the typing would look something like:
type SqlmancerClient = {
[key: string]: Knex & { models: Models }
}
If a single Knex instance is passed in instead of a map, we can default to using knex
or db
as the key.
I'm not opposed to exposing the underlying Knex instance as a property on each individual model -- but I don't think the relationship between the connection and its models should be completely inverted.
Sure! The point was to expose each database connections. So, what you're suggesting is fine with me also. But I fail to see why createSqlmancerClient
should return the connections if they are already provided as second argument. That is, this does not make much sense :
const db = {
pg: pgKnex
mysql: myKnex
};
const client = createSqlmancerClient('./src/**/*.graphql', db);
// client.db === db
What is actually relevant are the models. Something like :
const db = {
pg: pgKnex
mysql: myKnex
};
const models = createSqlmancerClient('./src/**/*.graphql', db);
export {Â db, models };
In short, whatever db
is, if it's a Knex
instance, then all models will make use of it, otherwise it will be an object defining named connections. No need to define a map with some default properties, the whole will be user-specific.
@yanickrochon Just to provide some context: the "client" returned by createSqlmancerClient
is meant to be an object that includes the defined models and also exposes the methods provided by the underlying Knex instance. At the moment, we're just assigning the models as a property of the Knex instance, but the "client" object is really intended to function as a proxy for those methods. This was an intentional design choice stemming from potentially needing to expose other properties or methods in the future that are not model-specific.
We could just have a createSqlmancerModels
function, which is effectively what you're proposing. But I think I'd like to maintain the concept of a "client" that we could tack additional properties onto if needed. In that context, each connection/Knex instance would have its own named client, with its own set of models and whatever other methods.
However, the current design, along with issue #122, introduces a chicken and the egg paradox, namely that creating the models from an already defined schema, including the typeDefs
and resolvers
, and returning the models
, does create a problem of module interdependence.
- the
models
are used in theresolvers
- the
models
are created from theresolvers
This is why I was suggesting keeping the models inside an object exposed by SQL Mancer, something like
// resolver.js
import { models }Â from 'sqlmancer';
export default {
Query: {
films: (root, args, ctx, info) => models.Film.findMany().resolveInfo(info).execute()
}
};
// setup.js
const typeDefs = loadTypeDefs();
const resolvers = loadResolvers();
const schema = makeSqlmancerSchema({ typeDefs, resolvers });
const db = { ... };
const client = createSqlmancerClient(schema, db);
However, one way of simplifying this would be to change the way this function is called, and provide a way to pass optional arguments, for example
interface SqlmancerClientOptions {
glob: String,
db: Knex | Object<String,Knex>,
typeDefs: Object,
resolvers: Object,
schema: Object
}
function createSqlmancerClient<T extends GenericSqlmancerClient = GenericSqlmancerClient>(options: SqlmancerClientOptions);