proofcarryingdata / zupass

Zuzalu Passport
259 stars 64 forks source link

apply snarkjs performance improvement to semaphore package #1787

Closed ichub closed 1 month ago

robknight commented 1 month ago

I wonder if this is going to get confusing if the underlying package gets upgraded in the future, and whether we couldn't maintain patches using something like this: https://www.npmjs.com/package/patch-package

ichub commented 1 month ago

It probably will get confusing, at which point I'd be down to check out patch-package as an alternative. I think for now this is fine. My goal is to reduce friction for the ongoing event ASAP.

artwyman commented 1 month ago

I wonder if this is going to get confusing if the underlying package gets upgraded in the future, and whether we couldn't maintain patches using something like this: https://www.npmjs.com/package/patch-package

I really hope that all of this patching is a short-term necessity which will go away via upstreaming a fix to snarkjs. Otherwise yes, I think managing these forks every time a dependency updates is going to be come unmanageable.

[patch-package](https://www.npmjs.com/package/patch-package) sounds cool to check out if we can't upstream our fix for some reason.

ichub commented 1 month ago

https://linear.app/0xparc-pcd/issue/0XP-954/follow-up-on-snarkjs-fixes-open-sourcely

ichub commented 1 month ago

superceded by https://github.com/proofcarryingdata/zupass/pull/1797, thanks @robknight for the patch-package suggestion. it turned out to be untenable for me to patch semaphore the same way i was doing snarkjs (urgh) so I used patch-package which ended up being a lot simpler!