search

kriszyp / lmdb-js

Simple, efficient, ultra-fast, scalable data store wrapper for LMDB
Other
505 stars 41 forks source link

lmdb doesn't work if installed with --ignore-optional #97

Closed KyleAMathews closed 2 years ago

KyleAMathews commented 2 years ago

As lmdb uses msgpackr by default so a few Gatsby users have had problems e.g. see https://github.com/gatsbyjs/gatsby/discussions/33927

So the solution I think is to either try/catch requiring msgpackr or to mark it as a required dependency.

kriszyp commented 2 years ago

Would it be feasible to include msgpackr as required dependency of gatsby, if you are using the default msgpackr encoding in lmdb?

For try/catch, I could do that, but would have to some fallback, which I suppose would probably be JSON. However, it might be considered a bit of a hazard that your database could end up in a completely different format based on your npm setting options (and switching them would render you db unreadable).

That being said, making msgpackr a required dependency seems quite reasonable, it is a very light dependency as far as node deps go. I think I will go ahead and do that.

kriszyp commented 2 years ago

I published v1.6.14 with this update to make msgpackr a required dependency.

KyleAMathews commented 2 years ago

Thanks!

On Thu, Nov 18, 2021, 2:49 PM Kris Zyp @.***> wrote:

I published v1.6.14 with this update to make msgpackr a required dependency.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/DoctorEvidence/lmdb-js/issues/97#issuecomment-973354588, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAARLB6PEPTD5UUH5AB57CLUMV7HVANCNFSM5IHP5X6A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.