Skip to content

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

Closed
brn wants to merge 1 commit into
tj:masterfrom
brn:master
Closed

Fix options key name conflict with commander object's' key#673
brn wants to merge 1 commit into
tj:masterfrom
brn:master

Conversation

@brn

@brn brn commented Aug 3, 2017

Copy link
Copy Markdown

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

brn commented Aug 24, 2017

Copy link
Copy Markdown
Author

any update?

@segfaultmedaddy

Copy link
Copy Markdown
Collaborator

Potentially breaking change, preserve until v3.0

@shadowspawn

Copy link
Copy Markdown
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
Copy Markdown
Collaborator

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

@shadowspawn

Copy link
Copy Markdown
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

shadowspawn commented Nov 26, 2019

Copy link
Copy Markdown
Collaborator

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.

4 participants