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

Suggestion: export kinds as named types #447

Merged
merged 6 commits into from
Oct 23, 2024
Merged

Suggestion: export kinds as named types #447

merged 6 commits into from
Oct 23, 2024

Conversation

antonioconselheiro
Copy link
Contributor

@antonioconselheiro antonioconselheiro commented Oct 23, 2024

Gm,

I would like to suggest exporting kinds as named types.
This will allow use content from kinds as a typescript type and not only as a value.

Without this dev will need to use typeof kinds.[kind] to represent the type.
image

This suggestion includes isKind guard.
This will help dev validate event type and also cast it into a kind checked event.
This will encourage dev require in method signature already validated events from the expected kind.

This guard makes import from './pure.ts', not sure if this is a problem.

@fiatjaf fiatjaf merged commit aba266b into nbd-wtf:master Oct 23, 2024
2 checks passed
@fiatjaf
Copy link
Collaborator

fiatjaf commented Oct 23, 2024

Looks interesting, let's try.

@antonioconselheiro
Copy link
Contributor Author

I feel sad when enum was removed, but with that I believe we have something better than the old enum.

@antonioconselheiro
Copy link
Contributor Author

antonioconselheiro commented Oct 23, 2024

I would like to dare to suggest bring back NostrEvent<T extends number = number> { kind: T }, it'll be compatible to NostrEvent with not generic type included because have default to T and will help use NostrEvent<Reaction | Zap> NostrEvent & { kind: ShortText | Reaction }

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 this pull request may close these issues.

2 participants