redux-orm icon indicating copy to clipboard operation
redux-orm copied to clipboard

1.0 roadmap

Open plandem opened this issue 7 years ago • 24 comments

Let's discuss what we need to release a v1

  • [x] Keep foreign key identifiers in a separate field instead of overwriting them with relationship descriptors [see #102, #219]
    • [ ] Do this by default unless field.as argument is given? See comment in #219.
  • [ ] Enforce primary key uniqueness [#81]
  • [ ] Add ability to add indexes on columns (other than the ID, which is indexed by default) [#88]
    • [x] Index on foreign keys by default, preventing full-table scans for e.g. book.authors (also need to memoize using the indexes – instance-based memoization would be insufficient)
  • [ ] Improve API regarding foreign keys: People use fk() on one side and many() on the other side. Maybe allow this or provide another intuitive alternative. At least warn them that many() is strictly for many-to-many relationships.
  • [ ] Refactor many-many field declaration to allow forward/backward declaration at each Model:
    • implicit (almost like now, but with ability to declare relation at target model too) or
    • explicit (without any internal magic, cleaner code, easier to support), breaking changes
  • [ ] Simplify (asynchronously) loading initial state [see #228]
  • [ ] Computed fields via attr descriptor? Either store in ref while retaining identity, ~or provide as model-only fields.~ views should not have access to sessions and models! [see #58, #258]
  • [x] Composable and easy-to-use selectors cached by ID attribute [#251]
  • [ ] Up to date documentation! [#261]

what else? @tiii @markerikson @tommikaikkonen @NathanBWaters

Edit by @haveyaseen: Referenced #81, #88, #219, #228, #58, #251, #258… Marked absolute requirements in bold.

plandem avatar Jun 02 '17 16:06 plandem

Up to date documentation! 😉

Keep relation ids in a seperate field instead of overloading it with the relation [#102] Computed fields via attr descriptor?

And since i guess everybody using redux-orm is doing implementing this in some way: A simple API for denormalizing (including computed fields / relations etc.)?

tiii avatar Jun 02 '17 23:06 tiii

@tiii +1 for the documentation and api for denormalizing (including computed fields / relations). Great to see that this library is getting active again!

nealoke avatar Jun 03 '17 07:06 nealoke

What do you mean denormalization API in our case?!

On 3 Jun 2017, 10:36 +0300, Neal van der Valk [email protected], wrote:

@tiii +1 for the documentation and api for denormalizing (including computed fields / relations). Great to see that this library is getting active again! — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

plandem avatar Jun 03 '17 10:06 plandem

@plandem I understood it as this.

Currently when you want to get something out of the orm in a selector with it's relations (in this case questions) you need to do the following.

export const getAccounts = createSelector(orm, getOrm, (session) => {
	const accounts = session.Account.all().toModelArray();
	
	return accounts.map(accountModel => ({
			...accountModel.ref,
			questions: accountModel.questions.toRefArray()
		})
	});
});

I think it is best to keep this as a possibility (from above) but have a way simpler API point for this as well such as:

export const getAccounts = createSelector(orm, getOrm, (session) => {
	return session.Account.all().toRefArrayWithConnections(); // with a different name for the function
});

nealoke avatar Jun 03 '17 10:06 nealoke

Well, we are using this library a lot in our application and I must say, that your case is very limited. E.g. You need only subset of relations where some just array of id, but other - array of reduced version of related model(e.g. Id, title), another one must be indexed by some index field. So many cases. In our application we just created few own createSelectors for most cases with some options to tune selectors.

Probably it's better to create another helper-library for it.

On 3 Jun 2017, 13:48 +0300, Neal van der Valk [email protected], wrote:

@plandem I understood it as this. Currently when you want to get something out of the orm in a selector with it's relations (in this case questions) you need to do the following export const getAccounts = createSelector(orm, getOrm, (session) => { const accounts = session.Account.all().toModelArray(); const enhancedAccounts = [];

   accounts.map(accountModel => {
           enhancedAccounts.push({
                   ...accountModel.ref,
                   questions: accountModel.questions.toRefArray()
           })
   });

   return enhancedAccounts;

}); While in the ideal case you could do something like: export const getAccounts = createSelector(orm, getOrm, (session) => { return session.Account.all().toRefArrayWithConnections(); // with a different name for the function }); — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

plandem avatar Jun 03 '17 12:06 plandem

@plandem yhea maybe that's better. I just think it is a nice convenience. I'm also using this in my application and I find myself doing this quite often though.

Maybe @tiii has more use cases or another insight on this? :)

nealoke avatar Jun 03 '17 13:06 nealoke

@nealoke in our case we created and using next selectors:

createSelector, //unified selector, that creates ORM selector or selector from reselect
createRecordSelector, //to select single record with query info for relations and etc
createListSelector, //to select all records with query info for relations and etc
createTreeSelector, //to select all records resolved as tree with query info for relations and etc
createGroupedSelector, //to select all records grouped by field, with query info for relations and etc
createMapSelector,//to select all records indexed by id with query info for relations and etc

All these selectors have set of options to tune result. So as I said, your case is very limited. And probably external helper library would be better. Maybe I will publish our version.

plandem avatar Jun 05 '17 15:06 plandem

@plandem @nealoke

I sometimes request multiple nested resources so the selectors to grab them get kind of nasty. But thats rather a problem of poor design on my side. I guess I should use more redux containers.

Nonetheless I am also interested in your selectors and I would say they already seem like some easier API for denormalizing. Why move them to a seperate library?

tiii avatar Jun 09 '17 16:06 tiii

@tiii separate library, because I'm not sure that it fits here (E.g. in our case we use multi-schema ORM, where some schemas are same, but has differ keys at global state. So it can be project specific). For me it's selector's area. Anyway, right now I'm not ready to make it public yet. Need to revise it :)

I guess I should use more redux containers

I recommend to look for this library: https://github.com/artsy/react-redux-controller

It's small and helped me to reduce the complexity of application.

plandem avatar Jun 09 '17 16:06 plandem

@plandem Are you also planning to update the redux-orm-propTypes? Or do you think this package is obsolete?

nealoke avatar Jun 11 '17 12:06 nealoke

I don't use it, so probably not. Maybe someone else

On 11 Jun 2017, 15:21 +0300, Neal van der Valk [email protected], wrote:

@plandem Are you also planning to update the redux-orm-propTypes? Or do you think this package is obsolete? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

plandem avatar Jun 11 '17 13:06 plandem

@plandem Only other thing I could think of would be performance testing and potentially incorporating Flow. I don't believe either of them are required for v1 though.

NathanBWaters avatar Jun 12 '17 18:06 NathanBWaters

@nealoke I'll take on updating https://github.com/tommikaikkonen/redux-orm-proptypes because I want to use it haha.

NathanBWaters avatar Jun 12 '17 18:06 NathanBWaters

@NathanBWaters agree, that someday we need to tune performance :)

plandem avatar Jun 13 '17 12:06 plandem

hey @plandem - just wanted to throw in my 2 cents...regarding " refactor many-many field declaration to allow forward/backward declaration at each Model"

If many to many has forward/backward declaration would the normal foreignkey/hasmany relationships also need to be defined separately on each model as well?

I think having the backwards relationship explicitly defined would be better. I currently have to keep comments in my code alerting other developers that "hey the backwards relationship is on the other model..." :)

Aryk avatar Jul 15 '17 01:07 Aryk

@Aryk yep, if explicitly then you will have to define on both models and in case of custom through model it can be verbose :) but...at least no surprises :)

plandem avatar Jul 15 '17 08:07 plandem

I really like @Aryk's suggestion, I'm doing the same thing currently with using comments to let others know backwards relationships.

NathanBWaters avatar Jul 17 '17 23:07 NathanBWaters

@NathanBWaters @Aryk as @plandem said, it's already an option. this is how i do it:

class Author extends Model {
  static modelName = 'Author'
  static fields = {
    id: attr(),
    title: attr(),
    books: many({
      to: 'Book',
      through: 'AuthorBook'
    })
  }
}

class Book extends Model {
  static modelName = 'Book'
  static fields = {
    id: attr(),
    title: attr(),
    authors: many({
      to: 'Author',
      through: 'AuthorBook'
    })
  }
}

class AuthorBook extends Model {
  static modelName = 'AuthorBook'
  static fields = {
    authorId: fk('Author'),
    bookId: fk('Book')
  }
}

no need to write alerting comments..

marxus avatar Jul 18 '17 06:07 marxus

@marxus85 - im refering to fk relationships. There is no way to define a one-to-many backwards on the other model currently...

Aryk avatar Jul 19 '17 16:07 Aryk

@plandem add ability to add indexes on columns (other than the ID, which is indexed by default) definitely needed

spachori94 avatar Oct 13 '17 16:10 spachori94

Is this project still alive? Love the simplicity and I think about throwing out realm js for it.

michelalbers avatar Dec 13 '17 09:12 michelalbers

@michelalbers I think it is, however even if it is not that active I already use it in various webapps and it is working like a charm :)

nealoke avatar Dec 13 '17 10:12 nealoke

Things have stalled a bit, but yeah, I think we ought to figure out what we really want for a 1.0 and get it out the door. (#158 probably ought to be in there.)

markerikson avatar Dec 31 '17 23:12 markerikson

library is using in production for few project at our company - quite huge projects with 20+ models/relations and 15k+ records for some models (not sure about 100k, but in some cases we have much more than 15k records).

plandem avatar Dec 31 '17 23:12 plandem