Skip to content

Reduce number of StrictMode warnings #6287

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

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/react-router-dom/modules/BrowserRouter.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class BrowserRouter extends React.Component {

history = createHistory(this.props);

componentWillMount() {
componentDidMount() {
warning(
!this.props.history,
"<BrowserRouter> ignores the history prop. To use a custom history, " +
Expand Down
2 changes: 1 addition & 1 deletion packages/react-router-dom/modules/HashRouter.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class HashRouter extends React.Component {

history = createHistory(this.props);

componentWillMount() {
componentDidMount() {
warning(
!this.props.history,
"<HashRouter> ignores the history prop. To use a custom history, " +
Expand Down
15 changes: 15 additions & 0 deletions packages/react-router-dom/modules/__tests__/BrowserRouter-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,21 @@ describe("A <BrowserRouter>", () => {
expect(typeof history).toBe("object");
});

it.skip("does not error in StrictMode", () => {
const node = document.createElement("div");

spyOn(console, "error");

ReactDOM.render(
<React.StrictMode>
<BrowserRouter />
</React.StrictMode>,
node
);

expect(console.error).toHaveBeenCalledTimes(0);
});

it("warns when passed a history prop", () => {
const history = {};
const node = document.createElement("div");
Expand Down
15 changes: 15 additions & 0 deletions packages/react-router-dom/modules/__tests__/HashRouter-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,21 @@ describe("A <HashRouter>", () => {
expect(typeof history).toBe("object");
});

it.skip("does not error in StrictMode", () => {
const node = document.createElement("div");

spyOn(console, "error");

ReactDOM.render(
<React.StrictMode>
<HashRouter />
</React.StrictMode>,
node
);

expect(console.error).toHaveBeenCalledTimes(0);
});

it("warns when passed a history prop", () => {
const history = {};
const node = document.createElement("div");
Expand Down
2 changes: 1 addition & 1 deletion packages/react-router/modules/MemoryRouter.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class MemoryRouter extends React.Component {

history = createHistory(this.props);

componentWillMount() {
componentDidMount() {
warning(
!this.props.history,
"<MemoryRouter> ignores the history prop. To use a custom history, " +
Expand Down
10 changes: 5 additions & 5 deletions packages/react-router/modules/Prompt.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class Prompt extends React.Component {
}
}

componentWillMount() {
componentDidMount() {
invariant(
this.context.router,
"You should not use <Prompt> outside a <Router>"
Expand All @@ -46,10 +46,10 @@ class Prompt extends React.Component {
if (this.props.when) this.enable(this.props.message);
}

componentWillReceiveProps(nextProps) {
if (nextProps.when) {
if (!this.props.when || this.props.message !== nextProps.message)
this.enable(nextProps.message);
componentDidUpdate(prevProps) {
if (this.props.when) {
if (!prevProps.when || prevProps.message !== this.props.message)
this.enable(this.props.message);
} else {
this.disable();
}
Expand Down
4 changes: 2 additions & 2 deletions packages/react-router/modules/Router.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@ class Router extends React.Component {
});
}

componentWillReceiveProps(nextProps) {
componentDidUpdate(prevProps) {
warning(
this.props.history === nextProps.history,
prevProps.history === this.props.history,
"You cannot change <Router history>"
);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/react-router/modules/StaticRouter.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ class StaticRouter extends React.Component {

handleBlock = () => noop;

componentWillMount() {
componentDidMount() {
warning(
!this.props.history,
"<StaticRouter> ignores the history prop. To use a custom history, " +
Expand Down
6 changes: 3 additions & 3 deletions packages/react-router/modules/Switch.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,14 @@ class Switch extends React.Component {
);
}

componentWillReceiveProps(nextProps) {
componentDidUpdate(prevProps) {
warning(
!(nextProps.location && !this.props.location),
!(this.props.location && !prevProps.location),
'<Switch> elements should not change from uncontrolled to controlled (or vice versa). You initially used no "location" prop and then provided one on a subsequent render.'
);

warning(
!(!nextProps.location && this.props.location),
!(!this.props.location && prevProps.location),
'<Switch> elements should not change from controlled to uncontrolled (or vice versa). You provided a "location" prop initially but omitted it on a subsequent render.'
);
}
Expand Down
15 changes: 15 additions & 0 deletions packages/react-router/modules/__tests__/MemoryRouter-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,19 @@ describe("A <MemoryRouter>", () => {
expect.stringContaining("<MemoryRouter> ignores the history prop")
);
});

it.skip("does not error in StrictMode", () => {
const node = document.createElement("div");

spyOn(console, "error");

ReactDOM.render(
<React.StrictMode>
<MemoryRouter />
</React.StrictMode>,
node
);

expect(console.error).toHaveBeenCalledTimes(0);
});
});
70 changes: 70 additions & 0 deletions packages/react-router/modules/__tests__/Prompt-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import React from "react";
import ReactDOM from "react-dom";
import MemoryRouter from "../MemoryRouter";
import Prompt from "../Prompt";

describe("A <Prompt>", () => {
it("is enabled when 'when' is undefined", () => {
const node = document.createElement("div");

spyOn(Prompt.prototype, "enable");

ReactDOM.render(
<MemoryRouter initialEntries={["/one"]}>
<Prompt message="hi" />
</MemoryRouter>,
node
);

expect(Prompt.prototype.enable).toHaveBeenCalledTimes(1);
expect(Prompt.prototype.enable).toHaveBeenCalledWith("hi");
});

it("is enabled when 'when' is true", () => {
const node = document.createElement("div");

spyOn(Prompt.prototype, "enable");

ReactDOM.render(
<MemoryRouter initialEntries={["/one"]}>
<Prompt when message="hi" />
</MemoryRouter>,
node
);

expect(Prompt.prototype.enable).toHaveBeenCalledTimes(1);
expect(Prompt.prototype.enable).toHaveBeenCalledWith("hi");
});

it("is not enabled when 'when' is false", () => {
const node = document.createElement("div");

spyOn(Prompt.prototype, "enable");

ReactDOM.render(
<MemoryRouter initialEntries={["/one"]}>
<Prompt when={false} message="hi" />
</MemoryRouter>,
node
);

expect(Prompt.prototype.enable).toHaveBeenCalledTimes(0);
});

it("does not error in StrictMode", () => {
const node = document.createElement("div");

spyOn(console, "error");

ReactDOM.render(
<MemoryRouter initialEntries={["/one"]}>
<React.StrictMode>
<Prompt when={false} message="hi" />
</React.StrictMode>
</MemoryRouter>,
node
);

expect(console.error).toHaveBeenCalledTimes(0);
});
});
23 changes: 23 additions & 0 deletions packages/react-router/modules/__tests__/Redirect-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,28 @@ describe("A <Redirect>", () => {
messageId: "123"
});
});

it.skip("does not error in StrictMode", () => {
const node = document.createElement("div");

spyOn(console, "error");

ReactDOM.render(
<MemoryRouter initialEntries={["/one"]}>
<Switch>
<React.StrictMode>
<Redirect
from="/users/:username/messages/:messageId"
to="/:username/messages/:messageId"
/>
</React.StrictMode>
<Route path="/:username/messages/:messageId" render={() => null} />
</Switch>
</MemoryRouter>,
node
);

expect(console.error).toHaveBeenCalledTimes(0);
});
});
});
18 changes: 18 additions & 0 deletions packages/react-router/modules/__tests__/Route-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,24 @@ describe("A <Route>", () => {
expect(node.innerHTML).toContain(TEXT);
});

it.skip("does not error in StrictMode", () => {
const TEXT = "Mrs. Kato";
const node = document.createElement("div");

spyOn(console, "error");

ReactDOM.render(
<MemoryRouter initialEntries={["/"]}>
<React.StrictMode>
<Route path="/" render={() => <h1>{TEXT}</h1>} />
</React.StrictMode>
</MemoryRouter>,
node
);

expect(console.error).toHaveBeenCalledTimes(0);
});

it("does not render when it does not match", () => {
const TEXT = "bubblegum";
const node = document.createElement("div");
Expand Down
15 changes: 15 additions & 0 deletions packages/react-router/modules/__tests__/Router-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,21 @@ describe("A <Router>", () => {
);
}).not.toThrow();
});

it.skip("does not error in StrictMode", () => {
spyOn(console, "error");

ReactDOM.render(
<React.StrictMode>
<Router history={createHistory()}>
<p>Bar</p>
</Router>
</React.StrictMode>,
node
);

expect(console.error).toHaveBeenCalledTimes(0);
});
});

describe("with no children", () => {
Expand Down
25 changes: 25 additions & 0 deletions packages/react-router/modules/__tests__/StaticRouter-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,31 @@ describe("A <StaticRouter>", () => {
);
});

it.skip("does not error in StrictMode", () => {
const ContextChecker = (props, reactContext) => {
expect(typeof reactContext.router.history).toBe("object");
return null;
};

ContextChecker.contextTypes = {
router: PropTypes.object.isRequired
};

const context = {};

spyOn(console, "error");

ReactDOMServer.renderToStaticMarkup(
<React.StrictMode>
<StaticRouter context={context}>
<ContextChecker />
</StaticRouter>
</React.StrictMode>
);

expect(console.error).toHaveBeenCalledTimes(0);
});

it("warns when passed a history prop", () => {
const context = {};
const history = {};
Expand Down
20 changes: 20 additions & 0 deletions packages/react-router/modules/__tests__/Switch-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,26 @@ describe("A <Switch>", () => {
expect(node.innerHTML).toMatch(/one/);
});

it.skip("does not error in StrictMode", () => {
const node = document.createElement("div");

spyOn(console, "error");

ReactDOM.render(
<MemoryRouter initialEntries={["/one"]}>
<React.StrictMode>
<Switch>
<Route path="/one" render={() => <h1>one</h1>} />
<Route path="/two" render={() => <h1>two</h1>} />
</Switch>
</React.StrictMode>
</MemoryRouter>,
node
);

expect(console.error).toHaveBeenCalledTimes(0);
});

it("renders the first <Redirect from> that matches the URL", () => {
const node = document.createElement("div");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ describe("A <Switch>", () => {
let mountCount = 0;

class App extends React.Component {
componentWillMount() {
componentDidMount() {
mountCount++;
}

Expand Down
22 changes: 22 additions & 0 deletions packages/react-router/modules/__tests__/withRouter-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,28 @@ describe("withRouter", () => {
);
});

it.skip("does not error in StrictMode", () => {
const WrappedComponent = withRouter(() => null);

spyOn(console, "error");

ReactDOM.render(
<MemoryRouter initialEntries={["/bubblegum"]}>
<Route
path="/bubblegum"
render={() => (
<React.StrictMode>
<WrappedComponent />
</React.StrictMode>
)}
/>
</MemoryRouter>,
node
);

expect(console.error).toHaveBeenCalledTimes(0);
});

it("provides the parent match as a prop to the wrapped component", () => {
let parentMatch;
const PropsChecker = withRouter(props => {
Expand Down