Skip to content

Remove key parameter from replace and use React.cloneElement instead #18

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

Merged
merged 3 commits into from
Aug 30, 2016

Conversation

remarkablemark
Copy link
Owner

Continuation of #16

Issue:

The previous fix to mitigate the React unique keys warning was to pass the key as the 2nd parameter in the replace method.

However, this burdens users with the extra work and responsibility to set the "key" prop on their custom React element.

Enhancement:

The smarter approach done in this pull request was to use React.cloneElement to clone the user's custom React element and merge the key prop for them (when applicable).

Tasks:

  • Remove the key parameter in replace method in README.md
  • Update .eslintrc

The previous implementation was to pass the `key` as the 2nd
parameter in `replace` method and force the user to set the
"key" prop in their custom React element.

This creates further complexity for the end-user.

As a result, use `React.cloneElement` which does the same thing
without additional work from the user.

`React.cloneElement` takes the React element and merges the
"key" prop taken from the array index with the original
element's props when applicable.

https://facebook.github.io/react/docs/top-level-api.html#react.cloneelement
Check all variables for usage but do not check function arguments.
@remarkablemark remarkablemark self-assigned this Aug 30, 2016
@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 89.573% when pulling 26e4e7c on remove-replace-key into ddd289b on master.

@remarkablemark remarkablemark merged commit 7522a0a into master Aug 30, 2016
@remarkablemark remarkablemark deleted the remove-replace-key branch August 30, 2016 02:45
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.

2 participants