framework
framework copied to clipboard
request: easier syntax for combining preconditions
Is your feature request related to a problem? Please describe.
Currently the way to combine preconditions is using nested arrays such as: [['AdminOnly', ['ModOnly', 'OwnerOnly']], 'InVoiceChannel'];. This is extremely developer unfriendly because it's extremely chaotic and you have to thoroughly know how preconditions are parsed to start making the least amount of sense of this array.
Describe the solution you'd like
We can take a note out of how other libraries handle cases like this, such as Prisma who has $and, $or, and $not to better build up the preconditions list.
Describe alternatives you've considered
Using what we have no? Nah.
Additional context
https://github.com/sapphiredev/website/pull/91/files/a56d8a9c7f2ac760ea394241092fcdcbc583afa8#diff-7cc7d069f52b3fcc7bf7039c05a4988bd8f895d16731450c21c391e1d0c06565
Wasn't using a builder class also considered a while ago?
I don't recall it but I can see it having some merit as well.
Wait also can't you do { mode: PreconditionRunMode.Or, entries: [a, b] } and stuff
Why not have a static class WhateverPreconditions and have methods and, or and not just like prisma
I'd think a helper class would be easy enough, it would only be something like this:
type PreconditionString = /* something */ string;
type RecursivePreconditionStrings = (PreconditionString | RecursivePreconditionStrings)[];
class PreconditionList extends null {
public static and(...entries: RecursivePreconditionStrings) {
return entries;
}
public static or(...entries: RecursivePreconditionStrings) {
return entries;
}
}
// ["0", ["1", "2", ["3", "4"]]]
PreconditionList.and("0", PreconditionList.or("1", "2", PreconditionList.and("3", "4")));
I guess the problem with that is there's nothing stopping a user from doing two of the same (.and/.or) in a row, which would give unexpected results. However, that wouldn't be able to happen if we used an object: (excuse the messy types)
type PreconditionString = /* something */ string;
type RecursivePreconditionList = (PreconditionString | { $or?: (PreconditionString | { $and?: RecursivePreconditionList })[] })[];
type Preconditions = (string | Preconditions)[];
const resolvePreconditionList = (list: RecursivePreconditionList): Preconditions => {
return list.map((element) => {
if (typeof element === 'string') {
return element;
}
if (Array.isArray(element)) {
return resolvePreconditionList(element);
}
return resolvePreconditionList(Object.values(element)[0] as RecursivePreconditionList);
})
}
// ["0", ["1", ["2", ["3", "4"]]]]
resolvePreconditionList(['0', { $or: ['1', { $and: ['2', { $or: ['3', '4'] }] }] }])
The object design is also less verbose, so I think that's the best way to go. Thoughts?
The idea is interesting. Instead of using a class extending null we can also leverage TS namespaces:
export namespace PreconditionList {
export function and(...entries: RecursivePreconditionStrings) {
return entries;
}
export function or(...entries: RecursivePreconditionStrings) {
return entries;
}
}
I prefer this over an object tbh. Sure an object may be typing less but verbosity adds clarity as well.
I understand where you're coming from, but honestly in this case, I don't think a leading PreconditionList.* (or whatever we'd call it) would add any clarity. People would already know it's a precondition list by seeing preconditions: [...]. And i don't know how the $and and $or could be misconstrued out of the intended context.
However, if we do go with functions, I agree namespaces (or just plain objects) would be better.