Skip to content
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

BREAKING: Improve reply types #210

Merged
merged 20 commits into from
Mar 13, 2021
Merged

BREAKING: Improve reply types #210

merged 20 commits into from
Mar 13, 2021

Conversation

uki00a
Copy link
Member

@uki00a uki00a commented Mar 9, 2021

Related to #189 and #80.

Summary

Currently, CommandExecutor returns replies in the form of a tuple of [<kind>, <raw data>] as follows:

  const reply = await redis.executor.exec("SET", "key", "a");
  assertEquals(reply, ["status", "OK"]);

In this PR, CommandExecutor is changed to return one of the following types:

  • BulkReply
  • StatusReply
  • IntegerReply
  • ArrayReply

Examples

  const reply = await redis.executor.exec("SET", "key", "a");
  assertEquals(reply.type, "status");
  assertEquals(reply.value(), "OK");

Motivation

This change is intended to support the following features in the future:

Getting the raw data as a Uint8Array

The BulkReply type have buffer method which returns Uint8Array:

  const reply = await redis.executor.exec("GET", "redis");
  assert(reply.type === BULK_STRING_TYPE);
  const buffer = reply.buffer();

Changes

  • Change return type of CommandExecutor#execute from tuple form to object form
  • Rename Status to SimpleString

@uki00a uki00a marked this pull request as ready for review March 13, 2021 13:45
@uki00a uki00a changed the title [WIP] BREAKING: Improve reply types BREAKING: Improve reply types Mar 13, 2021
@uki00a uki00a merged commit d1347ac into denodrivers:master Mar 13, 2021
@uki00a uki00a deleted the improve-reply-types branch March 13, 2021 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant