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

Fix: make v5 Router compatible with v18 StrictMode #8831

Merged
merged 2 commits into from
May 9, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
18 changes: 13 additions & 5 deletions packages/react-router/modules/Router.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,26 @@ class Router extends React.Component {

if (!props.staticContext) {
this.unlisten = props.history.listen(location => {
if (this._isMounted) {
this.setState({ location });
} else {
this._pendingLocation = location;
}
this._pendingLocation = location;
});
}
}

componentDidMount() {
this._isMounted = true;

if (this.unlisten) {
// Any pre-mount location changes have been captured at
// this point, so unregister the listener.
this.unlisten();
}
if (!this.props.staticContext) {
this.unlisten = this.props.history.listen(location => {
if (this._isMounted) {
this.setState({ location });
}
});
}
if (this._pendingLocation) {
this.setState({ location: this._pendingLocation });
}
Expand Down
62 changes: 62 additions & 0 deletions packages/react-router/modules/__tests__/Router-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,66 @@ describe("A <Router>", () => {
}).not.toThrow();
});
});

describe("with react v18 StrictMode semantics", () => {
function simulateV18StrictModeRender(props, afterRender) {
// Constructor gets called twice
let router = new Router(props);
router = new Router(props);

// Swap out setState since we're not actually mounting the component
router.setState = jest.fn();

// Render is called twice
router.render();
router.render();

if (afterRender) afterRender();

router.componentDidMount();

// Effects are invoked twice
router.componentWillUnmount();
router.componentDidMount();

return router;
}

it("responds to history changes", () => {
const history = createHistory();
const props = { history, children: <p>Foo</p> };

const router = simulateV18StrictModeRender(props);

history.push("/foo");

expect(router.setState).toHaveBeenCalledTimes(1);
expect(router.setState).toHaveBeenCalledWith({
location: expect.objectContaining({ pathname: "/foo" })
});
});

it("can handle path changes on child mount", () => {
const history = createHistory();
const props = { history, children: <p>Foo</p> };

const router = simulateV18StrictModeRender(props, () => {
history.push("/baz");
history.push("/qux");
history.push("/foo");
});

expect(router.setState).toHaveBeenCalledTimes(1);
expect(router.setState).toHaveBeenCalledWith({
location: expect.objectContaining({ pathname: "/foo" })
});

history.push("/bar");

expect(router.setState).toHaveBeenCalledTimes(2);
expect(router.setState).toHaveBeenCalledWith({
location: expect.objectContaining({ pathname: "/bar" })
});
});
});
});