miketromba / drizzle-pagination

Easily paginate your Drizzle ORM queries
MIT License
52 stars 3 forks source link

Update dependencies to allow newest drizzle orm, clean up code a bit #1

Closed rmtmckenzie closed 1 year ago

rmtmckenzie commented 1 year ago

I'm trying to test this out in my project and npm was complaining, so I updated the dependencies on the drizzle orm to be a bit more flexible. Figured I might as well clean up a couple things while I was at it (it was giving me type errors).

This looks like a super helpful little function though, thanks for sharing it!

miketromba commented 1 year ago

Hey, this looks good, thanks for submitting!

Is this bit necessary <0.29.0? Trying to figure out if it would make more sense to change it to <1.0.0, or remove it for now and wait for a drizzle update to actually break this package before defining a version cap.

Wonder if you have an opinion on this? Thanks

rmtmckenzie commented 1 year ago

The reason I did it that way is that I know 0.28.x for sure doesn't have breaking changes that affect this. Unfortunately it's a bit hard to tell from their numbering system what would, as normally that's what major versions are for, and I find it a little hard to believe that there haven't been any breaking changes in the 28 iterations so far.

You'd probably be completely fine to use <1.0.0, but I just went with the safest option, or you could avoid putting an upper bound completely but I think it's better practice not to do that as this would result in an error at update time rather than at runtime if there did happen to be breaking changes. I lean towards having things error out as early as possible even if it sometimes means a teensy bit more maintenance.

miketromba commented 1 year ago

Got it, thanks for the input & explanation. I don't have the bandwidth to watch this package closely and bump for every minor drizzle release so going to opt for the middle ground approach (<1.0.0) for now.