Merge branch 'master' into typed-children

README.md

@@ -105,6 +105,7 @@ Preact supports modern browsers and IE9+:
 - [**Preact Coffeescript**](https://github.com/crisward/preact-coffee)
 - [**Preact + TypeScript + Webpack**](https://github.com/k1r0s/bleeding-preact-starter)
 - [**0 config => Preact + Poi**](https://github.com/k1r0s/preact-poi-starter)
+- [**Zero configuration => Preact + Typescript + Parcel**](https://github.com/aalises/preact-typescript-parcel-starter)
 
 ---
 

src/component.js

@@ -34,6 +34,8 @@ export function Component(props, context) {
 	 * @type {object}
 	 */
 	this.state = this.state || {};
+
+	this._renderCallbacks = [];
 }
 
 
@@ -61,7 +63,7 @@ extend(Component.prototype, {
 		let s = this.state;
 		if (!this.prevState) this.prevState = extend({}, s);
 		extend(s, typeof state==='function' ? state(s, this.props) : state);
-		if (callback) (this._renderCallbacks = (this._renderCallbacks || [])).push(callback);
+		if (callback) this._renderCallbacks.push(callback);
 		enqueueRender(this);
 	},
 
@@ -73,7 +75,7 @@ extend(Component.prototype, {
 	 * @private
 	 */
 	forceUpdate(callback) {
-		if (callback) (this._renderCallbacks = (this._renderCallbacks || [])).push(callback);
+		if (callback) this._renderCallbacks.push(callback);
 		renderComponent(this, FORCE_RENDER);
 	},
 

src/dom/index.js

@@ -80,11 +80,18 @@ export function setAccessor(node, name, old, value, isSvg) {
 		(node._listeners || (node._listeners = {}))[name] = value;
 	}
 	else if (name!=='list' && name!=='type' && !isSvg && name in node) {
-		setProperty(node, name, value==null ? '' : value);
-		if (value==null || value===false) node.removeAttribute(name);
+		// Attempt to set a DOM property to the given value.
+		// IE & FF throw for certain property-value combinations.
+		try {
+			node[name] = value==null ? '' : value;
+		} catch (e) { }
+		if ((value==null || value===false) && name!='spellcheck') node.removeAttribute(name);
 	}
 	else {
 		let ns = isSvg && (name !== (name = name.replace(/^xlink:?/, '')));
+		// spellcheck is treated differently than all other boolean values and
+		// should not be removed when the value is `false`. See:
+		// https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input#attr-spellcheck
 		if (value==null || value===false) {
 			if (ns) node.removeAttributeNS('http://www.w3.org/1999/xlink', name.toLowerCase());
 			else node.removeAttribute(name);
@@ -97,17 +104,6 @@ export function setAccessor(node, name, old, value, isSvg) {
 }
 
 
-/**
- * Attempt to set a DOM property to the given value.
- * IE & FF throw for certain property-value combinations.
- */
-function setProperty(node, name, value) {
-	try {
-		node[name] = value;
-	} catch (e) { }
-}
-
-
 /**
  * Proxy an event to hooked event handlers
  * @private

src/preact.d.ts

@@ -694,7 +694,7 @@ declare global {
 			sizes?: string;
 			slot?: string;
 			span?: number;
-			spellCheck?: boolean;
+			spellcheck?: boolean;
 			src?: string;
 			srcset?: string;
 			srcDoc?: string;

src/vdom/component.js

@@ -75,10 +75,12 @@ export function renderComponent(component, opts, mountAll, isChild) {
 		initialBase = isUpdate || nextBase,
 		initialChildComponent = component._component,
 		skip = false,
+		snapshot = previousContext,
 		rendered, inst, cbase;
 
 	if (component.constructor.getDerivedStateFromProps) {
-		state = component.state = extend(state, component.constructor.getDerivedStateFromProps(props, state));
+		previousState = extend({}, previousState);
+		component.state = extend(state, component.constructor.getDerivedStateFromProps(props, state));
 	}
 
 	// if updating
@@ -110,6 +112,10 @@ export function renderComponent(component, opts, mountAll, isChild) {
 			context = extend(extend({}, context), component.getChildContext());
 		}
 
+		if (isUpdate && component.getSnapshotBeforeUpdate) {
+			snapshot = component.getSnapshotBeforeUpdate(previousProps, previousState);
+		}
+
 		let childComponent = rendered && rendered.nodeName,
 			toUnmount, base;
 
@@ -187,14 +193,12 @@ export function renderComponent(component, opts, mountAll, isChild) {
 		// flushMounts();
 
 		if (component.componentDidUpdate) {
-			component.componentDidUpdate(previousProps, previousState, previousContext);
+			component.componentDidUpdate(previousProps, previousState, snapshot);
 		}
 		if (options.afterUpdate) options.afterUpdate(component);
 	}
 
-	if (component._renderCallbacks!=null) {
-		while (component._renderCallbacks.length) component._renderCallbacks.pop().call(component);
-	}
+	while (component._renderCallbacks.length) component._renderCallbacks.pop().call(component);
 
 	if (!diffLevel && !isChild) flushMounts();
 }

test/browser/components.js

@@ -151,6 +151,12 @@ describe('Components', () => {
 		expect(scratch.innerHTML).to.equal('');
 	});
 
+	// Test for #651
+	it('should set enumerable boolean attribute', () => {
+		render(<input spellcheck={false} />, scratch);
+		expect(scratch.firstChild.spellcheck).to.equal(false);
+	});
+
 	// Test for Issue #73
 	it('should remove orphaned elements replaced by Components', () => {
 		class Comp extends Component {

test/browser/lifecycle.js

@@ -22,6 +22,83 @@ describe('Lifecycle methods', () => {
 		scratch = null;
 	});
 
+	it('should call nested new lifecycle methods in the right order', () => {
+		let log;
+		const logger = function(msg) {
+			return function() {
+				// return true for shouldComponentUpdate
+				log.push(msg);
+				return true;
+			};
+		};
+		class Outer extends Component {
+			static getDerivedStateFromProps() {
+				log.push('outer getDerivedStateFromProps');
+				return null;
+			}
+			render() {
+				return (
+					<div>
+						<Inner x={this.props.x} />
+					</div>
+				);
+			}
+		}
+
+		Object.assign(Outer.prototype, {
+			componentDidMount: logger('outer componentDidMount'),
+			shouldComponentUpdate: logger('outer shouldComponentUpdate'),
+			getSnapshotBeforeUpdate: logger('outer getSnapshotBeforeUpdate'),
+			componentDidUpdate: logger('outer componentDidUpdate'),
+			componentWillUnmount: logger('outer componentWillUnmount')
+		});
+
+		class Inner extends Component {
+			static getDerivedStateFromProps() {
+				log.push('inner getDerivedStateFromProps');
+				return null;
+			}
+			render() {
+				return <span>{this.props.x}</span>;
+			}
+		}
+		Object.assign(Inner.prototype, {
+			componentDidMount: logger('inner componentDidMount'),
+			shouldComponentUpdate: logger('inner shouldComponentUpdate'),
+			getSnapshotBeforeUpdate: logger('inner getSnapshotBeforeUpdate'),
+			componentDidUpdate: logger('inner componentDidUpdate'),
+			componentWillUnmount: logger('inner componentWillUnmount')
+		});
+
+		log = [];
+		render(<Outer x={1} />, scratch);
+		expect(log).to.deep.equal([
+			'outer getDerivedStateFromProps',
+			'inner getDerivedStateFromProps',
+			'inner componentDidMount',
+			'outer componentDidMount'
+		]);
+
+		// Dedup warnings
+		log = [];
+		render(<Outer x={2} />, scratch, scratch.firstChild);
+		// Note: we differ from react here in that we apply changes to the dom
+		// as we find them while diffing. React on the other hand separates this
+		// into specific phases, meaning changes to the dom are only flushed
+		// once the whole diff-phase is complete. This is why
+		// "outer getSnapshotBeforeUpdate" is called just before the "inner" hooks.
+		// For react this call would be right before "outer componentDidUpdate"
+		expect(log).to.deep.equal([
+			'outer getDerivedStateFromProps',
+			'outer shouldComponentUpdate',
+			'outer getSnapshotBeforeUpdate',
+			'inner getDerivedStateFromProps',
+			'inner shouldComponentUpdate',
+			'inner getSnapshotBeforeUpdate',
+			'inner componentDidUpdate',
+			'outer componentDidUpdate'
+		]);
+	});
 
 	describe('static getDerivedStateFromProps', () => {
 		it('should set initial state with value returned from getDerivedStateFromProps', () => {
@@ -192,8 +269,6 @@ describe('Lifecycle methods', () => {
 			expect(Foo.getDerivedStateFromProps).to.have.been.called;
 		});
 
-		// TODO: Consider if componentWillUpdate should still be called
-		// Likely, componentWillUpdate should not be called only if getSnapshotBeforeUpdate is implemented
 		it('should NOT invoke deprecated lifecycles (cWM/cWRP) if new static gDSFP is present', () => {
 			class Foo extends Component {
 				static getDerivedStateFromProps() {}
@@ -214,10 +289,150 @@ describe('Lifecycle methods', () => {
 			expect(Foo.prototype.componentWillReceiveProps).to.not.have.been.called;
 		});
 
+		it('is not called if neither state nor props have changed', () => {
+			let logs = [];
+			let childRef;
+
+			class Parent extends Component {
+				constructor(props) {
+					super(props);
+					this.state = { parentRenders: 0 };
+				}
+
+				static getDerivedStateFromProps(props, prevState) {
+					logs.push('parent getDerivedStateFromProps');
+					return prevState.parentRenders + 1;
+				}
+
+				render() {
+					logs.push('parent render');
+					return <Child parentRenders={this.state.parentRenders} ref={child => childRef = child} />;
+				}
+			}
+
+			class Child extends Component {
+				render() {
+					logs.push('child render');
+					return this.props.parentRenders;
+				}
+			}
+
+			render(<Parent />, scratch);
+			expect(logs).to.deep.equal([
+				'parent getDerivedStateFromProps',
+				'parent render',
+				'child render'
+			]);
+
+			logs = [];
+			childRef.setState({});
+			rerender();
+			expect(logs).to.deep.equal([
+				'child render'
+			]);
+		});
+
 		// TODO: Investigate this test:
 		// [should not override state with stale values if prevState is spread within getDerivedStateFromProps](https://github.com/facebook/react/blob/25dda90c1ecb0c662ab06e2c80c1ee31e0ae9d36/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js#L1035)
 	});
 
+	describe("#getSnapshotBeforeUpdate", () => {
+		it('should pass the return value from getSnapshotBeforeUpdate to componentDidUpdate', () => {
+			let log = [];
+
+			class MyComponent extends Component {
+				constructor(props) {
+					super(props);
+					this.state = {
+						value: 0
+					};
+				}
+				static getDerivedStateFromProps(nextProps, prevState) {
+					return {
+						value: prevState.value + 1
+					};
+				}
+				getSnapshotBeforeUpdate(prevProps, prevState) {
+					log.push(
+						`getSnapshotBeforeUpdate() prevProps:${prevProps.value} prevState:${
+							prevState.value
+						}`,
+					);
+					return 'abc';
+				}
+				componentDidUpdate(prevProps, prevState, snapshot) {
+					log.push(
+						`componentDidUpdate() prevProps:${prevProps.value} prevState:${
+							prevState.value
+						} snapshot:${snapshot}`,
+					);
+				}
+				render() {
+					log.push('render');
+					return null;
+				}
+			}
+
+			render(<MyComponent value="foo" />, scratch);
+			expect(log).to.deep.equal(['render']);
+			log = [];
+
+			render(<MyComponent value="bar" />, scratch, scratch.firstChild);
+			expect(log).to.deep.equal([
+				'render',
+				'getSnapshotBeforeUpdate() prevProps:foo prevState:1',
+				'componentDidUpdate() prevProps:foo prevState:1 snapshot:abc'
+			]);
+			log = [];
+
+			render(<MyComponent value="baz" />, scratch, scratch.firstChild);
+			expect(log).to.deep.equal([
+				'render',
+				'getSnapshotBeforeUpdate() prevProps:bar prevState:2',
+				'componentDidUpdate() prevProps:bar prevState:2 snapshot:abc'
+			]);
+			log = [];
+
+			render(<div />, scratch, scratch.firstChild);
+			expect(log).to.deep.equal([]);
+		});
+
+		it('should call getSnapshotBeforeUpdate before mutations are committed', () => {
+			let log = [];
+
+			class MyComponent extends Component {
+				getSnapshotBeforeUpdate(prevProps) {
+					log.push('getSnapshotBeforeUpdate');
+					expect(this.divRef.textContent).to.equal(
+						`value:${prevProps.value}`,
+					);
+					return 'foobar';
+				}
+				componentDidUpdate(prevProps, prevState, snapshot) {
+					log.push('componentDidUpdate');
+					expect(this.divRef.textContent).to.equal(
+						`value:${this.props.value}`,
+					);
+					expect(snapshot).to.equal('foobar');
+				}
+				render() {
+					log.push('render');
+					return <div ref={ref => this.divRef = ref}>{`value:${this.props.value}`}</div>;
+				}
+			}
+
+			render(<MyComponent value="foo" />, scratch);
+			expect(log).to.deep.equal(['render']);
+			log = [];
+
+			render(<MyComponent value="bar" />, scratch, scratch.firstChild);
+			expect(log).to.deep.equal([
+				'render',
+				'getSnapshotBeforeUpdate',
+				'componentDidUpdate'
+			]);
+		});
+	});
 
 	describe('#componentWillUpdate', () => {
 		it('should NOT be called on initial render', () => {