feat: add ExtendedPrecondition

[Lioness100]

This PR adds the ExtendedPrecondition class, where you can define a precondition to extend. Wherever the extended precondition is run, the base will be run first, and execution will only continue if the base returns Ok. This is for performing operations that depend on another precondition passing, and imo is easier and more readable than just adding both preconditions sequentially every time you want to use the second one.

I basically just copied ExtendedArgument and changed Argument to Precondition but I have a few thoughts after looking through the source code a bit more:

• Since listeners, commands, arguments, and preconditions have a generic type O for the options that is then passed into Store<O>, shouldn't the type for options in the constructor also be O?
• I feel like I remember someone saying something about the new docusaurus setup and how (at) no longer needs to replace @?
• Following the naming convention that came from idek but I like naming conventions because I'm indecisive, shouldn't arguments be named User* in the JSDoc?
• Are in depth JSDoc descriptions even necessary anymore if we can just make guides? Like "Just like with Argument, you can use export default or export = too." seems pretty guide like which doesn't fit in API docs. Also, why does ExtendedArgument get jsdoc guides but not Command, Listener, etc? They feel left out :(
• Why does PreconditionContext extend a record instead of fetching context types from the Preconditions interface which would be much safer?
• Why are all the internal identifiers camel case but the ones in examples are pascal case?
• Conceptually, the return type of a precondition shouldn't matter, so it shouldn't be passed into the #handle function, right?
• Why does Precondition use a namespace for its affiliated interfaces but none of the other stores do?

Sorry for my thought vomit lol

This PR is untested

[kyranet]

While there's a use for this, I don't know if it's worth the trouble, you see, you can define an array of entries to require one step to pass before the former, so if you have ['GuildOnly', 'Moderator'], it'll check for GuildOnly before Moderator, so you know Moderator will always receive a message from a guild.

ExtendedPrecondition just binds the two together and can lead to inefficient paths due to the duplication of checks, but I'll leave this to the other two.

[Lioness100]

That's true, but if you add the Moderator precondition to a lot of commands, it could be quite tiresome. Furthermore, with the syntax of extended argument, it can make it perfectly clear to other readers that it depends on another, and it's not just a coincidence that they are coupled together in commands. If a contributor adds in one precondition but doesn't realize they need the other, insecurities will open up in the bot.

Lastly, although a very rare case, if you have a precondition that depends on another that depends on another, it will be very tedious to list them out each time. With extended preconditions, the method to do so will be super easy no matter how long the chain gets.

[kyranet]

That's why you define abstractions in your commands to push those entries, just like Sapphire's Command class does.

[Lioness100]

Yeah, that would definitely be my alternative, and I'm fine if that's the direction you guys think we should go in. I just find it a bit hacky, and would always prefer a built-in system.

[Lioness100]

@vladfrangu @favna any thoughts?

[vladfrangu]

I don't see the use for this when you can do something like this

// maybe we should export our preconditions...
import { CoreGuildOnlyPrecondition, isErr } from '@sapphire/framework';

export class UserPrecondition extends CoreGuildOnlyPrecondition {
	public override async run(...args: VladIsLazyTheseAreTheArgs[]) {
		const baseResult = await super.run(...args);
		if (isErr(baseResult)) return baseResult;
		
		// My custom checks
		return this.ok()
	}
}

So, I'm with kyra on this one

[Lioness100]

Why didn't I think of that lmao that's so simple

But then why do we need ExtendedArgument either?

[vladfrangu]

ExtendedArgument exists..but is not even used by us... @kyranet?

[favna]

ExtendedArgument exists..but is not even used by us... @kyranet?

It was kept for backwards compatibility and for users who like to use it, but we removed it internally when we switched to resolvers because they could achieve the same anyway.

[favna]

Other than that, I'm with @kyranet and @vladfrangu

[favna]

@Lioness100 as for your brain dumps:

I feel like I remember someone saying something about the new docusaurus setup and how (at) no longer needs to replace @?

