Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(refactor) move context update logic to diffing #1468

Merged
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
126 changes: 125 additions & 1 deletion hooks/test/browser/useContext.test.js
@@ -1,4 +1,4 @@
import { h, render, createContext } from 'preact';
import { h, render, createContext, Component } from 'preact';
import { setupScratch, teardown } from '../../../test/_util/helpers';
import { useContext } from '../../src';

Expand Down Expand Up @@ -36,4 +36,128 @@ 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();

function Comp() {
const foo = useContext(Foo);
const bar = useContext(Bar);
spy(foo, bar);

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;
});
});
13 changes: 4 additions & 9 deletions src/create-context.js
@@ -1,5 +1,3 @@
import { enqueueRender } from './component';

JoviDeCroock marked this conversation as resolved.
Show resolved Hide resolved
export let i = 0;

/**
Expand All @@ -23,20 +21,16 @@ export function createContext(defaultValue) {
let ctx = { [id]: null };

function initProvider(comp) {
let subs = [];
comp.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.sub = (c) => {
subs.push(c);
comp.subs.push(c);
let old = c.componentWillUnmount;
c.componentWillUnmount = () => {
subs.splice(subs.indexOf(c), 1);
comp.subs.splice(comp.subs.indexOf(c), 1);
old && old();
};
};
Expand All @@ -46,6 +40,7 @@ export function createContext(defaultValue) {
if (!this.getChildContext) initProvider(this);
return props.children;
}
Provider._id = '_P';
context.Provider = Provider;

return context;
Expand Down
6 changes: 5 additions & 1 deletion src/diff/index.js
Expand Up @@ -55,7 +55,6 @@ export function diff(dom, parentDom, newVNode, oldVNode, context, isSvg, excessD
let cxType = newType.contextType;
let provider = cxType && context[cxType._id];
let cctx = cxType != null ? (provider ? provider.props.value : cxType._defaultValue) : context;

JoviDeCroock marked this conversation as resolved.
Show resolved Hide resolved
// Get component and set it to `c`
if (oldVNode._component) {
c = newVNode._component = oldVNode._component;
Expand Down Expand Up @@ -110,6 +109,11 @@ export function diff(dom, parentDom, newVNode, oldVNode, context, isSvg, excessD
break outer;
}

if (newType._id==='_P' && c.props.value!==newVNode.props.value) {
const v = newVNode.props.value;
newVNode._component.subs.map(sub => v!==sub.context && (sub.context = v, enqueueRender(sub)));
JoviDeCroock marked this conversation as resolved.
Show resolved Hide resolved
}

if (c.componentWillUpdate!=null) {
c.componentWillUpdate(newVNode.props, s, cctx);
}
Expand Down
14 changes: 12 additions & 2 deletions test/browser/createContext.test.js
Expand Up @@ -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' };
Expand All @@ -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', () => {
Expand Down Expand Up @@ -181,7 +186,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' };
Expand Down Expand Up @@ -240,6 +245,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} />
Expand All @@ -251,6 +257,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"', () => {
Expand Down