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

Kinds are limited, and I'm forced to cast them #275

Closed
jeremyd opened this issue Aug 12, 2023 · 3 comments · Fixed by #276
Closed

Kinds are limited, and I'm forced to cast them #275

jeremyd opened this issue Aug 12, 2023 · 3 comments · Fixed by #276

Comments

@jeremyd
Copy link

jeremyd commented Aug 12, 2023

Sometimes I need a new Kind, I'd rather just be using the integer. The Enum just can't keep up with all the kinds that we may want.

eg. I'm trying out ndk. I get this error:

Type error: Type '30000' is not assignable to type 'Kind'.

  82 |         if (session && session.user != null && session.user.name != null) {
  83 |             ndk.connect()
> 84 |             const filter: NDKFilter = { kinds: [30000, 10000, 3], authors: [session.user.name] }

It appears that NDKFilter just uses the nostr-tools filter, which has less kinds than NDKKind..

I attempted to patch nostr-tools but I went into a rabbithole I can't seem to install nostr-tools from a forked github repository source, so, I guess I'll just force cast the additional kinds I need:

const kind30000: Kind = 30000 as Kind;
const kind10000: Kind = 10000 as Kind;
const filter: NDKFilter = { kinds: [kind30000, kind10000, 3], authors: [session.user.name] }

can we just get rid of these enums? All it does is make it hard to keep up with the various kinds.

@alexgleason
Copy link
Collaborator

I don't know about NDK, but in this library Filter<K> is generic, so you can use Filter<number> to support any kind, or Filter<0 | 1 | 3> to support specific kinds.

That being said, I have experienced problems with the enum. For example, function toActor(event: Event<Kind.Metadata>) does NOT guard the kind like function toActor(event: Event<0>) does. Also, I don't think people want to import two types to use a single type. I think people want to memorize the kind numbers.

We can leave the Kinds object exported for backwards-compat, but I would be in favor of changing Kinds to number in all nostr-tools internal type definitions. That's less maintenance and easier for users.

I would also suggest changing the type definitions of NDK to accept <K extends number = number> and pass <K> to all nostr-tools types.

alexgleason added a commit to alexgleason/nostr-tools that referenced this issue Aug 12, 2023
@alexgleason
Copy link
Collaborator

I opened #276

@alexgleason
Copy link
Collaborator

Well, what do you know, NDKFilter is just an alias of Filter, so using NDKFilter<number> will already work.

fiatjaf pushed a commit that referenced this issue Aug 12, 2023
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 a pull request may close this issue.

2 participants