Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Options-first Command and EventListener constructors #98

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

eritbh
Copy link
Owner

@eritbh eritbh commented Aug 30, 2021

Fixes #96. Constructors for Command and EventListener currently take an optional object argument for additional options. However, because these both have required function arguments that can have long bodies, it can hurt readability to have options that impact how the function is run at the very bottom of the constructor, especially when working with single-file commands.

This PR adds support for passing the the object argument before the function argument in both these cases, and deprecates the signatures that put the function argument first. Signatures still exist to allow passing no object argument; both these constructors treat that case identically to passing an empty object.

  • Command constructor
  • CommandWithHelp constructor
  • EventListener constructor
  • Migrate internal usage in default commands
what the deprecation will say

Deprecation: Object-last Client and EventListener constructor signatures

The `Command` and `EventListener` constructors both take an optional object argument containing additional options. #98 introduced new forms for these constructors, which place the options argument before the function argument, and deprecated the old forms which place the options argument at the end of the argument list.

Deprecated forms can be replaced by simply switching the position of the arguments, like so:

```js
// old
new Command('test', message => {
  // ...
}, {
  guildOnly: true,
  permissions: ['manageMessages'],
})
new EventListener('ready', () => {
  // ...
}, {once: true})

// new
new Command('test', {
  guildOnly: true,
  permissions: ['manageMessages'],
}, message => {
  // ...
})
new EventListener('ready', {once: true}, () => {
  // ...
})
```

Omitting the object argument entirely is still supported:

```js
new Command('ping', message => {
  // ...
})
new EventListener('messageCreate', message => {
  // ...
})
```

The object-last constructor signatures of `Command` and `EventListener` will be removed in the 3.0.0 release.

@eritbh eritbh added changes: api modifies behavior of the public API quality removes something unnecessary or updates code style version: minor semver-minor, involves feature addition labels Aug 30, 2021
@eritbh
Copy link
Owner Author

eritbh commented Sep 6, 2021

Blocked on #84, I ain't dealing with that EventEmitter constructor until it's reasonable

@eritbh eritbh added blocked another issue needs to be resolved first blocked upstream an issue in another project needs to be resolved first labels Sep 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked upstream an issue in another project needs to be resolved first blocked another issue needs to be resolved first changes: api modifies behavior of the public API quality removes something unnecessary or updates code style version: minor semver-minor, involves feature addition
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move options object arguments before function arguments in Command/EventListener
1 participant