Why in line 79 for src/preconditions/ClientPermissions.ts is there `PermissionsBitField | null`?
Hello, I'm doing some (potentially bad, please warn me) things to be able to check permissions whenever I need it in my bot. I figured to have sane code, I'd copy off of Sapphire's precondition code for myself to use, specifically ClientPermissions.
As far as I know, if I want localization and sane control over the message text when there's a permission error, and the permission check can be done anyway, I'm guessing this is how I do it.
While looking through the code, I noticed the explicit type cast i'd ping favna judging by the commit author but that seems impolite)
Commit in question: https://github.com/sapphiredev/framework/commit/725a7d3a8e8f92860aaf946aecfc988abc8d9553
PermissionsBitField | null
. I also noticed this could be avoided, assuming the types weren't lying to me from channel.permissionFor(me)
.
Upon checking the commit for more info, it said it fixed permission checking for old applications. I can't find any more info besides that, and the pull request attached to it.
Would anyone from the dev team be able to shine any light on it? (17 Replies
My potential code, in which I'm naively unaware if I'm about to make a mistake:
It looks like
channel.permissionsFor()
is only nullable in some cases. In the typings, there is an override where it is nullable (if you pass in a role or member resolvable, because it doesn't know if it actually exists), and an override where it isn't (if you pass the role or member itself)
fetchMe()
doesn't return a resolvable, though, so I think you're right in that the | null
is unnecessary
Oh, nevermind
It could also be the return type of permissionsFor
where the application id is passed, which is a resolvable, and therefore the result is nullable.
So, in conclusion, the | null
is added to allow:
I was about to post this:
in my edit, i spun off...exceptpermissions
into a new variablenewPermissions
. This is so I could dochannel.permissionsFor(messageOrInteraction.applicationId);
without the potentialnull
output affecting the type of the functiongetPermissionsForChannel()
, which I understand to be unnecessary. I thinkgetPermissionsForChannel()
can just be of typePermissionsBitField
and notPermissionsBitField | null
. If this was ever turned null by theapplicationId
fetch, I believe it'd make sure it'd always not be null, due tofetchMe()
, presumably becausefetchMe()
would never benull
permissions
is still technically nullable, because if fetchMe()
for some reason can be undefined
. I'm not 100% sure on how that'd be the case, but I presume that makes the | null
function typing make sense. Did I get that right? (also, the reason on why it'd be undefined
would be great. best guess is that somehow d.js has http bot support or something)From the docs, I don't think
fetchMe()
can be undefined. However, me
would be undefined if messageOrInteraction.guild
is.oh wait a sec
im getting undefined because of
messageOrInteraction.guild?
, yep
in that case, how can we be in a guild and have the guild be null?Use
inCachedGuild()
instead of inGuild()
inGuild()
will only ensure guildId
, not guild
inCachedGuild()
doesn't seem to work due to the Message
typing, hmmYou could have it be
Message<true> | BaseInteraction<'cached'>
in the parameter types, and force the caller to do the typeguards specific to their context beforehand?
Or you could just use the non-null assertion operator
Or you could use channel.guild
instead, if that's not nullableoh hey yeah, i can guarantee channel being a non-partial also! i think that's the best solution there
cool beans 😎
null
typing is not needed then. Even if channel is a partial, and guild is unavailable, it'd be a DM channel judging by types. Might be worth refactoring later, unless a contributor who actually knows the codebase says otherwise.Wdym by this? It's
null
because the guild might not be cachedi'm guessing guild will always be cached, solely because of d.js's typings, since
guild
in channel.guild
after !channel.isDMBased()
can't be null. I could be easily wrong, as i don't know d.js's internals at allThere are edge cases where it wouldn't be there, so sapphire's just playing it safe 🙂
oh fun
what's the edge cases?
If you configure it not to be in your client options or if you're sharding and the shard to that guild has gone down
ah, that makes sense. thanks for the reply!