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

warn if key is param of component render function #421

Merged
merged 2 commits into from Jul 12, 2021
Merged

warn if key is param of component render function #421

merged 2 commits into from Jul 12, 2021

Conversation

rmorshea
Copy link
Collaborator

@rmorshea rmorshea commented Jul 7, 2021

previously key was a required parameter if it was given when constructing the component. initially we thought that just making it optional would be fine. however, after some thought, this could lead to unexpected behavior when users don't want 'key' to indicate identity - if it were optional the user would not get any warning when this confusion occurred. As a result we will warn the user about this until, in a future release we raise an exception.

that is, if `key` is a kwarg of the function, it will
be given as None when no key was specified, and the
given value otherwise.
previously key was a required parameter if it was given. initially
we thought that just making it optional would be fine. however,
after some thought, this could lead to unexpected behavior when
users don't want 'key' to indicate identity - if it were optional
the user would not get any warning when this confusion occured. As
a result we will warn the user about this until, in a future
release we raise an exception.
@rmorshea rmorshea changed the title component key only given if requested warn if key is param of component render function Jul 12, 2021
@rmorshea rmorshea merged commit e449122 into main Jul 12, 2021
@rmorshea rmorshea deleted the fix-420 branch July 12, 2021 23:22
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

1 participant