turt2live / matrix-bot-sdk

TypeScript/JavaScript SDK for Matrix bots
MIT License
199 stars 69 forks source link

Update presence types & documentation #267

Closed SethFalco closed 2 years ago

SethFalco commented 2 years ago

Checklist

While working on a PR for matrix-appservice-discord, I was tripped up by the presence states. I usually only look in the JSDocs for information while working on projects/PRs, and the term unavailable wasn't clearly defined or linked to before.

Looked to me that "unavailable" was either:

I had to ask in chat to find out it was actually neither of those things. ^-^'

Maybe for convenience for myself and others, could we just add a link to the spec that clearly defines these?

turt2live commented 2 years ago

Unfortunately the spec isn't the most helpful resource for who this SDK is trying to target: beginner or near-beginner bot writers (obviously still supporting the more advanced users too). The SDK is far from that point currently, but I'd like to avoid spec links for now.

If there's a guide which can be linked to (or created), that would be preferable

SethFalco commented 2 years ago

Unfortunately the spec isn't the most helpful resource for who this SDK is trying to target

Tbh, I thought the spec did a pretty good job in this case at least. Though I'll admit the spec can be intimidating to check out.

However, if we're keen on not putting spec links for now, I could try and find a guide, or just duplicate some of the information over as JSDocs.

turt2live commented 2 years ago

A guide or duplicating would be fine, yea

SethFalco commented 2 years ago

I couldn't find a guide for it, so opted to duplicate. Probably depends on the processor, but VSC renders Markdown in JSDocs so just did it in Markdown.

image

image

Also added the @see in PresenceEventContent#presence since it's more often going to be exposed through here. This one might be silly, technically a user could just click through on their own anyway.

While making this change, I noticed this was actually already documented in MatrixClient#syncingPresence. I just never ran into that while consuming the SDK in matrix-appservice-discord. However, the one in MatrixClient also covers null, so I guess it's technically different.


On the side, I noticed the type "online" | "offline" | "unavailable" was done in multiple places. Is there any reason we couldn't just have PresenceState used in those places? I updated it in this PR, but please let me know if that should be undone.