Implement the orderBy feature

[jonesnc]

No description provided.

[wovalle]

Hey @jonesnc, you're killing it lately! I know this is a WIP but I have a few comments:

• It seems that there are many linting discrepancies between your branch and the project (dangling commas, spaces and so on). I use Prettier to format the code and I shared my rules configuration in the repo
• In some places you declare the orcerBy params as prop: keyof T, in others as prop: keyof T & string. Isn't the first one enough?
• I would make a orderDirection (or other name) enum (asc, desc) instead of having strings in this.orderByObj
• yarn.lock diff is huge, even though you didn't added/removed any package.

[jonesnc]

Thanks for the feedback, @wovalle!

• Fixed(?) - VSCode was using the wrong formatter, thanks for catching that!
• keyof T has a type of string | number | symbol, so I was using keyof T & string as a way to ensure that the type of prop is string. I have updated my PR so it consistently uses prop: keyof T & string, but I'm certainly open to any suggestions you have.
• My thought behind this is that firestore already lists the asc and desc values in their OrderByDirection type definition. It seemed to me that making our own Enum for those values would duplicate that type definition. I can certainly make this correction if you still deem it better.
• Fixed

EDIT: I'm still seeing some formatting differences that probably shouldn't be there. I'll keep messing with Prettier until it works correctly.

[wovalle]

Sorry it took me a couple of days to get to this PR, wanted to sit quietly to check it out. This feature is almost done! Good one catching the OrderByTypeDefinition :)

I left some small comments. It seems that you'll have to rebase from master since there are a couple of conflicting files. Lastly, remove the WIP from the title or I won't be able to merge.

Just out of curiosity: how did you find this library? do you use fireorm as a dependency for another project?

Thanks!!

[jonesnc]

@wovalle I don't actually use fireorm at all right now. I have a side project that uses Firestore, mobx+react, and firestorter for integrating with mobx.

I've been looking for a library that allowed for more complex interaction with Firestore than what is provided out-of-the-box, and this was one of the results.

I also saw a project called fiery-data, which is farther along than this project is, but so far I like how this package is designed a lot more than fiery-data.

Thanks for being patient with me with your suggestions! I should have something that's workable this week.

[jonesnc]

@wovalle I have an idea on how to implement #46

Do you mind if I add it to this PR, or would you rather that be implemented in a separate PR?

Or if you want to keep that issue open for newcomers to tackle, I can work on other issues.

[wovalle]

If you want to implement it, go ahead! I created those issues so more people might get interested to start with an easy one ;) Hopefully I'll write more challenging ones soon.

[wovalle]

That implementation of the error is perfect. Is this PR finished? Should I review it again? @jonesnc

[wovalle]

closes #46

[jonesnc]

@wovalle Yes, this PR is ready.

[wovalle]

I promise this is the last one @jonesnc, just a styling error.

Great job!

[jonesnc]

@wovalle that styling error has been fixed. Let me know if you have any other changes you'd like me to make.

Thanks!

[wovalle]

Hey! Sorry I haven't merged this yet. I took some vacations from fireorm, I'll be 100% back early next week :)

In the mean time, I tried to rebase master but didn't find how to do it since your branch is in a fork that I don't have permissions. Can you rebase from master (or just merge master?) I updated the dependencies and the error that travis is throwing should be fixed there.

[wovalle]

Closes #30

[whateverbot]

🎉 This PR is included in version 0.6.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