Skip to content

Commit

Permalink
(fix) - resolve attributes of reused nodes that weren't present in vd…
Browse files Browse the repository at this point in the history
…om (#1987)

* (fix) - resolve attributes of reused nodes that weren't present in vdom

* (chore) - move test to use hydrate this does make sense
  • Loading branch information
JoviDeCroock committed Oct 15, 2019
1 parent b32c12a commit 81ac3c4
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 16 deletions.
8 changes: 8 additions & 0 deletions src/diff/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ function diffElementNodes(dom, newVNode, oldVNode, context, isSvg, excessDomChil
if (dom==null && excessDomChildren!=null) {
for (i=0; i<excessDomChildren.length; i++) {
const child = excessDomChildren[i];

if (child!=null && (newVNode.type===null ? child.nodeType===3 : child.localName===newVNode.type)) {
dom = child;
excessDomChildren[i] = null;
Expand Down Expand Up @@ -236,6 +237,13 @@ function diffElementNodes(dom, newVNode, oldVNode, context, isSvg, excessDomChil
// During hydration, props are not diffed at all (including dangerouslySetInnerHTML)
// @TODO we should warn in debug mode when props don't match here.
if (!isHydrating) {
if (oldProps === EMPTY_OBJ) {
oldProps = {};
for (let i=0; i<dom.attributes.length; i++) {
oldProps[dom.attributes[i].name] = dom.attributes[i].value;
}
}

if (newHtml || oldHtml) {
// Avoid re-applying the same '__html' if it did not changed between re-render
if (!newHtml || !oldHtml || newHtml.__html!=oldHtml.__html) {
Expand Down
16 changes: 16 additions & 0 deletions test/browser/hydrate.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,22 @@ describe('hydrate()', () => {
expect(getLog()).to.deep.equal([]);
});

it('should not merge attributes with node created by the DOM', () => {
const html = (htmlString) => {
const div = document.createElement('div');
div.innerHTML = htmlString;
return div.firstChild;
};

const DOMElement = html`<div><a foo="bar"></a></div>`;
scratch.appendChild(DOMElement);

const preactElement = <div><a /></div>;

hydrate(preactElement, scratch);
expect(scratch).to.have.property('innerHTML', '<div><a foo="bar"></a></div>');
});

it('should attach event handlers', () => {
let spy = sinon.spy();
scratch.innerHTML = '<span>Test</span>';
Expand Down
50 changes: 34 additions & 16 deletions test/browser/render.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,40 @@ describe('render()', () => {
expect(style.zIndex.toString()).to.equal('');
});

it('should remove existing attributes', () => {
const div = document.createElement('div');
div.setAttribute('class', 'red');
const span = document.createElement('span');
const text = document.createTextNode('Hi');

span.appendChild(text);
div.appendChild(span);
scratch.appendChild(div);

const App = () => (
<div>
<span>Bye</span>
</div>
);

render(<App />, scratch);
expect(scratch.innerHTML).to.equal('<div class=""><span>Bye</span></div>');
});

it('should remove class attributes', () => {
const App = props => (
<div className={props.class}>
<span>Bye</span>
</div>
);

render(<App class="hi" />, scratch);
expect(scratch.innerHTML).to.equal('<div class="hi"><span>Bye</span></div>');

render(<App />, scratch);
expect(scratch.innerHTML).to.equal('<div class=""><span>Bye</span></div>');
});

it('should remove old styles', () => {
render(<div style={{ color: 'red' }} />, scratch);
render(<div style={{ backgroundColor: 'blue' }} />, scratch);
Expand Down Expand Up @@ -782,22 +816,6 @@ describe('render()', () => {
expect(scratch.firstChild.lastChild).to.equalNode(a);
});

it('should not merge attributes with node created by the DOM', () => {
const html = (htmlString) => {
const div = document.createElement('div');
div.innerHTML = htmlString;
return div.firstChild;
};

const DOMElement = html`<div><a foo="bar"></a></div>`;
scratch.appendChild(DOMElement);

const preactElement = <div><a /></div>;

render(preactElement, scratch);
expect(scratch).to.have.property('innerHTML', '<div><a foo="bar"></a></div>');
});

// Discussion: https://github.com/preactjs/preact/issues/287
// <datalist> is not supported in Safari, even though the element
// constructor is present
Expand Down

0 comments on commit 81ac3c4

Please sign in to comment.