Merge 6597287 into 76c4918

preactjs · Mar 30, 2019 · 6075eb8 · 6075eb8
2 parents 76c4918 + 6597287
commit 6075eb8
Show file tree

Hide file tree

Showing 3 changed files with 203 additions and 8 deletions.
diff --git a/hooks/test/browser/useContext.test.js b/hooks/test/browser/useContext.test.js
@@ -1,6 +1,6 @@
-import { h, render, createContext } from 'preact';
+import { h, render, createContext, Component } from 'preact';
 import { setupScratch, teardown } from '../../../test/_util/helpers';
-import { useContext } from '../../src';
+import { useContext, useEffect } from '../../src';
 
 /** @jsx h */
 
@@ -36,4 +36,135 @@ describe('useContext', () => {
 		expect(values).to.deep.equal([13, 42, 69]);
 	});
 
+	it('should use default value', () => {
+		const Foo = createContext(42);
+		const spy = sinon.spy();
+
+ 		function App() {
+			spy(useContext(Foo));
+			return <div />;
+		}
+
+ 		render(<App />, scratch);
+		expect(spy).to.be.calledWith(42);
+	});
+
+	it('should update when value changes with nonUpdating Component on top', done => {
+		const spy = sinon.spy();
+		const Ctx = createContext(0);
+
+		class NoUpdate extends Component {
+			shouldComponentUpdate() {
+				return false;
+			}
+			render() {
+				return this.props.children;
+			}
+		}
+
+ 		function App(props) {
+			return (
+				<Ctx.Provider value={props.value}>
+					<NoUpdate>
+						<Comp />
+					</NoUpdate>
+				</Ctx.Provider>
+			);
+		}
+
+ 		function Comp() {
+			const value = useContext(Ctx);
+			spy(value);
+			return <h1>{value}</h1>;
+		}
+
+ 		render(<App value={0} />, scratch);
+		expect(spy).to.be.calledOnce;
+		expect(spy).to.be.calledWith(0);
+		render(<App value={1} />, scratch);
+
+ 		// Wait for enqueued hook update
+		setTimeout(() => {
+			// Should not be called a third time
+			expect(spy).to.be.calledTwice;
+			expect(spy).to.be.calledWith(1);
+			done();
+		}, 0);
+	});
+
+	it('should only update when value has changed', done => {
+		const spy = sinon.spy();
+		const Ctx = createContext(0);
+
+ 		function App(props) {
+			return (
+				<Ctx.Provider value={props.value}>
+					<Comp />
+				</Ctx.Provider>
+			);
+		}
+
+ 		function Comp() {
+			const value = useContext(Ctx);
+			spy(value);
+			return <h1>{value}</h1>;
+		}
+
+ 		render(<App value={0} />, scratch);
+		expect(spy).to.be.calledOnce;
+		expect(spy).to.be.calledWith(0);
+		render(<App value={1} />, scratch);
+
+ 		expect(spy).to.be.calledTwice;
+		expect(spy).to.be.calledWith(1);
+
+ 		// Wait for enqueued hook update
+		setTimeout(() => {
+			// Should not be called a third time
+			expect(spy).to.be.calledTwice;
+			done();
+		}, 0);
+	});
+
+ 	it('should allow multiple context hooks at the same time', () => {
+		const Foo = createContext(0);
+		const Bar = createContext(10);
+		const spy = sinon.spy();
+		const unmountspy = sinon.spy();
+
+ 		function Comp() {
+			const foo = useContext(Foo);
+			const bar = useContext(Bar);
+			spy(foo, bar);
+			useEffect(() => {
+				() => {
+					unmountspy();
+				}
+			})
+
+ 			return <div />;
+		}
+
+ 		render((
+			<Foo.Provider value={0}>
+				<Bar.Provider value={10}>
+					<Comp />
+				</Bar.Provider>
+			</Foo.Provider>
+		), scratch);
+
+ 		expect(spy).to.be.calledOnce;
+		expect(spy).to.be.calledWith(0, 10);
+
+ 		render((
+			<Foo.Provider value={11}>
+				<Bar.Provider value={42}>
+					<Comp />
+				</Bar.Provider>
+			</Foo.Provider>
+		), scratch);
+
+		 expect(spy).to.be.calledTwice;
+		 expect(unmountspy).not.to.be.called;
+	});
 });
