Skip to content

DropdownToggle unknown prop warning#220

Closed
bskimball wants to merge 4 commits intoreactstrap:masterfrom
bskimball:master
Closed

DropdownToggle unknown prop warning#220
bskimball wants to merge 4 commits intoreactstrap:masterfrom
bskimball:master

Conversation

@bskimball
Copy link
Copy Markdown

React throws unknown prop warning when using the tag prop. This was due to the tag prop being set twice in the render method. I added componentWillMount in order to evaluate the tag prop before the component is rendered. I also removed the de-structured props variable.

bskimball added 4 commits November 4, 2016 09:00
React throws unknown prop warning when using the tag prop. This was due to the tag prop being set twice in the render method. I added componentWillMount in order to evaluate the tag prop before the component is rendered. I also removed the de-structured props variable.
@TheSharpieOne
Copy link
Copy Markdown
Member

TheSharpieOne commented Nov 4, 2016

First, thanks for find this!
The problem is that while the tag is pulled out of the props during the destructuring, it is later assigned back into the props when nav is true
The fix here is to not set tag in/on props when nav is true, but instead to change the Tag variable which was defined in the destructuring. This means that the destructuring assignment cannot be to a const variable.

EDIT
Actually, I see the intent here. It was to make the Button have a tag of 'a'. In this case, there should be an aditional check that the Tag has not changed and is the default, Button before setting the tag which will be passed.
To break it down, setting tag on DropdownToggle will with nav set to true can cause issues.

@bskimball
Copy link
Copy Markdown
Author

bskimball commented Nov 4, 2016

I see what you mean. The easy fix would be to change const to let on props de-structure or evaulate the tag prop again.

Would that be preferred to evaluating the tag prop in componentWillMount method?

I see you are already fixing the issue. Should I stop on my pull request?

@TheSharpieOne
Copy link
Copy Markdown
Member

Yes, you can close this. The change was simple (1 liner + a test to ensure there is no regression) after further analyzing the code and issue to determine the intent of setting the tag that way.

@bskimball bskimball closed this Nov 4, 2016
@eddywashere
Copy link
Copy Markdown
Member

fix released in 3.9.0

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.

3 participants