nbd-wtf / nostr-tools

Tools for developing Nostr clients.
The Unlicense
685 stars 188 forks source link

including nostr specialized types #409

Open antonioconselheiro opened 3 months ago

antonioconselheiro commented 3 months ago

exporting nip19 nostr specific types, implements: https://github.com/nbd-wtf/nostr-tools/issues/408

fiatjaf commented 2 months ago

Is this ready to merge?

antonioconselheiro commented 2 months ago

In my view yes, but I'm nobody. You think the regex in type guard are ok?

fiatjaf commented 2 months ago

Yes, except the types should be called "npub" not "npublic", "nsec" not "nsecret", but I can fix that after I merge.

antonioconselheiro commented 2 months ago

so, maybe nadress should be just nadrr

antonioconselheiro commented 2 months ago

I've changed the names in types, type guards and in the included unit tests

fiatjaf commented 1 month ago

I'll take a look at this soon, if I don't forget. Thank you.

antonioconselheiro commented 1 month ago

Thank you for Nostr sir, but talking about types exportations, can this lib export too the { write: boolean, read: boolean } relay config interface with a name? As you said, it doesn't hurt. If yes, I'll include in this pull request.

fiatjaf commented 1 month ago

It does hurt my ability to understand my own library.

alexgleason commented 1 month ago

Naming types is overrated. It's better to just copy the same type in multiple places, unless it's a truly universal interface.

antonioconselheiro commented 1 month ago

ok

antonioconselheiro commented 1 month ago

Another thing... There is already a regex to valid nip05 and would be nice include type guard for this too, NIP05 type is included in this PR but not the type guard, but to include this it will be in nip5.ts and not with the others in NostrTypeGuard object in core.ts.

Make sense include this in nip5.ts?

export const isNip05 = (value?: string | null): value is Nip05 => NIP05_REGEX.test(value || '')
fiatjaf commented 1 month ago

Yes. In nip.ts

antonioconselheiro commented 1 month ago

NIP05_REGEX matches for domain.com and name@domain.com, but my type in this PR for NIP05 is ${string}@${string}, is my type right? or should have a regex with @ as mandatory to validate?

alexgleason commented 1 month ago

NIP05_REGEX is wrong, I learned after looking at NIP-05 itself. It's too lenient. I was following what the person had done before, but the @ is actually required by the spec.

Btw I think this is still a pointless type.

antonioconselheiro commented 1 month ago

Thank you, but I can't see they as pointless types, I see a lot of value in this.

antonioconselheiro commented 1 month ago

This unit test requires NIP05_REGEX to include @ as optional 🤔, is this wrong? Or is this a particular validation of the function queryProfile? image