diff --git a/src/create-context.js b/src/create-context.js
@@ -23,14 +23,19 @@ export function createContext(defaultValue) {
 	let ctx = { [id]: null };
 
 	function initProvider(comp) {
-		let subs = [];
+		const subs = [];
 		comp.getChildContext = () => {
 			ctx[id] = comp;
 			return ctx;
 		};
-		comp.componentDidUpdate = () => {
-			let v = comp.props.value;
-			subs.map(c => v!==c.context && (c.context = v, enqueueRender(c)));
+		comp.shouldComponentUpdate = props => {
+			subs.map(c => {
+				// Check if still mounted
+				if (c._parentDom) {
+					c.context = props.value;
+					enqueueRender(c);
+				}
+			});
 		};
 		comp.sub = (c) => {
 			subs.push(c);

diff --git a/test/browser/createContext.test.js b/test/browser/createContext.test.js
@@ -44,7 +44,7 @@ describe('createContext', () => {
 		expect(scratch.innerHTML).to.equal('<div><div>a</div></div>');
 	});
 
-	it('should preserve provider context through nesting providers', () => {
+	it('should preserve provider context through nesting providers', (done) => {
 		const { Provider, Consumer } = createContext();
 		const CONTEXT = { a: 'a' };
 		const CHILD_CONTEXT = { b: 'b' };
@@ -71,7 +71,12 @@ describe('createContext', () => {
 
 		// initial render does not invoke anything but render():
 		expect(Inner.prototype.render).to.have.been.calledWithMatch({ ...CONTEXT, ...CHILD_CONTEXT }, {}, { ['__cC' + (ctxId - 1)]: {} });
+		expect(Inner.prototype.render).to.be.calledOnce;
 		expect(scratch.innerHTML).to.equal('<div>a - b</div>');
+		setTimeout(() => {
+			expect(Inner.prototype.render).to.be.calledOnce;
+			done();
+		}, 0);
 	});
 
 	it('should preserve provider context between different providers', () => {
@@ -130,6 +135,55 @@ describe('createContext', () => {
 		expect(scratch.innerHTML).to.equal('<div>a</div>');
 	});
 
+	it('should not emit when value does not update', () => {
+		const { Provider, Consumer } = createContext();
+		const CONTEXT = { a: 'a' };
+
+		class NoUpdate extends Component {
+			shouldComponentUpdate() {
+				return false;
+			}
+
+			render() {
+				return this.props.children;
+			}
+		}
+
+		class Inner extends Component {
+			render(props) {
+				return <div>{props.a}</div>;
+			}
+		}
+
+		sinon.spy(Inner.prototype, 'render');
+
+		render(
+			<div>
+				<Provider value={CONTEXT}>
+					<NoUpdate>
+						<Consumer>
+							{data => <Inner {...data} />}
+						</Consumer>
+					</NoUpdate>
+				</Provider>
+			</div>, scratch);
+
+		expect(Inner.prototype.render).to.have.been.calledOnce;
+
+		render(
+			<div>
+				<Provider value={CONTEXT}>
+					<NoUpdate>
+						<Consumer>
+							{data => <Inner {...data} />}
+						</Consumer>
+					</NoUpdate>
+				</Provider>
+			</div>, scratch);
+
+		expect(Inner.prototype.render).to.have.been.calledOnce;
+	});
+
 	it('should preserve provider context through nested components', () => {
 		const { Provider, Consumer } = createContext();
 		const CONTEXT = { a: 'a' };
@@ -181,7 +235,7 @@ describe('createContext', () => {
 		expect(scratch.innerHTML).to.equal('<div><div><strong>a</strong></div></div>');
 	});
 
-	it('should propagates through shouldComponentUpdate false', () => {
+	it('should propagates through shouldComponentUpdate false', done => {
 		const { Provider, Consumer } = createContext();
 		const CONTEXT = { a: 'a' };
 		const UPDATED_CONTEXT = { a: 'b' };
@@ -240,6 +294,7 @@ describe('createContext', () => {
 			<App value={CONTEXT} />
 		), scratch);
 		expect(scratch.innerHTML).to.equal('<div><div><strong>a</strong></div></div>');
+		expect(Consumed.prototype.render).to.have.been.calledOnce;
 
 		render((
 			<App value={UPDATED_CONTEXT} />
@@ -251,6 +306,10 @@ describe('createContext', () => {
 		expect(Consumed.prototype.render).to.have.been.calledTwice;
 		// expect(Consumed.prototype.render).to.have.been.calledWithMatch({ ...UPDATED_CONTEXT }, {}, { ['__cC' + (ctxId - 1)]: {} });
 		expect(scratch.innerHTML).to.equal('<div><div><strong>b</strong></div></div>');
+		setTimeout(() => {
+			expect(Consumed.prototype.render).to.have.been.calledTwice;
+			done();
+		});
 	});
 
 	it('should keep the right context at the right "depth"', () => {