feat: 🔥 First punt at priority option

[theryansmee]

Allow user to set length priority to length or unique (POC)

✅ Closes: #220

PR Checklist

Please check if your PR fulfills the following requirements:

• [x] The commit message follows our guidelines: https://github.com/ngneat/falso/blob/master/CONTRIBUTING.md#commit
• [x] Tests for the changes have been added (for bug fixes / features)
• [x] Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

[theryansmee]

@NetanelBasal @shaharkazaz - This is a super sloppy implementation that I threw together at lunch today, what are your thoughts?

[NetanelBasal]

Thanks @theryansmee, @shaharkazaz will go over it asap.

[theryansmee]

🚀

https://github.com/ngneat/falso/pull/223#discussion_r831881743: Yeah you are right. I'll have a think about how we can compare complex objects. With most of our entities be happy to consider it uniques as long as the id is unique.. but i'll have to look through our functions to see what other challenges we face.

I've fixed the other bits

[theryansmee]

@shaharkazaz @NetanelBasal I've still got to write tests and clean up a few bits, but I think this is mostly there now. What are your thoughts? 😊

[shaharkazaz]

@theryansmee I'll re-review it again today 👍

[theryansmee]

@shaharkazaz I've ended up creating objectIsUnique which is then used by the other checker functions around the library. Originally, I was hoping to just have a single function that got passed around by I ended up in type hell.

That said, I think this is actually quite an elegant solution.

PS I think once we are happy with this MR, we should do some follow up work on splitting core.ts into multiple files and ensuring that it has a decent test coverage.

~~PPS I'm not sure why the build it failing. Looks like we have an issue with packages somewhere down the line.~~

[NetanelBasal]

@shaharkazaz what's the status here?

[shaharkazaz]

@NetanelBasal I'll re-review this week @theryansmee update from master 🙂

[theryansmee]

@NetanelBasal I'll re-review this week

@theryansmee update from master 🙂

Ok cool. I'll get this updated later today

[theryansmee]

@shaharkazaz @NetanelBasal - Do we have any movement on this? (I know it's a large one)

[NetanelBasal]

@shaharkazaz will check it asap. sorry for the delay

[theryansmee]

@shaharkazaz will check it asap. sorry for the delay

No worries. I hadn't realised how bit this has grown. I can imagine it's going to be a large job to review

[theryansmee]

@NetanelBasal - Hello mate, i've got most of the changes done with 2 queries to be answered 😊

[theryansmee]

@NetanelBasal - I've made some tweaks and asked some questions above

[NetanelBasal]

@theryansmee, I think that you missed some comments.

[theryansmee]

@theryansmee, I think that you missed some comments.

@NetanelBasal - Hello mate. Sorry I cant see any comments that I haven't either replied to or fixed. I have resolved all conversations that I believe to be resolved to make everything a little more readable.

Below I have listed the last 2 outstanding comments:

1. Nested objects - objectIsUnique does not support nested object at the moment, but we don't have any functions in Falso that needs support for this at the moment. I'm happy to add this functionality, but I feel that it adds unnecessary complexity to an already very large pull request
2. Verbose comments - I have simplified to now read // default is 'length' (See below screenshot)

[NetanelBasal]

@theryansmee what's the status :) ?

[theryansmee]

@theryansmee what's the status :) ?

Hello mate, i'm just waiting for your reply to https://github.com/ngneat/falso/pull/223#issuecomment-1125297656 and I think it's done

[NetanelBasal]

@theryansmee one last comment. Also, please squash to one commit.

[NetanelBasal]

@theryansmee can we please finish with this and merge it 😝

[theryansmee]

@theryansmee can we please finish with this and merge it 😝

Yeah 100%. Sorry my life have been a little crazy the last month or so with work etc.

I've looked at the index query above and although I can't work out why we needed it before, I don't think we need it now. In fact Typescript complains if we add it in now.

I'm happy to merge if you are? I can get Github squash all the commits on merge.

[theryansmee]

🙃 Just seen theres been commits to core.ts so i'm going to have to look at what they are and get them integrated into my work

[NetanelBasal]

Cool, thanks!

[theryansmee]

I will be fixing the conflicts and merging this later this week :)

[theryansmee]

@NetanelBasal - I've merged in the latest changes but now the build is failing on the setup-node@v2 step with getCacheEntry failed: Cache service responded with 503.

Any thoughts on what might be happening?

[NetanelBasal]

Nopehttps://stackoverflow.com/questions/72598852/getcacheentry-failed-cache-service-responded-with-503

[theryansmee]

@NetanelBasal @shaharkazaz - Hi guys, sorry for the radio silence from me (Life got a little hectic). I have fixed the merge conflicts and added a few essential tests to prove priority logic works as expected with data from arrays and from functions.

I think it's ready to be reviewed again 😊

[shaharkazaz]

@theryansmee Hi Ryan always a pleasure to hear from you 😁 I'll re-review it today-tomorrow.

[theryansmee]

@theryansmee Hi Ryan always a pleasure to hear from you 😁 I'll re-review it today-tomorrow.

I'm back for good this time! haha Cheers mate 😊

[theryansmee]

This is the first cycle, I'll go in-depth over the core files, these are the first things that I saw

Cheers mate, i'll have a look through and get these sorted tomorrow 👍