redis / redis-om-node

Object mapping, and more, for Redis and Node.js. Written in TypeScript.
MIT License
1.16k stars 78 forks source link

Version 0.4.4 (latest) does not ship with types #241

Closed pastelsky closed 1 month ago

pastelsky commented 1 month ago

The latest version does not ship with type definitions — https://unpkg.com/browse/redis-om@0.4.4/dist/

however, the older versions seem to have it —

https://unpkg.com/browse/redis-om@0.4.3/dist/

guyroyse commented 1 month ago

That's really odd as I literally just did the same npm run build and npm publish that I've always done. The changes weren't such that it should behave any differently. And the build does build the type definitions. Guess I've got some digging to do.

If you have any thoughts, please do share them. And thanks for catching this!

guyroyse commented 1 month ago

Published on NPM under version 0.4.6. Looks like NPM's behavior around glob-style paths in the files property of package.json changed. Sorry for any inconvenience.

pastelsky commented 1 month ago

So 0.4.6 has type definitions now (thanks!). However I see that the client type (as defined by redis) and that expected by redis-om do not seem to match.

This means this throws a type error —

import { createClient } from 'redis';
import { Repository } from 'redis-om';

import { createClient } from 'redis';
import { branchSchema } from '../entities/branch.entity';

const client = createClient();
const repository = new Repository(branchSchema, client);

Error

 error TS2345: Argument of type 'RedisClientType<{ graph: { CONFIG_GET: typeof import("/project/node_modules/@redis/graph/dist/commands/CONFIG_GET"); configGet: typeof import("/project/node_modules/@redis/graph/dist/commands/CONFIG_GET"); ... 15 more ...; slowLog: t...' is not assignable to parameter of type 'Client | RedisConnection'.
  Type 'import("/project/node_modules/@redis/client/dist/lib/client/index").RedisClientType<{ graph: { CONFIG_GET: typeof import("/project/node_modules/@redis/graph/dist/commands/CONFIG_GET"); configGet: typeof import("/project...' is not assignable to type 'import("/project/node_modules/redis-om/node_modules/@redis/client/dist/lib/client/index").RedisClientType<{ graph: { CONFIG_GET: typeof import("/project/node_modules/@redis/graph/dist/commands/CONFIG_GET"); configGet: typeof import("...'.
    Type 'import("/project/node_modules/@redis/client/dist/lib/client/index").RedisClientType<{ graph: { CONFIG_GET: typeof import("/project/node_modules/@redis/graph/dist/commands/CONFIG_GET"); configGet: typeof import("/project...' is not assignable to type 'import("/project/node_modules/redis-om/node_modules/@redis/client/dist/lib/client/index").RedisClientType<{ graph: { CONFIG_GET: typeof import("/project/node_modules/@redis/graph/dist/commands/CONFIG_GET"); configGet: typeof import("...'.
      Property '#private' is missing in type 'import("/project/node_modules/@redis/client/dist/lib/client/index").default<{ graph: { CONFIG_GET: typeof import("/project/node_modules/@redis/graph/dist/commands/CONFIG_GET"); configGet: typeof import("/projectian/uip-...' but required in type 'import("/project/node_modules/redis-om/node_modules/@redis/client/dist/lib/client/index").default<{ graph: { CONFIG_GET: typeof import("/project/node_modules/@redis/graph/dist/commands/CONFIG_GET"); configGet: typeof import("/Users/s...'.

15     const repository = new Repository(branchSchema, createClient());
                                                       ~~~~~~~~~~~~~~~~

  node_modules/redis-om/node_modules/@redis/client/dist/lib/client/index.d.ts:92:5
    92     #private;
           ~~~~~~~~
    '#private' is declared here.

I'm on redis 4.6.14

guyroyse commented 1 month ago

I'll dig into it. I suspect that something was marked #private that shouldn't have been as Redis OM used Node Redis 4.6.4. I'd be very surprised if a breaking change with Node Redis was introduced on a minor version change.

I will say in my defense, the unit and integration tests did pass. 😉

guyroyse commented 1 month ago

Okay. I'm not able to reproduce this but I do see a bug in error and your sample code:

const repository = new Repository(branchSchema, createClient());

You need to pass in a connected client to Repository:

const client = createClient()
await client.connect()

const repository = new Repository<BoringEntity>(boringSchema, client)

When I do this, it works just fine. However, when I tried doing it without connecting, I don't get the same error message as you. Instead, I get this one:

/Users/guyroyse/code/redis/om-node/issue-241/node_modules/redis/node_modules/@redis/client/dist/lib/client/index.js:510
        return Promise.reject(new errors_1.ClientClosedError());
                              ^

ClientClosedError: The client is closed
    at Commander._RedisClient_sendCommand (/Users/guyroyse/code/redis/om-node/issue-241/node_modules/redis/node_modules/@redis/client/dist/lib/client/index.js:510:31)
    at Commander.commandsExecutor (/Users/guyroyse/code/redis/om-node/issue-241/node_modules/redis/node_modules/@redis/client/dist/lib/client/index.js:190:154)
    at Commander.<computed>.<computed> [as set] (/Users/guyroyse/code/redis/om-node/issue-241/node_modules/redis/node_modules/@redis/client/dist/lib/commander.js:58:33)
    at Client.jsonset (/Users/guyroyse/code/redis/om-node/issue-241/node_modules/redis-om/dist/index.js:1786:27)
    at Repository.writeEntityToJson (/Users/guyroyse/code/redis/om-node/issue-241/node_modules/redis-om/dist/index.js:1685:23)
    at Repository.writeEntity (/Users/guyroyse/code/redis/om-node/issue-241/node_modules/redis-om/dist/index.js:1658:91)
    at Repository.save (/Users/guyroyse/code/redis/om-node/issue-241/node_modules/redis-om/dist/index.js:1611:16)
    at async main (file:///Users/guyroyse/code/redis/om-node/issue-241/dist/app.js:12:14)

Can you try connecting and seeing of the error goes away? If it doesn't, could you provide a sample repo that reproduces the error?

Thanks!

guyroyse commented 1 month ago

Any movement on your side @pastelsky? Can I close this issue?