fix(adapter-commons): faster sorter

feathersjs / feathers

The API and real-time application framework

https://feathersjs.com

MIT License

14.97k stars 742 forks source link

fix(adapter-commons): faster sorter #3495

Closed fratzinger closed 3 weeks ago

fratzinger commented 4 weeks ago

This is a small refactoring.

check for 'null && undefined' in one if
detect typeof only once
move get fn outside
check key.includes('.') before key.split('.')
type compareNSB & compareString

cloudflare-pages[bot] commented 4 weeks ago

Deploying feathers-dove with Cloudflare Pages

Latest commit:	`a30281f`
Status:	✅ Deploy successful!
Preview URL:	https://1d8832bb.feathers.pages.dev
Branch Preview URL:	https://refactor-sorter.feathers.pages.dev

View logs

daffl commented 3 weeks ago

This looks good, thank you. I wonder if there is a more generic package for this kind of thing. Doesn't seem to be something we should have to maintain ourselves - I took this from NeDB when it stopped being maintained.

fratzinger commented 3 weeks ago

https://github.com/kofrasa/mingo could help. But I think it's fine. It's a small utility which is not a subject of change often and most adapters use @feathersjs/adapter-commons for other reasons anyways. It's good to have it in this package.

I think we should not overcomplicate this. Or we end up importing and exporting from another library and their versioning has to align with our versioning. We have enough of these problems.

daffl commented 3 weeks ago

True. It might be worth getting more test coverage for it eventually if we do want to keep it around.

fratzinger commented 3 weeks ago

💯. I also thought about that.