Skip to content

Commit

Permalink
fix: #285 react-css-modules causes modifying key property of the comp…
Browse files Browse the repository at this point in the history
…onent children (#286)
  • Loading branch information
vitaliymaz authored and gajus committed Aug 10, 2018
1 parent 54b4367 commit 982fcf5
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/linkClass.js
Expand Up @@ -50,7 +50,7 @@ const linkElement = (element: ReactElement, styles: Object, configuration: Objec
if (React.isValidElement(children)) {
elementShallowCopy.props.children = linkElement(React.Children.only(children), styles, configuration);
} else if (_.isArray(children) || isIterable(children)) {
elementShallowCopy.props.children = React.Children.map(children, (node) => {
elementShallowCopy.props.children = _.map(children, (node) => {

This comment has been minimized.

Copy link
@evisong

evisong Sep 6, 2018

This change breaks Iterable support, lodash.map() doesn't support Iterable.

Please refer to the following test:

import { List } from 'immutable';
import { map } from 'lodash';

const myList = new List([1, 2, 3]);
const Demo = () => (
  <div>
    {map(myList, i => <p key={i}>{i}</p>)}
  </div>
);

export default Demo;

Which would prompt error in console:

Uncaught (in promise) Error: Objects are not valid as a React child (found: object with keys {array, ownerID}). If you meant to render a collection of children, use an array instead or wrap the object using createFragment(object) from the React add-ons. Check the render method of Demo.

This comment has been minimized.

Copy link
@gajus

gajus Sep 6, 2018

Owner

Can you submit a fix and test case, please?

This comment has been minimized.

Copy link
@NelsonBrandao

NelsonBrandao Dec 7, 2018

Likewise it also break the Iterable for immutable.js List

if (React.isValidElement(node)) {
// eslint-disable-next-line no-use-before-define
return linkElement(React.Children.only(node), styles, configuration);
Expand Down
16 changes: 16 additions & 0 deletions tests/linkClass.js
Expand Up @@ -141,6 +141,22 @@ describe('linkClass', () => {
expect(subject.props.children[0].props).not.to.have.property('styleName');
expect(subject.props.children[1].props).not.to.have.property('styleName');
});
it('preserves original keys', () => {
let subject;

subject = <div>
<p key='1' styleName='foo' />
<p key='2' styleName='bar' />
</div>;

subject = linkClass(subject, {
bar: 'bar-1',
foo: 'foo-1'
});

expect(subject.props.children[0].key).to.equal('1');
expect(subject.props.children[1].key).to.equal('2');
});
});
context('when multiple descendants have styleName and are iterable', () => {
it('assigns a generated className', () => {
Expand Down

0 comments on commit 982fcf5

Please sign in to comment.