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

Fix options key name conflict with commander object's' key #673

Closed
wants to merge 1 commit into from

Conversation

brn
Copy link

@brn brn commented Aug 3, 2017

Fix options key name confliction #404, #648 to use Object.create(null) and commander.opts() return object that has only options key value.
Object.create(null) is create object that hasn't any prototype, so it can hold any key without confiliction.

But, I'm warried about that it will become problem.
Object.create(null) hasn't any prototype chain, so if user extend Object.prototype and expect opts() result object is extended too, it will become problem.
What do you think of that?

@brn
Copy link
Author

brn commented Aug 24, 2017

any update?

@roman-vanesyan
Copy link
Collaborator

Potentially breaking change, preserve until v3.0

@shadowspawn
Copy link
Collaborator

Name clashes is one of the pain points with commander!

This pull request addresses just one aspect, with keeping track of the option values separate from the commander object. However, writing the option values onto the commander object is still going to cause clashes in that direction:

i.e. the lines like this:

self[name] = optionKeyValueHolder[name] = val;

(I am currently wondering about adding a feature-flag to track the values separately, as you are doing here, and not write them onto the commander object at all.)

@abetomo abetomo requested a review from shadowspawn April 2, 2019 09:34
@shadowspawn
Copy link
Collaborator

See also #515 which removed the writing to the command object but added more syntax.

@shadowspawn
Copy link
Collaborator

I am closing this in favour of #951, which builds on work in this Pull Request and aims for backwards compatibility.

Thank you for your contributions.

@shadowspawn
Copy link
Collaborator

shadowspawn commented Nov 26, 2019

I have opened a new Pull Request which allows storing option values separately rather than as command properties (access using .opts()), and passes the options (rather than the command) to the action handler.

See #1102

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

Successfully merging this pull request may close these issues.

None yet

4 participants