Skip to content

Commit

Permalink
Merge be84180 into 9e07832
Browse files Browse the repository at this point in the history
  • Loading branch information
JoviDeCroock committed Apr 20, 2019
2 parents 9e07832 + be84180 commit 1d0cfcc
Show file tree
Hide file tree
Showing 5 changed files with 122 additions and 15 deletions.
41 changes: 30 additions & 11 deletions demo/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

47 changes: 46 additions & 1 deletion hooks/test/browser/useMemo.test.js
@@ -1,4 +1,3 @@
import { setupRerender } from 'preact/test-utils';
import { createElement as h, render } from 'preact';
import { setupScratch, teardown } from '../../../test/_util/helpers';
import { useMemo } from '../../src';
Expand Down Expand Up @@ -42,4 +41,50 @@ describe('useMemo', () => {
expect(memoFunction).to.have.been.calledTwice;
});

it('short circuits diffing for memoized components', () => {
let spy = sinon.spy();
let spy2 = sinon.spy();
const X = ({ count }) => {
spy();
return <span>{count}</span>;
};

const Y = ({ count }) => {
spy2();
return <p>{count}</p>;
};

const App = ({ x }) => {
const y = useMemo(() => <Y count={x} />, [x]);
return (
<div>
<X count={x} />
{y}
</div>
);
};

render(<App x={0} />, scratch);
expect(spy).to.be.calledOnce;
expect(spy2).to.be.calledOnce;
expect(scratch.innerHTML).to.equal('<div><span>0</span><p>0</p></div>');

render(<App x={0} />, scratch);
expect(spy).to.be.calledTwice;
expect(spy2).to.be.calledOnce;
expect(scratch.innerHTML).to.equal('<div><span>0</span><p>0</p></div>');

render(<App x={1} />, scratch);
expect(spy).to.be.calledThrice;
expect(spy2).to.be.calledTwice;
expect(scratch.innerHTML).to.equal('<div><span>1</span><p>1</p></div>');

render(<App x={1} />, scratch);
expect(spy2).to.be.calledTwice;
expect(scratch.innerHTML).to.equal('<div><span>1</span><p>1</p></div>');

render(<App x={2} />, scratch);
expect(spy2).to.be.calledThrice;
expect(scratch.innerHTML).to.equal('<div><span>2</span><p>2</p></div>');
});
});
7 changes: 5 additions & 2 deletions src/create-element.js
Expand Up @@ -46,9 +46,11 @@ export function createElement(type, props, children) {
* diffing it against its children
* @param {import('./internal').VNode["ref"]} ref The ref property that will
* receive a reference to its created child
* @param {import('./internal').VNode} original The original vnode
* this instance was created from
* @returns {import('./internal').VNode}
*/
export function createVNode(type, props, text, key, ref) {
export function createVNode(type, props, text, key, ref, original) {
// V8 seems to be better at detecting type shapes if the object is allocated from the same call site
// Do not inline into createElement and coerceToVNode!
const vnode = {
Expand All @@ -57,6 +59,7 @@ export function createVNode(type, props, text, key, ref) {
text,
key,
ref,
_original: original,
_children: null,
_dom: null,
_lastDomChild: null,
Expand Down Expand Up @@ -94,7 +97,7 @@ export function coerceToVNode(possibleVNode) {

// Clone vnode if it has already been used. ceviche/#57
if (possibleVNode._dom!=null) {
let vnode = createVNode(possibleVNode.type, possibleVNode.props, possibleVNode.text, possibleVNode.key, null);
let vnode = createVNode(possibleVNode.type, possibleVNode.props, possibleVNode.text, possibleVNode.key, null, possibleVNode._original || possibleVNode);
vnode._dom = possibleVNode._dom;
return vnode;
}
Expand Down
6 changes: 5 additions & 1 deletion src/diff/index.js
Expand Up @@ -143,7 +143,11 @@ export function diff(dom, parentDom, newVNode, oldVNode, context, isSvg, excessD
if (options.render) options.render(newVNode);

let prev = c._prevVNode || null;
let vnode = c._prevVNode = coerceToVNode(c.render(c.props, c.state, c.context));
let vnode = c._prevVNode;
if (newVNode._original!==oldVNode) {
vnode = c._prevVNode = coerceToVNode(c.render(c.props, c.state, c.context));
}

c._dirty = false;

if (c.getChildContext!=null) {
Expand Down
36 changes: 36 additions & 0 deletions test/browser/render.test.js
Expand Up @@ -820,6 +820,42 @@ describe('render()', () => {
expect(scratch.innerHTML).to.contain(`<span>${todoText}</span>`);
});

it.skip('equal vnodes should short-circuit diffing', () => {
let i = 0;
const spy = sinon.spy();

class X extends Component {
render(props) {
i++;
spy();
return <div>{i}</div>;
}
}
const A = <X />;
class App extends Component {
componentDidMount() {
this.setState({}); // eslint-disable-line
}

componentDidUpdate() {
if (i===1) {
this.setState({}); // eslint-disable-line
}
}

render(props) {
return <div>{A}</div>;
}
}

render(<App />, scratch);
expect(spy).to.be.calledOnce;
expect(scratch.textContent).to.equal('1');
rerender();
expect(scratch.textContent).to.equal('1');
expect(spy).to.be.calledOnce;
});

it('should always diff `checked` and `value` properties against the DOM', () => {
// See https://github.com/developit/preact/issues/1324

Expand Down

0 comments on commit 1d0cfcc

Please sign in to comment.