From e9adfd8fa829c8cd39343068c652a14fce5ef705 Mon Sep 17 00:00:00 2001 From: Veniamin Krol <153412+vkrol@users.noreply.github.com> Date: Fri, 2 Aug 2019 22:21:42 +0300 Subject: [PATCH 1/4] Move Provider tests from inject.test.js to Provider.test.js --- test/Provider.test.js | 128 +++++++++++++++++- ...ect.test.js.snap => Provider.test.js.snap} | 2 +- test/inject.test.js | 122 ----------------- 3 files changed, 127 insertions(+), 125 deletions(-) rename test/__snapshots__/{inject.test.js.snap => Provider.test.js.snap} (84%) diff --git a/test/Provider.test.js b/test/Provider.test.js index 4de20bd9..8eeea366 100644 --- a/test/Provider.test.js +++ b/test/Provider.test.js @@ -1,6 +1,8 @@ -import React from "react" -import { Provider } from "../src" +import React, { Component } from "react" +import * as mobx from "mobx" +import { observer, inject, Provider } from "../src" import { render } from "@testing-library/react" +import renderer, { act } from "react-test-renderer" import { MobXProviderContext } from "../src/Provider" describe("Provider", () => { @@ -22,4 +24,126 @@ describe("Provider", () => { const { container } = render() expect(container).toHaveTextContent("children was not provided") }) + + it("supports overriding stores", () => { + const C = inject("foo", "bar")( + observer( + class C extends Component { + render() { + return ( +
+ context: + {this.props.foo} + {this.props.bar} +
+ ) + } + } + ) + ) + const B = () => + const A = class A extends Component { + render() { + return ( + +
+ + + +
+ + + +
+
+ + ) + } + } + const wrapper = renderer.create() + expect(wrapper).toMatchInlineSnapshot(` +
+ +
+ context: + bar + 1337 +
+ +
+
+ context: + 42 + 1337 +
+
+
+`) + }) + + it("should throw an error when changing stores", () => { + let msgs = [] + const baseError = console.error + console.error = m => msgs.push(m.split("\n").slice(0, 7)) // drop stacktraces to avoid line number issues + const a = mobx.observable.box(3) + const C = inject("foo")( + observer( + class C extends Component { + render() { + return ( +
+ context: + {this.props.foo} +
+ ) + } + } + ) + ) + const B = observer( + class B extends Component { + render() { + return + } + } + ) + const A = observer( + class A extends Component { + render() { + return ( +
+ {a.get()} + + + +
+ ) + } + } + ) + const wrapper = renderer.create() + + expect(wrapper).toMatchInlineSnapshot(` +
+ + 3 + +
+ context: + 3 +
+
+`) + + expect(() => { + act(() => { + a.set(42) + }) + }).toThrow( + "The set of provided stores has changed. Please avoid changing stores as the change might not propagate to all children" + ) + expect(msgs).toMatchSnapshot() // nobody caught the error of the rendering + + console.error = baseError + }) }) diff --git a/test/__snapshots__/inject.test.js.snap b/test/__snapshots__/Provider.test.js.snap similarity index 84% rename from test/__snapshots__/inject.test.js.snap rename to test/__snapshots__/Provider.test.js.snap index 37099253..89df212b 100644 --- a/test/__snapshots__/inject.test.js.snap +++ b/test/__snapshots__/Provider.test.js.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`inject based context warning is printed when changing stores 2`] = ` +exports[`Provider should throw an error when changing stores 2`] = ` Array [ Array [ "The above error occurred in the component:", diff --git a/test/inject.test.js b/test/inject.test.js index 0c715586..29be27e4 100644 --- a/test/inject.test.js +++ b/test/inject.test.js @@ -127,62 +127,6 @@ describe("inject based context", () => { expect(wrapperC.root.children[0].type.displayName).toBe("inject(ComponentC)") }) - test("overriding stores is supported", () => { - const C = inject("foo", "bar")( - observer( - class C extends Component { - render() { - return ( -
- context: - {this.props.foo} - {this.props.bar} -
- ) - } - } - ) - ) - const B = () => - const A = class A extends Component { - render() { - return ( - -
- - - -
- - - -
-
- - ) - } - } - const wrapper = renderer.create() - expect(wrapper).toMatchInlineSnapshot(` -
- -
- context: - bar - 1337 -
- -
-
- context: - 42 - 1337 -
-
-
-`) - }) - // FIXME: see other comments related to error catching in React // test does work as expected when running manually test("store should be available", () => { @@ -257,72 +201,6 @@ describe("inject based context", () => { renderer.create() }) - test("warning is printed when changing stores", () => { - let msgs = [] - const baseError = console.error - console.error = m => msgs.push(m.split("\n").slice(0, 7)) // drop stacktraces to avoid line number issues - const a = mobx.observable.box(3) - const C = inject("foo")( - observer( - class C extends Component { - render() { - return ( -
- context: - {this.props.foo} -
- ) - } - } - ) - ) - const B = observer( - class B extends Component { - render() { - return - } - } - ) - const A = observer( - class A extends Component { - render() { - return ( -
- {a.get()} - - - -
- ) - } - } - ) - const wrapper = renderer.create() - - expect(wrapper).toMatchInlineSnapshot(` -
- - 3 - -
- context: - 3 -
-
-`) - - expect(() => { - act(() => { - a.set(42) - }) - }).toThrow( - "The set of provided stores has changed. Please avoid changing stores as the change might not propagate to all children" - ) - expect(msgs).toMatchSnapshot() // nobody caught the error of the rendering - - console.error = baseError - }) - test("custom storesToProps", () => { const C = inject((stores, props) => { expect(stores).toEqual({ foo: "bar" }) From 9dc599e75ea1a89083e37122f2d9f2aeed97e06c Mon Sep 17 00:00:00 2001 From: Veniamin Krol <153412+vkrol@users.noreply.github.com> Date: Fri, 2 Aug 2019 22:21:48 +0300 Subject: [PATCH 2/4] Simplify Provider tests that was moved from inject.test.js --- test/Provider.test.js | 139 ++++++----------------- test/__snapshots__/Provider.test.js.snap | 15 --- 2 files changed, 34 insertions(+), 120 deletions(-) delete mode 100644 test/__snapshots__/Provider.test.js.snap diff --git a/test/Provider.test.js b/test/Provider.test.js index 8eeea366..93779508 100644 --- a/test/Provider.test.js +++ b/test/Provider.test.js @@ -1,8 +1,6 @@ -import React, { Component } from "react" -import * as mobx from "mobx" -import { observer, inject, Provider } from "../src" +import React from "react" +import { inject, Provider } from "../src" import { render } from "@testing-library/react" -import renderer, { act } from "react-test-renderer" import { MobXProviderContext } from "../src/Provider" describe("Provider", () => { @@ -26,123 +24,54 @@ describe("Provider", () => { }) it("supports overriding stores", () => { - const C = inject("foo", "bar")( - observer( - class C extends Component { - render() { - return ( -
- context: - {this.props.foo} - {this.props.bar} -
- ) - } - } - ) - ) - const B = () => - const A = class A extends Component { - render() { - return ( - -
- - - -
- - - -
-
+ const B = inject("overridable", "nonOverridable")(function({ + overridable, + nonOverridable + }) { + return `${overridable} ${nonOverridable}` + }) + + function A() { + return ( + + + + - ) - } + + ) } - const wrapper = renderer.create() - expect(wrapper).toMatchInlineSnapshot(` + const { container } = render() + expect(container).toMatchInlineSnapshot(`
- -
- context: - bar - 1337 -
- -
-
- context: - 42 - 1337 -
-
+ original original + overriden original
`) }) it("should throw an error when changing stores", () => { - let msgs = [] const baseError = console.error - console.error = m => msgs.push(m.split("\n").slice(0, 7)) // drop stacktraces to avoid line number issues - const a = mobx.observable.box(3) - const C = inject("foo")( - observer( - class C extends Component { - render() { - return ( -
- context: - {this.props.foo} -
- ) - } - } + console.error = () => {} + const B = inject("foo")(function C({ foo }) { + return foo + }) + + function A({ foo }) { + return ( + + + ) - ) - const B = observer( - class B extends Component { - render() { - return - } - } - ) - const A = observer( - class A extends Component { - render() { - return ( -
- {a.get()} - - - -
- ) - } - } - ) - const wrapper = renderer.create() + } - expect(wrapper).toMatchInlineSnapshot(` -
- - 3 - -
- context: - 3 -
-
-`) + const { rerender } = render() expect(() => { - act(() => { - a.set(42) - }) + rerender() }).toThrow( "The set of provided stores has changed. Please avoid changing stores as the change might not propagate to all children" ) - expect(msgs).toMatchSnapshot() // nobody caught the error of the rendering console.error = baseError }) diff --git a/test/__snapshots__/Provider.test.js.snap b/test/__snapshots__/Provider.test.js.snap deleted file mode 100644 index 89df212b..00000000 --- a/test/__snapshots__/Provider.test.js.snap +++ /dev/null @@ -1,15 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`Provider should throw an error when changing stores 2`] = ` -Array [ - Array [ - "The above error occurred in the component:", - " in MobXProvider (created by A)", - " in section (created by A)", - " in A", - "", - "Consider adding an error boundary to your tree to customize error handling behavior.", - "Visit https://fb.me/react-error-boundaries to learn more about error boundaries.", - ], -] -`; From e58fd73c97cad195340b9a669e3df0f0f5021bb8 Mon Sep 17 00:00:00 2001 From: Veniamin Krol <153412+vkrol@users.noreply.github.com> Date: Fri, 2 Aug 2019 22:21:53 +0300 Subject: [PATCH 3/4] Do not use inject in Provider tests (we test Provider only, not Provider + inject) --- test/Provider.test.js | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/test/Provider.test.js b/test/Provider.test.js index 93779508..dc2cd9b6 100644 --- a/test/Provider.test.js +++ b/test/Provider.test.js @@ -1,5 +1,5 @@ import React from "react" -import { inject, Provider } from "../src" +import { Provider } from "../src" import { render } from "@testing-library/react" import { MobXProviderContext } from "../src/Provider" @@ -24,12 +24,13 @@ describe("Provider", () => { }) it("supports overriding stores", () => { - const B = inject("overridable", "nonOverridable")(function({ - overridable, - nonOverridable - }) { - return `${overridable} ${nonOverridable}` - }) + function B() { + return ( + + {({ overridable, nonOverridable }) => `${overridable} ${nonOverridable}`} + + ) + } function A() { return ( @@ -53,14 +54,11 @@ describe("Provider", () => { it("should throw an error when changing stores", () => { const baseError = console.error console.error = () => {} - const B = inject("foo")(function C({ foo }) { - return foo - }) function A({ foo }) { return ( - + {({ foo }) => foo} ) } From ee424a9e6ef00d73a4e9e2aefb77c9f4ac8fb7fe Mon Sep 17 00:00:00 2001 From: Veniamin Krol <153412+vkrol@users.noreply.github.com> Date: Fri, 2 Aug 2019 22:35:18 +0300 Subject: [PATCH 4/4] Add tests for the Provider component --- test/Provider.test.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/Provider.test.js b/test/Provider.test.js index dc2cd9b6..b9d2baa1 100644 --- a/test/Provider.test.js +++ b/test/Provider.test.js @@ -4,6 +4,19 @@ import { render } from "@testing-library/react" import { MobXProviderContext } from "../src/Provider" describe("Provider", () => { + it("should work in a simple case", () => { + function A() { + return ( + + {({ foo }) => foo} + + ) + } + + const { container } = render() + expect(container).toHaveTextContent("bar") + }) + it("should not provide the children prop", () => { function A() { return (