{"action":"created","issue":{"url":"https://api.github.com/repos/tgriesser/bookshelf/issues/552","labels_url":"https://api.github.com/repos/tgriesser/bookshelf/issues/552/labels{/name}","comments_url":"https://api.github.com/repos/tgriesser/bookshelf/issues/552/comments","events_url":"https://api.github.com/repos/tgriesser/bookshelf/issues/552/events","html_url":"https://github.com/tgriesser/bookshelf/issues/552","id":52567372,"number":552,"title":"Discussion about next Bookshelf version","user":{"login":"tgriesser","id":154748,"avatar_url":"https://avatars.githubusercontent.com/u/154748?v=3","gravatar_id":"","url":"https://api.github.com/users/tgriesser","html_url":"https://github.com/tgriesser","followers_url":"https://api.github.com/users/tgriesser/followers","following_url":"https://api.github.com/users/tgriesser/following{/other_user}","gists_url":"https://api.github.com/users/tgriesser/gists{/gist_id}","starred_url":"https://api.github.com/users/tgriesser/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tgriesser/subscriptions","organizations_url":"https://api.github.com/users/tgriesser/orgs","repos_url":"https://api.github.com/users/tgriesser/repos","events_url":"https://api.github.com/users/tgriesser/events{/privacy}","received_events_url":"https://api.github.com/users/tgriesser/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/tgriesser/bookshelf/labels/bookshelf-next","name":"bookshelf-next","color":"e11d21"},{"url":"https://api.github.com/repos/tgriesser/bookshelf/labels/discussion","name":"discussion","color":"fad8c7"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":20,"created_at":"2014-12-20T15:37:54Z","updated_at":"2015-07-23T17:21:08Z","closed_at":null,"body":"I figured that now that I have enough code and ideas that I'm starting to get somewhere with the next iteration of Bookshelf I should open a ticket to discuss some ideas I have and see if anyone has thoughts / feedback / ideas / unaddressed pain points as I continue to work on it in the coming weeks.\r\n\r\n### Rationale / Background:\r\n\r\nBookshelf was originally ported heavily from Backbone, to bring some of the ideas of the Model & Collection structure to the server via an ORM. While it's been nice, a lot of the underlying concepts of Backbone are either unnecessary or antithetical to the core of what an good ORM should include. Also, sharing code on the client and server between Bookshelf and Backbone isn't really going to happen to any major degree (beyond ad-hoc handmade integrations). Further, a lot of the code to keep consistent with Backbone has also turned the source into a damn disaster.\r\n\r\nSo now that I've had a chance to step back and evaluate / learn a bit from other ORMs, languages, libraries, etc I feel it's time to refine a decent bit of the core of Bookshelf to really make for the best possible experience out of the box, while providing for a lot of the issues that have come up which we haven't been able to address and simultaneously cutting the codebase in at least half, hopefully more.\r\n\r\n#### Active Record vs Data Mapper\r\n\r\nBookshelf currently, unfortunately, follows an Active Record Pattern. Put simply, it means the backing classes know about the underlying connection. This is a design flaw making things harder to test and ultimately to use in an application. Currently, the initialized bookshelf instance needs to be either created as a singleton cached require module or injected into every file creating a model. #90\r\n\r\nThis isn't sustainable:\r\n\r\n```js\r\nvar bookshelf = require('/path/to/bookshelf-init.js');\r\n\r\nvar User = bookshelf.Model.extend({\r\n  tableName: 'users'\r\n})\r\n```\r\n\r\nrather than just being able to do:\r\n\r\n```js\r\nvar Bookshelf = require('bookshelf');\r\n\r\nvar User = Bookshelf.Model.extend({\r\n  tableName: 'users'\r\n});\r\n```\r\nor with ES6:\r\n\r\n```js\r\nvar Bookshelf = require('bookshelf');\r\n\r\nclass User extends Bookshelf.Model {\r\n   getTableName() {\r\n     return 'users';\r\n   }\r\n}\r\n```\r\n\r\n(ESNext spec doesn't provide for prop definitions on the class body, still thinking about different syntax ideas for some of these things).\r\n\r\nModels would then be constructed by the initialized bookshelf instance, via (long form):\r\n\r\n```js\r\nvar bookshelf = require('bookshelf')(knex);\r\nvar User = require('./models/user')\r\n\r\nbookshelf.createModel(User).where({id: 1}).fetch().then(...\r\n```\r\n\r\nor alternatively:\r\n\r\n```js\r\nbookshelf('user').where({id: 1}).fetch().then(...\r\n\r\nbookshelf('user', {name: 'Tim'}).save().then(...\r\n```\r\n\r\nThe second form would act similar to how the registry plugin does now, allowing one to register specific models on the bookshelf instance, or look them up on a require path (default `process.cwd() + '/models'`)... or just create a new model constructor which assumes that table exists, you just don't want to do any configuration on it (similar to how knex assumes the table exists). If a schema follows the conventions laid out by the library, there's a good possibility you won't need to define any model files.\r\n\r\n#### Collections\r\n\r\nCollections should not be as first class as they are in Bookshelf, (I would argue they shouldn't be as much in Backbone, but I guess with a limited scope of simple REST API calls it doesn't matter as much to most). \r\n\r\nI'm thinking in Bookshelf, the collection will be stripped down measurably, turning into a very simple `ResultSet` object. This will have no purpose other than to iterate the results of a query, and possibly to load extra data onto the result models (though I'm thinking even that might be better placed elsewhere). \r\n\r\nIt won't be possible to add directly to a `ResultSet` anymore, you'll only be able to iterate. If you want to add to it, you'll need to cast as an array and work with it there. Further, you can't lookup based on PK, since ResultSets won't be indexed map-like as they are in Backbone (though it'll be possible to do this on the backing session described below).\r\n\r\nSo, in summary, the heavy collection is pretty much going to go away as is, though it'll be possible to define the target `ResultSet` class if you really need to define something special for handling your results.\r\n\r\nLooking here for any cases where people are using features from the current `Collection` api which seem like they might go away and you won't be able to live without them, to see how we can work on that.\r\n\r\n#### Attributes\r\n\r\nThere does need to be some way of doing prop type validation / casting that can be optionally defined on the model, but ideally we'll be able to gather this meta information on the model and apply/cache it prior to the first async action. This way we'll have the nice getter/setters on the attributes. It'll also be possible for anyone to create their own type definitions and transforms.\r\n\r\n#### Relations\r\n\r\nRelations should be both more configurable and more generic than they are now, currently the lack of ability to define every possible permutation of joins is a weakness, as is the lack of support for such items as foreign keys. Relation methods should take two arguments, the `Target` class and `options`, where flags can be defined constraining the relation (we can figure out what these mean later).\r\n\r\nThe `related` method on a model is also a bit ambiguous, since you can only have a single instance of a `related` model e.g. `user.related('accounts')`, but you can construct multiple `related` models via creating new instances with `user.accounts()`.\r\n\r\nSo I'm thinking there'll be two ways of defining relations, dynamically and statically. The only difference with statically defined relations is they be made available with defineProperty getters on the model, and they will always return the same fetched instance.\r\n\r\nI'm also thinking that on relations, you should be able to do something like:\r\n\r\n```\r\nwithRelated: {\r\n   users: {\r\n     where: {active: true},\r\n     withRelated: {\r\n        posts: {\r\n           where: {moderated: true},\r\n           withRelated: {\r\n             comments: {limit: 5}\r\n           }\r\n        },\r\n        comments: true\r\n     }\r\n   }\r\n}\r\n```\r\n\r\nas a more declarative version of:\r\n\r\n```js\r\nwithRelated: [{'users': function(qb) {\r\n  qb.where('active', true)\r\n}}, {'users.posts': function(qb) {\r\n  qb.where('moderated', true)\r\n}}, 'users.comments', \r\n{'users.posts.comments': function(qb) {\r\n  qb.limit(5);\r\n}}]\r\n```\r\n\r\nThis would address issues like #516\r\n\r\nI have some other ideas about being able to do joins on models so you don't need to do multiple queries to load some relations, especially now that we should know the table schemas prior to querying.\r\n\r\n#### Session\r\n\r\nThe concept of having a backing \"Session\" would be helpful in mitigating the [\"object relational impendence problem\"](http://en.wikipedia.org/wiki/Object-relational_impedance_mismatch) a bit. Here we can keep track of the rows which are fetched and ensure each model has a `===` backing \"row\". This should not be mistaken for a cache, though we could likely fast-path some very simple queries which have been already fully fetched and only lookup based on primary keys. Any visibility modifications defined on a specific model would happen at the model layer (so if two models share the same table/backing row but you're not permitted to set or retrieve a model item on one, that'd be the model's job to handle during the get / set / iterate).\r\n\r\nBecause all the data is fetched to a single location, it should be significantly easier to format everything into something like \"JSON API\" or Transit or whatever you want.\r\n\r\nThe session will also be able to log every query applied over a certain amount of time, which would be awesome if it's tied to the request/response cycle, all of the queries that go into a particular request/response can be easily dumped or displayed in a dashboard or something like that. If anyone has any interest in building a nice UI for that once this gets a little further along let me know, I think it could be a really great debug feature.\r\n\r\n#### Transactions\r\n\r\nTransactions are certainly more cumbersome than they are in knex, particularly since the 0.6 release of knex. They're also a bit dangerous, since in order for them to work reliably, you need to explicitly pass the transacting option to every query you want on the transaction. I think Bookshelf should follow a similar model as knex, where the transaction is bound to the instance:\r\n\r\n```js\r\nbookshelf.transaction(function(bookshelf) {\r\n   return bookshelf('users').fetchAll().then(function(users) {\r\n     return users.accounts().fetch();\r\n   }).then(function(accounts) {\r\n     return P.all(accounts.map(function(account) {\r\n       return account.destroy();\r\n     }))\r\n   })\r\n}).then(function() {\r\n  // transaction success\r\n}).catch(errHandler)\r\n```\r\n\r\n#### Object Lifecycles\r\n\r\nWhen a model is persisted, it should be immediately fetched afterwards by default. In postgres this can be done easily in one hit with `returning *` on pretty much every query. This could be optionally skipped, but in most cases I'm thinking keeping everything in a consistent state will be worth it (and if you don't need that, use knex).\r\n\r\n#### Bulk inserts / deletes / updates:\r\n\r\nThese should be made possible by setting a flag to queue for deletion / save, etc. \r\n\r\n```js\r\nusers.forEach(function(user) {\r\n  if (user.status === 'inactive') users.markDestroyed();\r\n})\r\nusers.getSession().sync().then(function() {\r\n  // delete from users where id in (....)\r\n})\r\n```\r\n\r\nThis should help us cut-down on queries in these situations. Not sure what the best interface would be here, `users.destroy({queue: true})`, `users.destroy({defer: true})`, `users.queueDestroy()`\r\n\r\n#### Soft Deletes\r\n\r\nThese should be available by default. No more hacking around to make this work, it'll just be a thing. One question would be how to deal with the case where you sometimes want to work with deleted items. I'm imagining a `withDeleted` function would be used to indicate you want to fetch / iterate the soft-deleted models.\r\n\r\n```js\r\nmodel.withDeleted(function(model) {\r\n   return model.fetchAll();\r\n}).then(function(models) {\r\n  // all models, even the deleted ones, though to see them \r\n  // in the iterator, you'll need to be within a withDeleted block\r\n})\r\n```\r\n\r\n#### Dialect specific adapters\r\n\r\nSo one thing I mentioned was the fact that we should be able to do things like `returning *`, this would require that we have different backing adapters. With this we should be able to start adding some of the nice new features of PG which have been released recently (and sort of prompted writing this post).\r\n\r\nThis may also eventually open up the potential for integrating with additional databases (though definitely not an immediate concern, it would be pretty cool), at the very least I'm thinking it'll be possible to relate with different relational db's - which would actually be really cool for instances of applications with sharded DB's.\r\n\r\n### Other Thoughts:\r\n\r\n#### Camel Case\r\n\r\n`parse` and `transform` and such are pretty weak, and they're mostly used for camel casing attrs, so we'll just add this as an option, keeping the model as the transform layer for the keys so we don't need to go to and from all the time when coming from the db. Then we'll advertise the parse and transform\r\n\r\n#### Events\r\n\r\nI'm wondering whether events are still the best way to deal with lifecycle actions. I haven't decided that they're going to be kicked entirely but I feel like having lifecycle methods defined on models would be much cleaner, less ad-hoc and easier to reason about. It'd also be simple to just hook the events into the lifecycle events at the top-level if need be.\r\n\r\n### Summary\r\n\r\nI'll add other thoughts as they come up, just wanted to see what anyone thoughts and if there were any major concerns with some of the ideas expressed here. Obviously in order to do something like this, there will be some breaking changes but I think the tradeoff between potential performance, features, and general code quality will heavily lean in favor of a little breakage, particularly if there's a very clear upgrade path. I've also got a sample application in mind to show off some of these features.\r\n\r\nLooking forward to addressing a lot of the flaws that have come up and getting back on track to making Bookshelf the de-facto JS ORM in 2015.\r\n\r\n/cc @tkellen"},"comment":{"url":"https://api.github.com/repos/tgriesser/bookshelf/issues/comments/124171917","html_url":"https://github.com/tgriesser/bookshelf/issues/552#issuecomment-124171917","issue_url":"https://api.github.com/repos/tgriesser/bookshelf/issues/552","id":124171917,"user":{"login":"joepie91","id":1663259,"avatar_url":"https://avatars.githubusercontent.com/u/1663259?v=3","gravatar_id":"","url":"https://api.github.com/users/joepie91","html_url":"https://github.com/joepie91","followers_url":"https://api.github.com/users/joepie91/followers","following_url":"https://api.github.com/users/joepie91/following{/other_user}","gists_url":"https://api.github.com/users/joepie91/gists{/gist_id}","starred_url":"https://api.github.com/users/joepie91/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/joepie91/subscriptions","organizations_url":"https://api.github.com/users/joepie91/orgs","repos_url":"https://api.github.com/users/joepie91/repos","events_url":"https://api.github.com/users/joepie91/events{/privacy}","received_events_url":"https://api.github.com/users/joepie91/received_events","type":"User","site_admin":false},"created_at":"2015-07-23T17:21:08Z","updated_at":"2015-07-23T17:21:08Z","body":"@bkniffler I'd imagine that if models become decoupled from a Bookshelf instance (ie. stateless), that shouldn't be too hard to write a third-party module for, even if it isn't in core."}}