Merge pull request #52 from developit/ref3

Properly add support for ref + createRef

src/create-element.js

@@ -30,6 +30,10 @@ function createVNode(tag, props, text, key, ref) {
 	return { tag, props, text, key, ref, _children: null, _el: null, _component: null };
 }
 
+export function createRef() {
+	return {};
+}
+
 export function Fragment(props) {
 	return props.children;
 }

src/diff/index.js

@@ -262,6 +262,8 @@ export function diff(dom, parent, newTree, oldTree, context, isSvg, append, exce
 			else {
 				c.base = dom = diff(dom, parent, vnode, prev, context, isSvg, append, excessChildren, mounts, c);
 			}
+
+			if (newTree.ref) applyRef(newTree.ref, c);
 			// context = assign({}, context);
 			// context.__depth = (context.__depth || 0) + 1;
 			// context = assign({
@@ -303,6 +305,10 @@ export function diff(dom, parent, newTree, oldTree, context, isSvg, append, exce
 		}
 		else {
 			dom = diffElementNodes(dom, parent, newTree, oldTree, context, isSvg, excessChildren, mounts, ancestorComponent);
+
+			if (newTree.ref && (oldTree.ref !== newTree.ref)) {
+				applyRef(newTree.ref, dom);
+			}
 		}
 
 		if (parent && append!==false) {
@@ -327,7 +333,6 @@ export function diff(dom, parent, newTree, oldTree, context, isSvg, append, exce
 		if (clearProcessingException) {
 			c._processingException = null;
 		}
-
 	}
 	catch (e) {
 		if (c && !dom) {
@@ -445,26 +450,29 @@ function diffElementNodes(dom, parent, vnode, oldVNode, context, isSvg, excessCh
 		// diffChildren(dom, newChildren, vnode===oldVNode ? newChildren : oldVNode==null ? [] : getVNodeChildren(oldVNode), context, isSvg, excessChildren);
 		if (vnode!==oldVNode) {
 			diffProps(dom, vnode.props, oldVNode==EMPTY_OBJ ? EMPTY_OBJ : oldVNode.props, isSvg);
-			if (vnode.ref!==oldVNode.ref) {
-				let ref;
-				if (ref = oldVNode.ref) ref(null);
-				if (ref = vnode.ref) ref(dom);
-			}
 		}
 		diffChildren(dom, getVNodeChildren(vnode), oldVNode==EMPTY_OBJ ? EMPTY_ARR : getVNodeChildren(oldVNode), context, isSvg, excessChildren, mounts, ancestorComponent);
 	}
-
 	// if (oldVNode!=null && dom!==d) unmount(oldVNode);
 
 	return dom;
 }
 
+/**
+ * Invoke or update a ref, depending on whether it is a function or object ref.
+ * @param {object|function} [ref=null]
+ * @param {any} [value]
+ */
+export function applyRef(ref, value) {
+	if (typeof ref=='function') ref(value);
+	else ref.current = value;
+}
 
 export function unmount(vnode, ancestorComponent) {
 	let r;
 	if (r = vnode.ref) {
 		try {
-			r(null);
+			applyRef(r, null);
 		}
 		catch (e) {
 			catchErrorInComponent(e, ancestorComponent);

src/index.js

@@ -4,7 +4,7 @@ import { Component } from './component';
 import { cloneElement } from './clone-element';
 
 export { render, hydrate } from './render';
-export { createElement, Fragment } from './create-element';
+export { createElement, Fragment, createRef } from './create-element';
 export { Component } from './component';
 export { cloneElement } from './clone-element';
 

test/TODO.md

@@ -6,6 +6,5 @@
 		- ✖ should NOT mutate state on mount, only create new versions
 	- \#shouldComponentUpdate
 		- ✖ should be passed next props and state
-- refs (***`SKIPPED`***)
 - render
 	- ✖ should skip non-preact elements

test/browser/refs.js

@@ -1,5 +1,5 @@
-import { createElement as h, render, Component } from '../../src/index';
-import { setupScratch, teardown } from '../_util/helpers';
+import { createElement as h, render, Component, createRef } from '../../src/index';
+import { setupScratch, teardown, setupRerender } from '../_util/helpers';
 
 /** @jsx h */
 
@@ -10,11 +10,13 @@ let spy = (name, ...args) => {
 	return spy;
 };
 
-describe.skip('refs', () => {
+describe('refs', () => {
 	let scratch;
+	let rerender;
 
 	beforeEach(() => {
 		scratch = setupScratch();
+		rerender = setupRerender();
 	});
 
 	afterEach(() => {
@@ -27,6 +29,14 @@ describe.skip('refs', () => {
 		expect(ref).to.have.been.calledOnce.and.calledWith(scratch.firstChild);
 	});
 
+	it('should support createRef', () => {
+		const r = createRef();
+		expect(r.current).to.equal(undefined);
+
+		render(<div ref={r} />, scratch);
+		expect(r.current).to.equal(scratch.firstChild);
+	});
+
 	it('should invoke refs in Component.render()', () => {
 		let outer = spy('outer'),
 			inner = spy('inner');
@@ -67,27 +77,27 @@ describe.skip('refs', () => {
 
 		const Foo = () => <div />;
 
-		let root = render(<Foo ref={ref} />, scratch);
+		render(<Foo ref={ref} />, scratch);
 		expect(ref).to.have.been.calledOnce;
 
 		ref.resetHistory();
-		render(<Foo ref={ref} />, scratch, root);
+		render(<Foo ref={ref} />, scratch);
 		expect(ref).to.have.been.calledOnce;
 
 		ref.resetHistory();
-		render(<span />, scratch, root);
+		render(<span />, scratch);
 		expect(ref).to.have.been.calledOnce.and.calledWith(null);
 	});
 
 	it('should pass children to ref functions', () => {
 		let outer = spy('outer'),
 			inner = spy('inner'),
 			InnermostComponent = 'span',
-			rerender, inst;
+			update, inst;
 		class Outer extends Component {
 			constructor() {
 				super();
-				rerender = () => this.forceUpdate();
+				update = () => this.forceUpdate();
 			}
 			render() {
 				return (
@@ -107,31 +117,30 @@ describe.skip('refs', () => {
 			}
 		}
 
-		let root = render(<Outer />, scratch);
+		render(<Outer />, scratch);
 
 		expect(outer).to.have.been.calledOnce.and.calledWith(inst);
 		expect(inner).to.have.been.calledOnce.and.calledWith(inst.base);
 
 		outer.resetHistory();
 		inner.resetHistory();
-
-		rerender();
+		update();
 
 		expect(outer, 're-render').to.have.been.calledOnce.and.calledWith(inst);
 		expect(inner, 're-render').not.to.have.been.called;
 
 		inner.resetHistory();
 		InnermostComponent = 'x-span';
-		rerender();
+		update();
+
 		expect(inner, 're-render swap');
 		expect(inner.firstCall, 're-render swap').to.have.been.calledWith(null);
 		expect(inner.secondCall, 're-render swap').to.have.been.calledWith(inst.base);
-		InnermostComponent = 'span';
 
+		InnermostComponent = 'span';
 		outer.resetHistory();
 		inner.resetHistory();
-
-		render(<div />, scratch, root);
+		render(<div />, scratch);
 
 		expect(outer, 'unrender').to.have.been.calledOnce.and.calledWith(null);
 		expect(inner, 'unrender').to.have.been.calledOnce.and.calledWith(null);
@@ -163,7 +172,7 @@ describe.skip('refs', () => {
 			}
 		}
 
-		let root = render(<Outer ref={outer} />, scratch);
+		render(<Outer ref={outer} />, scratch);
 
 		expect(outer, 'outer initial').to.have.been.calledOnce.and.calledWith(outerInst);
 		expect(inner, 'inner initial').to.have.been.calledOnce.and.calledWith(innerInst);
@@ -172,15 +181,16 @@ describe.skip('refs', () => {
 		outer.resetHistory();
 		inner.resetHistory();
 		innermost.resetHistory();
-		root = render(<Outer ref={outer} />, scratch, root);
+		render(<Outer ref={outer} />, scratch);
 
 		expect(outer, 'outer update').to.have.been.calledOnce.and.calledWith(outerInst);
 		expect(inner, 'inner update').to.have.been.calledOnce.and.calledWith(innerInst);
 		expect(innermost, 'innerMost update').not.to.have.been.called;
 
 		innermost.resetHistory();
 		InnermostComponent = 'x-span';
-		root = render(<Outer ref={outer} />, scratch, root);
+		render(<Outer ref={outer} />, scratch);
+
 		expect(innermost, 'innerMost swap');
 		expect(innermost.firstCall, 'innerMost swap').to.have.been.calledWith(null);
 		expect(innermost.secondCall, 'innerMost swap').to.have.been.calledWith(innerInst.base);
@@ -189,19 +199,22 @@ describe.skip('refs', () => {
 		outer.resetHistory();
 		inner.resetHistory();
 		innermost.resetHistory();
-		render(<div />, scratch, root);
+		render(<div />, scratch);
 
 		expect(outer, 'outer unmount').to.have.been.calledOnce.and.calledWith(null);
 		expect(inner, 'inner unmount').to.have.been.calledOnce.and.calledWith(null);
 		expect(innermost, 'innerMost unmount').to.have.been.calledOnce.and.calledWith(null);
 	});
 
+	// Test for #1143
 	it('should not pass ref into component as a prop', () => {
 		let foo = spy('foo'),
 			bar = spy('bar');
 
 		class Foo extends Component {
-			render(){ return <div />; }
+			render() {
+				return <div />;
+			}
 		}
 		const Bar = spy('Bar', () => <div />);
 
@@ -220,7 +233,7 @@ describe.skip('refs', () => {
 
 	// Test for #232
 	it('should only null refs after unmount', () => {
-		let root, outer, inner;
+		let outer, inner;
 
 		class TestUnmount extends Component {
 			componentWillUnmount() {
@@ -244,14 +257,13 @@ describe.skip('refs', () => {
 
 		sinon.spy(TestUnmount.prototype, 'componentWillUnmount');
 
-		root = render(<div><TestUnmount /></div>, scratch, root);
+		render(<div><TestUnmount /></div>, scratch);
 		outer = scratch.querySelector('#outer');
 		inner = scratch.querySelector('#inner');
 
 		expect(TestUnmount.prototype.componentWillUnmount).not.to.have.been.called;
 
-		render(<div />, scratch, root);
-
+		render(<div />, scratch);
 		expect(TestUnmount.prototype.componentWillUnmount).to.have.been.calledOnce;
 	});
 
@@ -283,14 +295,14 @@ describe.skip('refs', () => {
 		inst.handleMount.resetHistory();
 
 		inst.setState({ show: true });
-		inst.forceUpdate();
+		rerender();
 		expect(inst.handleMount).to.have.been.calledTwice;
 		expect(inst.handleMount.firstCall).to.have.been.calledWith(null);
 		expect(inst.handleMount.secondCall).to.have.been.calledWith(scratch.querySelector('#span'));
 		inst.handleMount.resetHistory();
 
 		inst.setState({ show: false });
-		inst.forceUpdate();
+		rerender();
 		expect(inst.handleMount).to.have.been.calledTwice;
 		expect(inst.handleMount.firstCall).to.have.been.calledWith(null);
 		expect(inst.handleMount.secondCall).to.have.been.calledWith(scratch.querySelector('#div'));
@@ -306,13 +318,33 @@ describe.skip('refs', () => {
 
 		let ref = spy('ref');
 
-		class Wrapper {
+		class Wrapper extends Component {
 			render() {
 				return <div />;
 			}
 		}
 
-		render(<div><Wrapper ref={c => ref(c.base)} /></div>, scratch, scratch.firstChild);
+		render(<div><Wrapper ref={c => ref(c.base)} /></div>, scratch);
 		expect(ref).to.have.been.calledOnce.and.calledWith(scratch.firstChild.firstChild);
 	});
+
+	// Test for #1177
+	it('should call ref after children are rendered', done => {
+		let input;
+		function autoFocus(el) {
+			if (el) {
+				input = el;
+
+				// Chrome bug: It will somehow drop the focus event if it fires too soon.
+				// See https://stackoverflow.com/questions/17384464/
+				setTimeout(() => {
+					el.focus();
+					done();
+				}, 1);
+			}
+		}
+
+		render(<input type="text" ref={autoFocus} value="foo" />, scratch);
+		expect(input.value).to.equal('foo');
+	});
 });