discord.js
discord.js copied to clipboard
refactor(PermissionOverwrites)!: cache-independent resolve
Please describe the changes this PR makes and why it should be merged:
Resolves #10450.
The linked issue relates to the current cache-dependent behaviour of PermissionOverwrites.resolve()
, specifically that passing identical parameters to .resolve()
can conditionally pass or fail validation depending on the state of the cache.
This isn't ideal, and the function would be better placed resolving the target via .resolveId()
in a cache-independent manner.
This approach doesn't allow for overwrite.type
to be optional if overwrite.id
is a Snowflake, since it would no longer attempt to seek the respective member/role object from the cache to identify it as one or the other. There are two options for how to handle this:
- always enforce
overwrite.type
as mandatory, even ifoverwrite.id
is a member object or role object; - keep
overwrite.type
as optional ifoverwrite.id
is a member object or role object (current behaviour), but enforce it as mandatory ifoverwrite.id
is a Snowflake.
This PR implements the latter approach. It validates overwrite.id
and overwrite.type
as GuildMemberResolvable|RoleResolvable
and OverwriteType|undefined
respectively, then:
- if
overwrite.id
is a Snowflake, it sets the type of the result tooverwrite.type
, throwing an error if it's not defined; - if
overwrite.id
is a member/role object, it sets the type of the result to the OverwriteType corresponding to that object.- There's then the question of what to do if
overwrite.type
is provided, but doesn't match the type ofoverwrite.id
. This changeset plumps for throwing a validation error if that occurs, since silently overruling the type provided to the function without warning the user could lead to some hard-to-debug quirks.
- There's then the question of what to do if
The same cache-dependent type
behaviour is also present on PermissionOverwriteManager#upsert()
, and the behaviour of PermissionOverwriteManager#edit()
is also seemingly unnecessarily cache-dependent. Think these should also be updated, but will wait for feedback on the PR so far.
Status and versioning classification:
- Code changes have been tested against the Discord API, or there are no code changes
- I know how to update typings and have done so, or typings don't need updating
- This PR includes breaking changes (methods removed or renamed, parameters moved or removed)