From 4e4f84302a981defa85e4afa7d1dc2b0895c2495 Mon Sep 17 00:00:00 2001 From: carlesba Date: Mon, 15 Oct 2018 21:07:11 +0200 Subject: [PATCH 1/3] Refetch when resolve prop changes --- src/Get.test.tsx | 133 ++++++++++++++++++++++++++++++++++++++++++++++- src/Get.tsx | 5 +- 2 files changed, 133 insertions(+), 5 deletions(-) diff --git a/src/Get.test.tsx b/src/Get.test.tsx index 0c9701c1..d63fa05b 100644 --- a/src/Get.test.tsx +++ b/src/Get.test.tsx @@ -293,6 +293,11 @@ describe("Get", () => { describe("with wait", () => { it("should render nothing if until we have data", async () => { + nock("https://my-awesome-api.fake") + .get("/") + .delay(1000) + .reply(200, { hello: "world" }); + const children = jest.fn(); children.mockReturnValue(
); @@ -521,16 +526,21 @@ describe("Get", () => { const children = jest.fn(); children.mockReturnValue(
); + const resolve = a => a; const { rerender } = render( - {children} + + {children} + , ); times(10, i => rerender( - {children} + + {children} + , ), ); @@ -538,4 +548,123 @@ describe("Get", () => { expect(apiCalls).toEqual(10); }); }); + describe("refetch after update", () => { + it("should not refetch when base, path or resolve don't change", () => { + let apiCalls = 0; + nock("https://my-awesome-api.fake") + .get("/") + .reply(200, () => ++apiCalls) + .persist(); + + const children = jest.fn(); + children.mockReturnValue(
); + + const resolve = a => a; + const { rerender } = render( + + + {children} + + , + ); + + rerender( + + + {children} + + , + ); + + expect(apiCalls).toEqual(1); + }); + it("should refetch when base changes", () => { + let apiCalls = 0; + nock("https://my-awesome-api.fake") + .get("/") + .reply(200, () => ++apiCalls); + + const children = jest.fn(); + children.mockReturnValue(
); + + const resolve = a => a; + const { rerender } = render( + + + {children} + + , + ); + + nock("https://my-new-api.fake") + .get("/") + .reply(200, () => ++apiCalls); + rerender( + + + {children} + + , + ); + + expect(apiCalls).toEqual(2); + }); + it("should refetch when path changes", () => { + let apiCalls = 0; + nock("https://my-awesome-api.fake") + .filteringPath(/test=[^&]*/g, "test=XXX") + .get("/?test=XXX") + .reply(200, () => ++apiCalls) + .persist(); + + const children = jest.fn(); + children.mockReturnValue(
); + + const resolve = a => a; + const { rerender } = render( + + + {children} + + , + ); + + rerender( + + + {children} + + , + ); + + expect(apiCalls).toEqual(2); + }); + it("should refetch when resolve changes", () => { + let apiCalls = 0; + nock("https://my-awesome-api.fake") + .get("/") + .reply(200, () => ++apiCalls) + .persist(); + + const children = jest.fn(); + children.mockReturnValue(
); + + const { rerender } = render( + + {children} + , + ); + + const resolve = a => a; + rerender( + + + {children} + + , + ); + + expect(apiCalls).toEqual(2); + }); + }); }); diff --git a/src/Get.tsx b/src/Get.tsx index a7f555cc..82b55896 100644 --- a/src/Get.tsx +++ b/src/Get.tsx @@ -158,9 +158,8 @@ class ContextlessGet extends React.Component< } public componentDidUpdate(prevProps: GetProps) { - // If the path or base prop changes, refetch! - const { path, base } = this.props; - if (prevProps.path !== path || prevProps.base !== base) { + const { base, path, resolve } = prevProps; + if (base !== this.props.base || path !== this.props.path || resolve !== this.props.resolve) { if (!this.props.lazy) { this.fetch(); } From 48452d27a9787396c8a92060e98b1fcd47472e59 Mon Sep 17 00:00:00 2001 From: carlesba Date: Tue, 16 Oct 2018 19:49:30 +0200 Subject: [PATCH 2/3] Add comments for resolve issue when rerendiring Provider On tests we can't just rerender Get component but rerender Provider and Get all together. This makes Provider to create a new defaultResolve function on every rerender. To mock a real scenario, resolve function is created in tests and passed as property for Provider so we can keep same the instance on every rerender, which is what it would happen on a real app. --- src/Get.test.tsx | 64 +++++++++++++++++++++++------------------------- 1 file changed, 30 insertions(+), 34 deletions(-) diff --git a/src/Get.test.tsx b/src/Get.test.tsx index d63fa05b..e07c1180 100644 --- a/src/Get.test.tsx +++ b/src/Get.test.tsx @@ -527,20 +527,25 @@ describe("Get", () => { children.mockReturnValue(
); const resolve = a => a; + + /** + * A new instance of RestfulProvider is created on every rerender. + * This will create a new resolve function every time forcing Get to + * refetch. + * In a real app, only Get would be rerendered so resolve would be the + * same on every new render. To mimic that behavior, resolve is created + * ahead so Get will get the same instance on every rerender. + */ const { rerender } = render( - - - {children} - + + {children} , ); times(10, i => rerender( - - - {children} - + + {children} , ), ); @@ -561,18 +566,14 @@ describe("Get", () => { const resolve = a => a; const { rerender } = render( - - - {children} - + + {children} , ); rerender( - - - {children} - + + {children} , ); @@ -589,10 +590,8 @@ describe("Get", () => { const resolve = a => a; const { rerender } = render( - - - {children} - + + {children} , ); @@ -600,8 +599,8 @@ describe("Get", () => { .get("/") .reply(200, () => ++apiCalls); rerender( - - + + {children} , @@ -622,18 +621,14 @@ describe("Get", () => { const resolve = a => a; const { rerender } = render( - - - {children} - + + {children} , ); rerender( - - - {children} - + + {children} , ); @@ -649,16 +644,17 @@ describe("Get", () => { const children = jest.fn(); children.mockReturnValue(
); + const providerResolve = a => a; const { rerender } = render( - + {children} , ); - const resolve = a => a; + const newResolve = a => a; rerender( - - + + {children} , From 3aaaebf3036613354fa521760ef45f1d9757d973 Mon Sep 17 00:00:00 2001 From: carlesba Date: Wed, 17 Oct 2018 21:19:53 +0200 Subject: [PATCH 3/3] Update data when resolve changes --- src/Poll.test.tsx | 31 +++++++++++++++++++++++++++++++ src/Poll.tsx | 19 +++++++++++++------ 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/src/Poll.test.tsx b/src/Poll.test.tsx index e0352c72..7b40c2ac 100644 --- a/src/Poll.test.tsx +++ b/src/Poll.test.tsx @@ -407,6 +407,37 @@ describe("Poll", () => { await wait(() => expect(children.mock.calls.length).toBe(3)); expect(children.mock.calls[2][0]).toEqual({ data: "hello you" }); }); + + it("should update data when resolver changes", async () => { + nock("https://my-awesome-api.fake") + .get("/") + .reply(200, { hello: "world" }); + + const children = jest.fn(); + children.mockReturnValue(
); + + const resolve = data => ({ ...data, too: "bar" }); + const newResolve = data => ({ ...data, foo: "bar" }); + + const { rerender } = render( + + + {children} + + , + ); + + rerender( + + + {children} + + , + ); + + await wait(() => expect(children.mock.calls.length).toBe(3)); + expect(children.mock.calls[2][0]).toEqual({ hello: "world", foo: "bar" }); + }); }); describe("with lazy", () => { diff --git a/src/Poll.tsx b/src/Poll.tsx index c56e20d4..1f56532c 100644 --- a/src/Poll.tsx +++ b/src/Poll.tsx @@ -128,6 +128,10 @@ export interface PollState { * What data are we holding in here? */ data: GetState["data"]; + /** + * What data did we had before? + */ + previousData: GetState["data"]; /** * Are we loading? */ @@ -151,6 +155,7 @@ class ContextlessPoll extends React.Component< > { public readonly state: Readonly> = { data: null, + previousData: null, loading: !this.props.lazy, lastResponse: null, polling: !this.props.lazy, @@ -205,7 +210,7 @@ class ContextlessPoll extends React.Component< } // If we should keep going, - const { base, path, resolve, interval, wait } = this.props; + const { base, path, interval, wait } = this.props; const { lastPollIndex } = this.state; const requestOptions = this.getRequestOptions(); @@ -240,7 +245,8 @@ class ContextlessPoll extends React.Component< this.setState(prevState => ({ loading: false, lastResponse: response, - data: resolve ? resolve(data, prevState.data) : data, + previousData: prevState.data, + data, error: null, lastPollIndex: response.headers.get("x-polling-index") || undefined, })); @@ -290,8 +296,8 @@ class ContextlessPoll extends React.Component< } public render() { - const { lastResponse: response, data, polling, loading, error, finished } = this.state; - const { children, base, path } = this.props; + const { lastResponse: response, previousData, data, polling, loading, error, finished } = this.state; + const { children, base, path, resolve } = this.props; const meta: Meta = { response, @@ -309,8 +315,9 @@ class ContextlessPoll extends React.Component< stop: this.stop, start: this.start, }; - - return children(data, states, actions, meta); + // data is parsed only when poll has already resolved so response is defined + const resolvedData = response && resolve ? resolve(data, previousData) : data; + return children(resolvedData, states, actions, meta); } }