The (at) for decorators is because otherwise VSCode IntelliSense breaks. The docusaurus based docs don't really change that. I'm probably going to go to JSON route similar to discord.js.org now that Crawl has figured out how to do it for TypeScript after which I can do a string replace of (at) to @ but that's beside the point.

Following the naming convention that came from idek but I like naming conventions because I'm indecisive, shouldn't arguments be named User* in the JSDoc?

I'm not sure to what part of the docs you're referring to. Anyway, the User* part comes from being an opposite of Core*. User = the developer, Core = the framework. And the reason Skyra uses that pattern is so when resolving the type of a store with an eval command and using @sapphire/type there aren't 10000000000 different types, but just 1, UserArgument.

Are in depth JSDoc descriptions even necessary anymore if we can just make guides?

Yes, because they show up in IntelliSense. If anything in depth JSDoc descriptions can be copied to guides, but because IntelliSense is provided right in the IDE this will remain to be valueable.

Also, why does ExtendedArgument get jsdoc guides but not Command, Listener, etc

Because that still needs to be written 🤔. PRs for documentation (be that website / guides, or JSDoc) are very, very welcome and appreciated.

Why does PreconditionContext extend a record instead of fetching context types from the Preconditions interface which would be much safer?

That's something for @kyranet to answer.

Why are all the internal identifiers camel case but the ones in examples are pascal case

Can you point to some examples? It might be typos, it might be intentional. I'm not sure.

Conceptually, the return type of a precondition shouldn't matter, so it shouldn't be passed into the #handle function, right?

In light of the responses above, I'll skip this one.

Why does Precondition use a namespace for its affiliated interfaces but none of the other stores do?

That's actually a good remark. We could add that for the other pieces. Feel free to make a PR for this.

[vladfrangu]

ExtendedArgument exists..but is not even used by us... @kyranet?

It was kept for backwards compatibility and for users who like to use it, but we removed it internally when we switched to resolvers because they could achieve the same anyway.

Let's deprecate it in v2, and remove in v3 please.

[favna]

ExtendedArgument exists..but is not even used by us... @kyranet?

It was kept for backwards compatibility and for users who like to use it, but we removed it internally when we switched to resolvers because they could achieve the same anyway.

Let's deprecate it in v2, and remove in v3 please.

#321

[Lioness100]

Following the naming convention that came from idek but I like naming conventions because I'm indecisive, shouldn't arguments be named User* in the JSDoc?

I'm not sure to what part of the docs you're referring to. Anyway, the User* part comes from being an opposite of Core*. User = the developer, Core = the framework. And the reason Skyra uses that pattern is so when resolving the type of a store with an eval command and using @sapphire/type there aren't 10000000000 different types, but just 1, UserArgument.

My bad! I wasn't specific. I was referencing that in the examples in the JSDoc for ExtendedArgument, it named the class TextChannelArgument instead of User*

Why are all the internal identifiers camel case but the ones in examples are pascal case

Can you point to some examples? It might be typos, it might be intentional. I'm not sure.

Again, this was referring to the examples in the JSDoc for ExtendedArgument which used the identifier 'ArgumentTextChannelInvalidTextChannel'. This may be because ExtendedArgument was implemented in 1.0.0-alpha.0, but in 1.0.0-alpha.7, all pascal case identifiers were changed to reference pascal case property names on the Identiefers object? enum? which instead pointed to camel case identitifers.

Thank you for all the other answers to my brain dump! As for PRs, I'd be happy to make one for namespaces of Command, Listener, and Argument (Maybe pieces in the plugins as well?). One question about that: should I change the usage of each piece in the internals to use the respective namespace instead of the individual imports? When should, for example, Precondition.Options be used instead of PreconditionOptions?

I'll also try to help with guides sometime in the future, but it's not something I can do right now :(. Lastly, Vlad mentioned in their code that it might be worthwhile to export internal preconditions? Could I also make a PR for this, and if so, is there a certain format you guys would prefer?

Thanks! ☺️