-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: switches from flexbuffer to flexbuffer2 #826
Conversation
Wow @kerihenare, thanks for that. It looks as the final solution. |
Although it's distributed as TS library. Shouldn't it be distributed as both, the |
@kuba-kubula You mean the npm package doesn't include the source files? I've now republished with them. |
Thanks for the pull request! However, given the pr has already changed the major code of the original flexbuffer, and it may potentially introduce inconsistent results to the module interface, I prefer to implementing the related part of flexbuffer in ioredis directly instead of requiring an additional module. What do you think? |
@luin Makes sense. @kerihenare has already started the work on the removal of flexbuffer–as–dependency, see #824 (comment) |
@luin flexbuffer2 is purely a port to ES6 and TS, no logic has really changed. It passes all of the original flexbuffer and ioredis tests. However, I agree it might be better to just reimplement (or port) the functionality that ioredis requires. I've been considering that in this branch. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 7 days if no further activity occurs, but feel free to re-open a closed issue if needed. |
@kerihenare Hi, are you planning to continue on integrating |
Closed via #856. |
@kuba-kubula is correct in #821 that we shouldn't use the package without a license. However, linking directly to a git repo has added a new dependency on git.
As an alternate solution, I've forked flexbuffer and created a modernized drop-in replacement.