node-redis icon indicating copy to clipboard operation
node-redis copied to clipboard

Strict type decleration for PropertyName causing tsc compile error

Open JmsPae opened this issue 1 year ago • 1 comments

I'm working on a project using typescript and created an index using json. Each json object has a unix timestamp, and I wanted to filter as well as sort objects with timestamps from the last 5 mins which led to the code below;

redis.ft.search('vehicleIdx', '@timestamp:[${Math.floor(Date.now() / 1000) - 300}, +inf]', {SORTBY: {BY: 'timestamp', DIRECTION: 'DESC'}, LIMIT: {from: 0, size: 100}})

At least when compiling from ts this will cause a compilation error due to the lack of a @ or $. prefix before 'timestamp' as the SORTBY argument. This can be fixed quite easily by packages/search/lib/commands/index.ts line 126 to

export type PropertyName =`${string}`;

... And it works for me just as intended.

I'm unfamiliar with the codebase and started working with redis the other night so I wonder if there would be any unintended consequences to such a change.

Environment:

  • Node.js Version: 18.7.0
  • Redis Server Version: 7.0.4
  • Node Redis Version: 4.2.0
  • Platform: Ubuntu 22.04

JmsPae avatar Aug 11 '22 20:08 JmsPae

For a little more context;

Index creation: FT.CREATE vehicleIdx ON JSON PREFIX 1 vehicles:data: SCHEMA $.timestamp AS timestamp NUMERIC SORTABLE

roughly equivalent ft.search: FT.SEARCH vehicleIdx "@timestamp:[0, +inf]" SORTBY "timestamp" DESC

JmsPae avatar Aug 11 '22 20:08 JmsPae

Have the same problem.

SortByProperty type accepts only PropertyName type, but should accept just string.

// packages/search/lib/commands/index.ts:137
export type PropertyName = `${'@' | '$.'}${string}`;

export type SortByProperty = PropertyName | {
    BY: PropertyName;
    DIRECTION?: 'ASC' | 'DESC';
};

Even in examples it is used without $. or @ https://github.com/redis/node-redis/blob/master/examples/search-hashes.js#L51

Environment:

  • Node Redis Version: "4.5.1"

VladimirChuprazov avatar Dec 08 '22 09:12 VladimirChuprazov

Fixed in #2343. I'll comment here again once it's released

leibale avatar Dec 14 '22 22:12 leibale