Skip to content

DomToReact: Allow custom keys for replacement#99

Merged
remarkablemark merged 2 commits intoremarkablemark:masterfrom
NadiaIvaniuckovich:dom-to-react-custom-keys
Mar 29, 2019
Merged

DomToReact: Allow custom keys for replacement#99
remarkablemark merged 2 commits intoremarkablemark:masterfrom
NadiaIvaniuckovich:dom-to-react-custom-keys

Conversation

@NadiaIvaniuckovich
Copy link
Copy Markdown
Contributor

In current implementation when you use dom-to-react with replace option created elements usually have keys generated from index of node, even if component in replace have it's own key.

According to https://fb.me/react-warning-keys
"We don’t recommend using indexes for keys if the order of items may change. This can negatively impact performance and may cause issues with component state. "

In our case it cause problems with performance.
This pull request will allow using keys from replacement components (for example id of element can be used as key if put it as key to component returned by replace).

@coveralls
Copy link
Copy Markdown

coveralls commented Mar 27, 2019

Coverage Status

Coverage remained the same at 99.286% when pulling 4eec53e on NadiaIvaniuckovich:dom-to-react-custom-keys into 570bf1a on remarkablemark:master.

Copy link
Copy Markdown
Owner

@remarkablemark remarkablemark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Just some minor nitpicks if you don't mind updating

);
});

it('does not modify keys for replacement if it have one', () => {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: s/have/has

return React.createElement(
'custom-button',
{
key: 'meyKey',
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: s/meyKey/myKey

React.createElement(
'custom-button',
{
key: 'meyKey',
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: s/meyKey/myKey

@remarkablemark remarkablemark merged commit 385f88f into remarkablemark:master Mar 29, 2019
@remarkablemark
Copy link
Copy Markdown
Owner

@NadiaIvaniuckovich Thanks for making the pull request! I'll make a release later tonight.

@remarkablemark
Copy link
Copy Markdown
Owner

html-react-parser@0.6.4 has been published

d-lazarev pushed a commit to d-lazarev/html-react-parser that referenced this pull request Apr 5, 2019
…eact-custom-keys

DomToReact: Allow custom keys for replacement
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