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

Created new OptionConfig class (child of Option) to include config ob… #1643

Closed

Conversation

the-unfactoring-guru
Copy link

@the-unfactoring-guru the-unfactoring-guru commented Nov 20, 2021

Pull Request

Problem

Adding a new form of creating an option object. This is to simplify work when creating multiple options

Solution

Create an OptionConfig class that extends Option class, the only difference is that this new class recieves a config object with necessary fields.

Since TS and JS are really peeky with multiple constructors in one class, I chose this option.

index.d.ts

//...
 export class OptionConfig extends Option{
    constructor(config: {flags: string; description?: string});
}
//...

option.js

class OptionConfig extends Option {
  /**
   * Initialize a new `Option` with the given `config` object
   *
   * @param {{flags: string; description?: string}} config
   */
  constructor(config) {
    super(config.flags, config.description);
  }
}

ChangeLog

@the-unfactoring-guru
Copy link
Author

References and improves on #1642

@shadowspawn
Copy link
Collaborator

Related: #665 #1359

@shadowspawn
Copy link
Collaborator

Adding a new form of creating an option object. This is to simplify work when creating multiple options

I am not sure I understand the problem you have in mind. Can you give an example of how OptionConfig will make it easier to create multiple options?

@shadowspawn
Copy link
Collaborator

I think a factory function might be a more appropriate approach than a subclass for the implementation. e.g.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/from

For a fuller implementation, issues have mentioned short and long, and an alternative approach to setup to avoid the difference between <required> and [optional].

As it is, this PR can easily be added in client code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants