-
Notifications
You must be signed in to change notification settings - Fork 639
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
Poll - What would you like to see in the future versions of objection? #1302
Comments
I concede the following is quite niche so feel free to shoot it down :) One thing I've always missed (and due to that I've implemented myself) is functionality similar to JPA's @Embeddable/@Embedded for logical grouping of related fields in a single table. If this sounds like something you might want to include in |
@tonisostrat I'll try not to shoot down anything or add my own opinions here too early to not affect the discussion. |
This may be more on the knex-level, but being able to selectively run seeds would be nice. Also maybe seeing tools like https://github.com/Vincit/knex-db-manager, https://github.com/Vincit/objection-db-errors, and https://github.com/Vincit/objection-find become a part of the core or have better visibility and code examples in the docs, to help folks write less boiler plate initially. Perhaps those features could be toggled via feature flags by the developer(s). |
+1 to @heisian 's note for at least https://github.com/Vincit/objection-db-errors (only b/c I'm not familiar with the other 2; thanks for sharing those links, good weekend reading :) ). My coworkers and I have started to default to pulling in that plugin on our projects, really appreciate the extra support on error handling. No opinion / suggestion (helpful or otherwise) on where to go with that, but lowering the barrier to entry for that utility somehow I think would be a really solid addition. (ps Objection is ill! Thanks for all your work here (and for polling for feedback on v2; looking forward to it)) |
Would a @column('columnName') property decorator be appropriate? You'd use it to decorate a property of a model and the default (or base class) columnNameMapper would consult this property when performing the mapping. e.g. class Singer extends Model {
static table = 'singer';
@column('pseudonym')
name: string;
} would yield code models with a |
Currently, I work with custom But I don't ask to implement the management of that at all; I would like however, a way to generate the ordering from relationship graphs, for example. I also agree with the naming, and would like to see custom behavior for |
I would like:
|
Support for for await (let person of People.query()) {
...
} You can kinda get this by using knex stream interface, as Readable streams support asyncIterator since v10.0.0. Though when I did this I had to use |
This comment has been minimized.
This comment has been minimized.
In a way, you're right. But I don't like how migrations separate the creation of tables from the Model object itself. It's perhaps a bit more cumbersome, but it's sure convenient! Regardless, I should probably move to the knex migration way, I think the support for MS SQL is improved now, and it's way better for my own sanity. Thanks! |
I have a handful of thoughts. Given that objection is overall fairly stable and feature-complete, I think the focus should be on using a breaking change (i.e. v2.0) to tighten things up: focus on naming, making any notable alterations to queries, fixing API warts, work on related tooling. You know, things that are worth considering only when there isn't a ton of debt to pay off. It's my impression that there isn't! I'll just list some ideas, and not so much propose specific solutions, although I'd be happy to talk about solutions if we want to peel-off some additional github issues.
|
@devinivy Thanks! You have alot of great ideas there.
This will be fixed at least for the class Pet extends Model {
static get modifiers() {
return {
onlySpecies: (query, species) => query.where('species', species)
}
}
}
Person
.query()
.eager('[pets, children.pets(onlyDogs)]', {
onlyDogs: query => query.modify('onlySpecies', 'dogs')
}) That's still pretty verbose, but it allows you to "bind" arguments to existing modifiers. What do you think? Could that be enough? Another option would be a mechanism like this: Person
.query()
.eager('[pets, children.pets(onlyDogs)]')
.bindModifier('onlySpecies', 'onlyDogs', 'dogs') but is that really better? It intruduces yet another concept. We could also add helpers like these: Person
.query()
.eager('[pets, children.pets(onlyDogs)]', {
onlyDogs: bindModifier('onlySpecies', 'dogs')
}) @devinivy Do you have any other ideas for this? I've also thought about making modifiers query-wide, so that they could be used with Person
.query()
.eager('[pets, children.pets(onlyDogs)]', {
onlyDogs: query => query.modify('onlySpecies', 'dogs')
}) you'd say this: Person
.query()
.eager('[pets, children.pets(onlyDogs)]'),
.modifiers({
onlyDogs: query => query.modify('onlySpecies', 'dogs')
})
This is a tricky one. I also hate that this happens. That's one of the reasons why I'll deprecate the Fixing this would again require objection to duplicate the knex API which I've been trying to avoid. We would need to wrap all If you have any good ideas, I'm all ears. One thing we could do is add a class Person extends Model {
static modifiers = {
boys: query => {
const ref = Person.ref
query.where(ref('gender'), 'male').where(ref('age'), '<', 18)
}
}
}
This is another complete design flaw I'll be addressing. We can use
The
// Fetch
await Person.query().findById(1)
.relatedQuery('pets').where('name', 'Fluffy');
// Unrelate
await Person.query().findById(1)
.relatedQuery('pets').findById(42)
.unrelate(); I agree with the title, but not with either of these examples. They would be confusing. The await Person.fromId(1).$relatedQuery('pets').where('name', 'Fluffy');
await Person.fromId(1).$relatedQuery('pets').findById(42).unrelate()
Will be done. This was also brought up by @heisian
This is pretty vague. I'd like to see more plugins and 3rd party tools, but I don't know how to help there. The Validator interface is one thing, but I don't think that is the biggest limiting factor right now. |
It's only a small feature, but how about a method called To see why this is useful, consider the following function. async function f(user) {
const u = user.$clone();
await u.$loadRelatedUnlessLoaded('favouriteBook.author');
// Do something with u.favouriteBook.author
} I want to be able to pass a
|
@phormio That's a good idea 👍 That has been requested before.
await u.$fetchGraph('favoriteBook.author', { reloadExisting: false }) |
@koskimas thanks for the thoughtful response 🙏
I like the example you gave, and I think that's totally fine! I like the
I agree, I think this is a really hard problem to solve and it's not easy to justify the effort. Going down that path would generate a lot of questions that I don't have answers to. I would be afraid of making the generated queries harder to control and more opaque. Making
Would
I'll look into it!
Fair! I don't think these two options are mutually exclusive, and the example you gave is totally straightforward / would be a nice improvement. At the same time it does feel a little "off" to me that in order to build some queries I need to create a model instance with some fields filled-in. It works, but it doesn't seem super objection-y (obviously this is just a matter of opinion—whatever you decide is "objection-y" by definition!) . I still think an API similar to what I proposed would be in the spirit of objection as it relates to query laziness and composability. Does it seem more reasonable if it's re-written in steps? /* select * from users where type = 'musician' */
const musicians = User.query().where('type', 'musician');
/* select * from animals where
type = 'dog' and
"ownerId" in (select "ownerId" from users where type = 'musician') */
const musiciansDogs = musicians.relatedQuery('pets').where('type', 'dog');
/* update animals set "ownerId" = null where
type = 'dog' and
"ownerId" in (select "ownerId" from users where type = 'musician') */
const unrelateMusiciansFromDogs = musiciansDogs.unrelate();
// An alternative idea, just for fun.
const musicians = User.query().where('type', 'musician');
const musiciansDogs = User.relatedQuery('pets').per(musicians).where('type', 'dog');
const unrelateMusiciansFromDogs = musiciansDogs.unrelate();
👍
Agree I was pretty vague there. I'm mostly trying to think of things that could fragment efforts to write generic tooling for objection, so that they can be squashed proactively in v2. Maybe there's not a lot in the way! |
This would be such a useful feature to have and reduce many unneeded queries! |
@caiquecastro That's a good idea. @phormio Could you create a separate issue about that feature? That's something we can add after 2.0 since it's not breaking. |
This comment has been minimized.
This comment has been minimized.
A small list I can think about to upvote some suggestions:
This is probably all I can think about, |
What about setting Any downsides/cons? 🤔 |
@vladshcherbin That's a good idea! I'll add that to the 2.0 TODO list. |
I am expecting model instance's ability to detect changes, and provides a I have been using ObjectionJS for Anemic model and it's going well. Now I am implementing Domain-driven model following "Backed By a State Object" pattern suggested by Vaughn Vernon - author of popular DDD books. I am too lazy to switch to Sequelize ORM in one microservice while the rest are still using Objection. |
@hirikarate Sorry, but that will never be added to objection. If you want an ORM like that, objection is probably a very bad match for you. |
Hi, I am going to need the following for a project :- That's my wish list ;) |
Remove Bluebird dependency in favor of native Promise! If you check the composition of objection, Bluebird is not a trivial contributor! https://bundlephobia.com/[email protected] |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
I have a few things now:
Why? Because drivers suck at this. Tedious/MSSQL doesn't have a solution for this. Postgres has a parsing solution, but no encoding solution. SQLite has nothing. Dealing with dates and decimal values is error-prone without some support for this. Typeorm has a transformer option, but supports poorly complex types like
|
You can already do this. See the hook docs.
There is no way to do this with the normal
Not going to happen 😄 All the issues listed here can easily be solved in other "objectiony" ways. Objection is not any other ORM, nor does it try to mimic them just because they are popular.
Already supported. Simply use knex's way to nesting transactions or Answers by @elhigu (an active knex maintainer)
I just wanted to point out that Anyways it is true that there is no common way for all databases to do that and it could probably be implemented as a objection.js plugin since there are already lifecycle methods where mappers could be called.
Objection is a bit like active record in a sense that class presents a table and instance a row, but objection does not create or update object back to database that way (and it has been design decision from the start to prevent having too much magic going on behind the scenes with caching and tracking changes in model attributes).
Knex supports nested transactions through single connection and they have been implemented by using save points. So objection supports them too. Multiple transactions in same pool work also just fine.
If you don't bind base |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
I would like to see in next version of Objection is conditinal clauses like in Laravel Eloquent https://laravel.com/docs/7.x/queries#conditional-clauses This feature will avoid to use if/switch expressions in light queries, like: const searchBooks = (year, searchTerm) => Books
.query()
.when(searchTerm.length, qb => qb.where('title', 'like', `%${searchTerm}%`)
.when(year !== undefined, qb => qb.where('year', '>=', year)
.orderBy('id', 'DESC')
.page(0, 100) |
@asergey87 : you could already use a custom QueryBuilder that adds conditions like: export default class ExtendedQueryBuilder extends QueryBuilder {
when(test, fnif, fnelse) {
return test ? fnif(this) : (fnelse ? fnelse(this) : this);
}
whenClient(name, ...args) {
const not = name[0] === '!';
if(not) {
name = name.slice(1);
}
return this.if(!!(not ^ (this.knex().client.config.client === name)), ...args);
}
}) |
It would be nice to have the option to leverage the ES6 named export syntax, now that Node 14 supports ESM without the experimental flag. This: import { Model, QueryBuilder } from 'objection' is much nicer than: import objection from 'objection'
const { Model, QueryBuilder } = objection |
Hey just chiming in, I saw some talks about unrelate and wanted to give this insight too:
Maybe there are challenges in making this more symmetrical, if so we could explain it in the doc if that's something we can't change. Thanks! |
This comment has been minimized.
This comment has been minimized.
@vvo Unrelate follows the same logic as user.$relatedQuery('teams').unrelate().where('teams.members', '<', 10) or use whatever SQL query you want. You wouldn't be able to do that if we changed the API to take an id. We could add a @vladshcherbin await user.$relatedQuery('teams').unrelate()
await user.$relatedQuery('teams').relate(newIds) but off course that will be slow if each user can have thousands of teams.
await User.query().upsertGraph(
{
id: user.id,
teams: newIds.map(id => ({ id })),
},
{ relate: true, unrelate: true }
) |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Not a specific request, but after using Objection for a couple months I feel like the API offers too many ways to do the same thing. For example, I'm currently trying to figure out the difference between If Objection intentionally provides several different approaches to ORM, it would be nice to make this explicit. Otherwise I'd very much like to see as many methods as possible deprecated. |
@Kinrany I disagree. The difference between these two is pretty clear.
|
Maybe this is more on knex level but I would love to see something like the Sandbox Adapter from ecto (https://hexdocs.pm/ecto_sql/Ecto.Adapters.SQL.Sandbox.html). This allows for pretty painless and fast integration tests. |
|
@koskimas |
Change the api for $fetchGraph. At the moment it accepts an option |
Thank you… will surely look into this… 😊 |
I'm starting to plan the 2.0 release and I'd like to hear the community's opinions what we should add to objection in 2.0 or later down the road.
I'd like to reiterate here that the goal of objection was never to become Eloquent, Django's ORM, Active Record or any other known ORM. Objection's goal is said pretty well at the top of the github README. Objection is a "query builder on steroids" that never get's in the way of SQL, but still attempts to provide tools for working with repetitive stuff and especially relations.
That said, there are many things those ORMs do better than objection and we should try to bring in the features that suit objection's design goal.
SO, I'd like you to post here what you miss from any other tool you've worked with and think objection should really have. Vote the features using 👍 and 👎
Couple of things 2.0 will have:
eager
is used as a verb, even though it's not,joinRelation
implies it joins a single relation.joinEager
sounds likejoinRelation
even though it does a very different thing. These will be something likewithGraphFetched
,joinRelated
,withGraphJoined
. etc. The old names will remain as aliases at least until 3.0.https://github.com/Vincit/objection.js/projects/1
The text was updated successfully, but these errors were encountered: