Skip to content
This repository has been archived by the owner on Feb 10, 2022. It is now read-only.

Mongorito v3 #138

Closed
vadimdemedes opened this issue Aug 27, 2016 · 32 comments
Closed

Mongorito v3 #138

vadimdemedes opened this issue Aug 27, 2016 · 32 comments

Comments

@vadimdemedes
Copy link
Owner

vadimdemedes commented Aug 27, 2016

Hey everyone, Mongorito's been quiet for a long time now, but it deserves more attention that it has right now. I've been looking into library's code, repository issues and pull requests and decided to roll out a v2 of Mongorito in the coming weeks.

Current state of Mongorito's codebase is not what I want it to be. It is bloated and it does not fit a variety of use cases. Here's a plan of improvements to build into a new major release. Note, that it will not be compatible with the current version, so a migration guide will be provided.

Roadmap

1. No more singleton Mongorito instance

Instead of this:

const mongorito = require('mongorito');
mongorito.connect(url);

We're going to have something like this:

const mongorito = require('mongorito');
const db = mongorito.create(); // or
const db = new mongorito.Database(); // or
const db = new mongorito.Db(); // or
const db = mongorito.connect();

Singleton Mongorito object leads to a dirty code in Mongorito's codebase and makes the whole library less flexible. It makes connecting to multiple databases inconvenient.

2. Drop .extend() and go ES6 only

After the next release, .extend() is no longer to be included in the core. To define a model, you will have to use ES6 classes.

3. Registration of models

Since singletons are to be discontinued (see point nr. 1) and the only way to define a model is through an ES6 class (see previous point), there has to be a way to connect models to databases. In Mongoose you have db.model() method and we'll have something similar:

db.register(Post);
db.register(Comment);

I think this is simple enough and easy to understand what it's for.

4. Export only necessary properties from the native driver

Currently, Mongorito exports every property with a few exceptions from the native driver. I don't think it serves any purpose, as what's most often needed is just ObjectId.

5. Drop generator support in hooks

Since you can define hooks explicitly for before, after and around events, there's no need for flow control via generator functions. Next release will only support "regular" functions and functions that return Promises (async functions).

Note, you will still be able to use code like this:

const post = yield Post.find();

This change is related only to defining hooks inside models.

6. Use query building library like mquery

I think it would be wise to not reinvent the wheel and wrap some query building library (like mquery) to handle the queries in Mongorito. Those libraries usually have good command coverage, so I think most people's needs would be satisfied.

Feel free to discuss and add anything to this list.

@mathieug
Copy link

Nice roadmap, do you plan to push the branch v2 anytime soon?

@vinhtq
Copy link

vinhtq commented Aug 31, 2016

Awesome! Mongorito fits perfect in Adonisjs syntax also! Thank for your nice working!

@vadimdemedes
Copy link
Owner Author

It's a WIP, will push the branch asap.

@vadimdemedes vadimdemedes changed the title Mongorito v2 Mongorito v3 Sep 4, 2016
@vadimdemedes
Copy link
Owner Author

@mathieug v3 branch pushed up - https://github.com/vdemedes/mongorito/tree/v3.

@vadimdemedes
Copy link
Owner Author

vadimdemedes commented Sep 10, 2016

Big update! Everything except the following is done:

  • Export only necessary properties from the native driver
  • Per-model and per-database hooks
  • Plugin system
  • New readme

@vadimdemedes
Copy link
Owner Author

Update! Mongorito v3 is ready and now only needs a new readme.

@nervgh
Copy link

nervgh commented Oct 2, 2016

Could I operate with mongorito' predefined fields like

created_at -> createdAt
updated_at -> updatedAt

camel cased fields in v3?

@nervgh
Copy link

nervgh commented Oct 2, 2016

Great job! 😸 When do you plan release v3? Do you release alpha/beta of v3?

@vadimdemedes
Copy link
Owner Author

@nervgh I want to make those field names configurable, I feel like both of those are good defaults.

When do you plan release v3?

I'd rather not give any ETAs. I still want to implement saving of models inside models (that's what MongoDB basically is, nested documents :D) And don't forget the most annoying part - documentation.

Do you release alpha/beta of v3?

For now, you can use v3 branch instead of the package published on npm. Make sure to report any bugs, greatly appreciated! Before the major bump, I'll definitely release v3 with alpha or beta tag.

@nervgh
Copy link

nervgh commented Oct 2, 2016

@vdemedes

For now, you can use v3 branch instead of the package published on npm

Exactly! I will install mongorito from branch v3. Thanks! 😃

I still want to implement saving of models inside models (that's what MongoDB basically is, nested documents :D)

I think my project recursive-iterator may be helpful for your purposes. Using it you could iterate through nested structures very simple.

@nervgh
Copy link

nervgh commented Oct 2, 2016

@vdemedes It looks like I cannot install v3 from branch because:

  1. package.json does not allow it
  2. lib vs src

@vadimdemedes
Copy link
Owner Author

@nervgh package.json allows it, why would you think it wasn't? What error are you getting?

@vadimdemedes
Copy link
Owner Author

OH, I'm not committing lib folder, so it's empty when installed...

@vadimdemedes
Copy link
Owner Author

I think I should get rid of unsupported ES6 syntax and remove babel transpilation of the source.

@vadimdemedes
Copy link
Owner Author

@nervgh Removed babel, now the install should go fine!

@nervgh
Copy link

nervgh commented Oct 2, 2016

@vdemedes thanks!

I need Query.in(key, array) method. How I could run it? It looks like the in method does not exists anymore or it has been deprecated.
What do you think about of next syntax?

let queryBuilder = new MyModel.Query(); // Query has been exported as the static property of Model
queryBuilder.in(key, array);
queryBuilder.sort(key, direction);
// etc
let result = yield queryBuilder.exec();

@vadimdemedes
Copy link
Owner Author

@nervgh Sorry for late response, here's how you can use .in():

const posts = await Post.where('title').in(['Bad title', 'Great title']).find();

@niallobrien
Copy link

niallobrien commented Nov 14, 2016

Great ORM and I'm excited to see continued work on this project.

@vadimdemedes
Copy link
Owner Author

@niallobrien Thank you, means a lot to me! I updated the docs, so v3 release is going to be (hopefully) released soon.

@vinhtq
Copy link

vinhtq commented Nov 17, 2016

Can I suggest a migration feature?

@vadimdemedes
Copy link
Owner Author

@vinhtq Could you elaborate what you mean exactly?

@niallobrien
Copy link

I'd love to see an adapter for Feathers.js

@vinhtq
Copy link

vinhtq commented Nov 28, 2016

@vdemedes Something looks like this: https://github.com/derickbailey/migroose

@zaaack
Copy link
Contributor

zaaack commented Jan 11, 2017

What about static configure method for each model? It's useful when creating indexes or adding global hooks.

@zaaack
Copy link
Contributor

zaaack commented Jan 23, 2017

Since ensureIndex is declared, should mongorito V3 add a new createIndexes method too? http://stackoverflow.com/questions/25968592/difference-between-createindex-and-ensureindex-in-java-using-mongodb

@vadimdemedes
Copy link
Owner Author

vadimdemedes commented Jan 23, 2017

@vinhtq It's out of scope of this library, it's just a database abstraction.

What about static configure method for each model? It's useful when creating indexes or adding global hooks.

@zaaack Might be a good idea (v2 had it), will see what can be done here. Thanks!

Since ensureIndex is declared, should mongorito V3 add a new createIndexes method too?

@zaaack From what I've read on SO link you referenced, ensureIndex is deprecated. I think I'll just remove ensureIndex and use createIndex instead. Thanks for pointing this out!

@zaaack
Copy link
Contributor

zaaack commented Jan 23, 2017

@vadimdemedes Actually insert, remove, update are all declared too by something like insertOne and insertMany... http://mongodb.github.io/node-mongodb-native/2.2/api/Collection.html#insert

@zaaack
Copy link
Contributor

zaaack commented Jan 23, 2017

The static configure method, I find out an implementation by v3's plugin system(I love that! ). Maybe can have some built-in plugins.

db.use(function staticConfigure(model) {
  model.connection().then(conn => {
    // static configure
    model.configure && model.configure()
  })
})

@zaaack
Copy link
Contributor

zaaack commented Feb 26, 2017

Allow sort with array arguments, hash option can't guarantee the order of multi-fields, new driver and mquery support array sort arguments.

https://github.com/aheckmann/mquery/blob/master/lib/mquery.js#L1214

PR: #167

@niallobrien
Copy link

Any plans to move to async/await?

@vadimdemedes
Copy link
Owner Author

@niallobrien Mongorito v3 (and legacy v2) already returns promises for all methods where async operations happen. You can use async/await on any of them.

@vadimdemedes
Copy link
Owner Author

Since v3 is out, closing this issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants