Skip to content

Commit

Permalink
Fix NoUpdate context (#2420)
Browse files Browse the repository at this point in the history
* fix context when under a NoUpdate barrier, this had to do with stale props inside shouldComponentUpdate

* revert change to way of updating
  • Loading branch information
JoviDeCroock committed Mar 17, 2020
1 parent 6599153 commit 50865d4
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 1 deletion.
44 changes: 44 additions & 0 deletions hooks/test/browser/useContext.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -304,4 +304,48 @@ describe('useContext', () => {
expect(scratch.innerHTML).to.equal('<div></div>');
expect(Inner).to.have.been.calledTwice;
});

it('should rerender when reset to defaultValue', () => {
const defaultValue = { state: 'hi' };
const context = createContext(defaultValue);
let set;

const Consumer = () => {
const ctx = useContext(context);
return <p>{ctx.state}</p>;
};

class NoUpdate extends Component {
shouldComponentUpdate() {
return false;
}

render() {
return <Consumer />;
}
}

const Provider = () => {
const [state, setState] = useState(defaultValue);
set = setState;
return (
<context.Provider value={state}>
<NoUpdate />
</context.Provider>
);
};

render(<Provider />, scratch);
expect(scratch.innerHTML).to.equal('<p>hi</p>');

act(() => {
set({ state: 'bye' });
});
expect(scratch.innerHTML).to.equal('<p>bye</p>');

act(() => {
set(defaultValue);
});
expect(scratch.innerHTML).to.equal('<p>hi</p>');
});
});
5 changes: 4 additions & 1 deletion src/create-context.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,16 @@ export function createContext(defaultValue) {
ctx[context._id] = this;
return ctx;
};

this.shouldComponentUpdate = _props => {
if (props.value !== _props.value) {
if (this.props.value !== _props.value) {
subs.some(c => {
c.context = _props.value;
enqueueRender(c);
});
}
};

this.sub = c => {
subs.push(c);
let old = c.componentWillUnmount;
Expand All @@ -35,6 +37,7 @@ export function createContext(defaultValue) {
};
};
}

return props.children;
}
};
Expand Down
43 changes: 43 additions & 0 deletions test/browser/createContext.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -848,4 +848,47 @@ describe('createContext', () => {
]);
});
});

it('should rerender when reset to defaultValue', () => {
const defaultValue = { state: 'hi' };
const context = createContext(defaultValue);
let set;

class NoUpdate extends Component {
shouldComponentUpdate() {
return false;
}

render() {
return <context.Consumer>{v => <p>{v.state}</p>}</context.Consumer>;
}
}

class Provider extends Component {
constructor(props) {
super(props);
this.state = defaultValue;
set = this.setState.bind(this);
}

render() {
return (
<context.Provider value={this.state}>
<NoUpdate />
</context.Provider>
);
}
}

render(<Provider />, scratch);
expect(scratch.innerHTML).to.equal('<p>hi</p>');

set({ state: 'bye' });
rerender();
expect(scratch.innerHTML).to.equal('<p>bye</p>');

set(defaultValue);
rerender();
expect(scratch.innerHTML).to.equal('<p>hi</p>');
});
});

0 comments on commit 50865d4

Please sign in to comment.