Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Idea: Add new hgetallMap method to return Map<String,String> #1417

Open
TysonAndre opened this issue Aug 18, 2021 · 1 comment
Open

Idea: Add new hgetallMap method to return Map<String,String> #1417

TysonAndre opened this issue Aug 18, 2021 · 1 comment
Labels

Comments

@TysonAndre
Copy link
Collaborator

(and hgetallMapBuffer: Map<String, Buffer>)

Motivation:

  • Make it easier to write consise code that avoids prototype pollution
  • Often, I'd end up converting Object.entries(await redis.hgetall(key)) to a Map anyway.
    (easier to make callers write map.has(fieldName) than Object.prototype.hasOwnProperty.call(obj, fieldName), since obj[fieldName] would be wrong for fields that are part of the object prototype)

Related to #1267

(Aside: would key in obj work better than obj[key] for the check if the property was already defined, in the PR fixing that issue for hgetall (to account for Object.prototype.customProperty = false;)? Otherwise, the PR looked correct.)

@luin
Copy link
Collaborator

luin commented Aug 18, 2021

(and hgetallMapBuffer: Map<String, Buffer>)

I definitely think it's useful to add it. However, I'm not sure if this is the right time to add it. For now, although ioredis is 100% written in TypeScript, many types are missing (ex there are no typings for Redis#get). I think we should solve the typing issue first and then add more methods to Redis/Commander since the process of adding types may result in breaking changes.

(Aside: would key in obj work better than obj[key] for the check if the property was already defined...

Good point! I didn't think of that. Can you take a look #1418 ?

luin added a commit that referenced this issue Aug 18, 2021
* fix: improve proto checking for hgetall

As mentioned in #1417

* Address feedbacks
@luin luin added the feature label Mar 28, 2022
janus-dev87 added a commit to janus-dev87/ioredis-work that referenced this issue Mar 1, 2024
* fix: improve proto checking for hgetall

As mentioned in redis/ioredis#1417

* Address feedbacks
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants