From 8a581136ca0782c9c894e0806c7eb0320f80fafc Mon Sep 17 00:00:00 2001 From: LFDM <1986gh@gmail.com> Date: Sun, 18 Mar 2018 10:20:28 +0100 Subject: [PATCH 01/15] refactor(with-data) Make clearing timeouts more flexible --- src/hocs/withData/index.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/hocs/withData/index.js b/src/hocs/withData/index.js index 8078d5b..cf7c93e 100644 --- a/src/hocs/withData/index.js +++ b/src/hocs/withData/index.js @@ -71,7 +71,7 @@ class Container extends Component { addResolvedData(field, data) { this.resolvedData[field] = data; if (this.resolvedDataTargetSize === Object.keys(this.resolvedData).length) { - this.clearPendingTimeout(); + this.clearTimeout('pendingScheduled'); this.rerender = true; this.safeSetState({ pending: false, @@ -83,7 +83,7 @@ class Container extends Component { } setError(field, error) { - this.clearPendingTimeout(); + this.clearTimeout('pendingScheduled'); this.safeSetState({ pending: false, pendingScheduled: false, @@ -91,11 +91,11 @@ class Container extends Component { }); } - clearPendingTimeout() { + clearTimeout(type) { const { timeouts } = this; - if (timeouts.pendingScheduled) { - clearTimeout(timeouts.pendingScheduled); - timeouts.pendingScheduled = null; + if (timeouts[type]) { + clearTimeout(timeouts[type]); + timeouts[type] = null; } } @@ -163,7 +163,7 @@ class Container extends Component { timeouts.pendingScheduled = setTimeout(() => { if (timeouts.pendingScheduled) { update(); - this.clearPendingTimeout(); + this.clearTimeout('pendingScheduled'); } }, delays.refetch); this.safeSetState({ pendingScheduled: true }); From d6effc0130f97c090bfbfe2096c8ee3a040a8d86 Mon Sep 17 00:00:00 2001 From: LFDM <1986gh@gmail.com> Date: Sun, 18 Mar 2018 10:20:42 +0100 Subject: [PATCH 02/15] feat(with-data) Add spec to document minimumPendingTime --- src/hocs/withData/index.spec.js | 40 +++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/src/hocs/withData/index.spec.js b/src/hocs/withData/index.spec.js index d6e2c5a..2fa98a1 100644 --- a/src/hocs/withData/index.spec.js +++ b/src/hocs/withData/index.spec.js @@ -9,6 +9,8 @@ import { withData } from '.'; const wait = (t = 1) => new Promise(res => setTimeout(() => res(), t)); +const getTimeDiff = (l1, l2) => l2.t - l1.t; + const peter = { id: 'peter', name: 'peter' }; const gernot = { id: 'gernot', name: 'gernot' }; const robin = { id: 'robin', name: 'robin' }; @@ -327,6 +329,44 @@ describe('withData', () => { }); }); }); + + fit('allows to specify a mininumPendingTime to reduce flicker', () => { + const minimumPendingTime = 10; + const api = build(createConfig()); + const { spy } = createSpyComponent(); + const { spy: pendingSpy, logger: pendingLogger } = createSpyComponent(); + const comp = withData({ + resolve: { + user: ({ userId }) => api.user.getUser(userId) + }, + pendingComponent: pendingSpy, + delays: { + refetch: 0, + minimumPendingTime + } + })(spy); + + let stateContainer = null; + + render(comp, { userId: 'peter' }, c => { stateContainer = c; }, ({ userId }) => ({ userId })); + + return wait().then(() => { + stateContainer.setState({ userId: 'gernot' }); + return wait().then(() => { + pendingLogger.expectRenderCount(2); + const firstMount = pendingLogger.getByType('componentWillMount')[0]; + const firstUnmount = pendingLogger.getByType('componentWillUnmount')[0]; + const secondMount = pendingLogger.getByType('componentWillMount')[1]; + const secondUnmount = pendingLogger.getByType('componentWillUnmount')[1]; + + const t1 = getTimeDiff(firstMount, firstUnmount); + const t2 = getTimeDiff(secondMount, secondUnmount); + + expect(t1).to.be.at.least(minimumPendingTime); + expect(t2).to.be.at.least(minimumPendingTime); + }); + }); + }); }); describe('pagination', () => { From e88c0d4ae25d7a1c540618ba06d6959125fddfb6 Mon Sep 17 00:00:00 2001 From: LFDM <1986gh@gmail.com> Date: Sun, 18 Mar 2018 10:25:34 +0100 Subject: [PATCH 03/15] feat(with-data) Add setTimeout implementation --- src/hocs/withData/index.js | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/hocs/withData/index.js b/src/hocs/withData/index.js index cf7c93e..a98008a 100644 --- a/src/hocs/withData/index.js +++ b/src/hocs/withData/index.js @@ -91,6 +91,11 @@ class Container extends Component { }); } + setTimeout(type, ...args) { + this.clearTimeout(type); + this.timeouts[type] = setTimeout(...args); + } + clearTimeout(type) { const { timeouts } = this; if (timeouts[type]) { @@ -160,7 +165,7 @@ class Container extends Component { }; if (delays.refetch) { const { timeouts } = this; - timeouts.pendingScheduled = setTimeout(() => { + this.setTimeout('pendingScheduled', () => { if (timeouts.pendingScheduled) { update(); this.clearTimeout('pendingScheduled'); @@ -197,16 +202,18 @@ class Container extends Component { } const DEFAULT_DELAYS = { - refetch: 0 + refetch: 0, + minimumPendingTime: 0 }; export function withData(conf) { return component => { + const delays = { ...DEFAULT_DELAYS, ...(conf.delays || {}) }; class WithDataWrapper extends PureComponent { render() { const props = { ...conf, - delays: conf.delays || DEFAULT_DELAYS, + delays, originalProps: this.props, component }; From a33db51f39ad6a35b65f37ea56ae0abe6ef1e939 Mon Sep 17 00:00:00 2001 From: LFDM <1986gh@gmail.com> Date: Sun, 18 Mar 2018 10:26:03 +0100 Subject: [PATCH 04/15] feat(with-data) Remove unnecessary state field --- src/hocs/withData/index.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/hocs/withData/index.js b/src/hocs/withData/index.js index a98008a..22e86d3 100644 --- a/src/hocs/withData/index.js +++ b/src/hocs/withData/index.js @@ -75,7 +75,6 @@ class Container extends Component { this.rerender = true; this.safeSetState({ pending: false, - pendingScheduled: false, resolvedProps: { ...this.resolvedData }, error: null }); @@ -86,7 +85,6 @@ class Container extends Component { this.clearTimeout('pendingScheduled'); this.safeSetState({ pending: false, - pendingScheduled: false, error }); } @@ -171,7 +169,6 @@ class Container extends Component { this.clearTimeout('pendingScheduled'); } }, delays.refetch); - this.safeSetState({ pendingScheduled: true }); } else { update(); } From 04e47b832c25c5665699d0a414596bafb07bbdc8 Mon Sep 17 00:00:00 2001 From: LFDM <1986gh@gmail.com> Date: Sun, 18 Mar 2018 10:26:27 +0100 Subject: [PATCH 05/15] fix(with-data) Unfocus spec --- src/hocs/withData/index.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hocs/withData/index.spec.js b/src/hocs/withData/index.spec.js index 2fa98a1..4ff86f0 100644 --- a/src/hocs/withData/index.spec.js +++ b/src/hocs/withData/index.spec.js @@ -330,7 +330,7 @@ describe('withData', () => { }); }); - fit('allows to specify a mininumPendingTime to reduce flicker', () => { + it('allows to specify a mininumPendingTime to reduce flicker', () => { const minimumPendingTime = 10; const api = build(createConfig()); const { spy } = createSpyComponent(); From efb2a0db5b4e77898690cd20a4a3d0f41cb1842c Mon Sep 17 00:00:00 2001 From: LFDM <1986gh@gmail.com> Date: Sun, 18 Mar 2018 10:27:27 +0100 Subject: [PATCH 06/15] feat(with-data) Add hasTimeout fn --- src/hocs/withData/index.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/hocs/withData/index.js b/src/hocs/withData/index.js index 22e86d3..43c95cf 100644 --- a/src/hocs/withData/index.js +++ b/src/hocs/withData/index.js @@ -89,6 +89,10 @@ class Container extends Component { }); } + hasTimeout(type) { + return !!this.timeouts[type]; + } + setTimeout(type, ...args) { this.clearTimeout(type); this.timeouts[type] = setTimeout(...args); @@ -162,9 +166,8 @@ class Container extends Component { this.safeSetState({ pending: true, pendingScheduled: false, error: null }); }; if (delays.refetch) { - const { timeouts } = this; this.setTimeout('pendingScheduled', () => { - if (timeouts.pendingScheduled) { + if (this.hasTimeout('pendingScheduled')) { update(); this.clearTimeout('pendingScheduled'); } From cbb86cf9fcd64a43f3672607b8fc42c3a585274e Mon Sep 17 00:00:00 2001 From: LFDM <1986gh@gmail.com> Date: Sun, 18 Mar 2018 10:37:17 +0100 Subject: [PATCH 07/15] feat(with-data) Make mininumPendingTime spec more robust --- src/hocs/withData/index.spec.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/hocs/withData/index.spec.js b/src/hocs/withData/index.spec.js index 4ff86f0..77e2b7d 100644 --- a/src/hocs/withData/index.spec.js +++ b/src/hocs/withData/index.spec.js @@ -332,6 +332,7 @@ describe('withData', () => { it('allows to specify a mininumPendingTime to reduce flicker', () => { const minimumPendingTime = 10; + const minimumPendingTimeWithThreshold = minimumPendingTime + 2; const api = build(createConfig()); const { spy } = createSpyComponent(); const { spy: pendingSpy, logger: pendingLogger } = createSpyComponent(); @@ -350,9 +351,9 @@ describe('withData', () => { render(comp, { userId: 'peter' }, c => { stateContainer = c; }, ({ userId }) => ({ userId })); - return wait().then(() => { + return wait(minimumPendingTimeWithThreshold).then(() => { stateContainer.setState({ userId: 'gernot' }); - return wait().then(() => { + return wait(minimumPendingTimeWithThreshold).then(() => { pendingLogger.expectRenderCount(2); const firstMount = pendingLogger.getByType('componentWillMount')[0]; const firstUnmount = pendingLogger.getByType('componentWillUnmount')[0]; From 621ac1fa4e3e7d863f13d6aada60b22526453ec6 Mon Sep 17 00:00:00 2001 From: LFDM <1986gh@gmail.com> Date: Sun, 18 Mar 2018 10:38:02 +0100 Subject: [PATCH 08/15] feat(with-data) Implement mininumPendingTime --- src/hocs/withData/index.js | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/src/hocs/withData/index.js b/src/hocs/withData/index.js index 43c95cf..b3eb191 100644 --- a/src/hocs/withData/index.js +++ b/src/hocs/withData/index.js @@ -71,16 +71,27 @@ class Container extends Component { addResolvedData(field, data) { this.resolvedData[field] = data; if (this.resolvedDataTargetSize === Object.keys(this.resolvedData).length) { - this.clearTimeout('pendingScheduled'); - this.rerender = true; - this.safeSetState({ - pending: false, - resolvedProps: { ...this.resolvedData }, - error: null - }); + const { minimumPendingTime } = this.props.delays; + + if (this.state.pending && minimumPendingTime) { + this.setTimeout('minimumPendingTime', () => this.publish(), minimumPendingTime); + } else { + this.publish(); + } } } + publish() { + this.clearTimeout('pendingScheduled'); + this.clearTimeout('minimumPendingTime'); + this.rerender = true; + this.safeSetState({ + pending: false, + resolvedProps: { ...this.resolvedData }, + error: null + }); + } + setError(field, error) { this.clearTimeout('pendingScheduled'); this.safeSetState({ @@ -101,6 +112,7 @@ class Container extends Component { clearTimeout(type) { const { timeouts } = this; if (timeouts[type]) { + const { timeouts } = this; clearTimeout(timeouts[type]); timeouts[type] = null; } From 185de31e698aa7e4fec2f3083aed9b388a181687 Mon Sep 17 00:00:00 2001 From: LFDM <1986gh@gmail.com> Date: Sun, 18 Mar 2018 10:38:23 +0100 Subject: [PATCH 09/15] refactor(with-data) Remove obsolete helper comments --- src/hocs/withData/index.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/hocs/withData/index.js b/src/hocs/withData/index.js index b3eb191..5fa87b3 100644 --- a/src/hocs/withData/index.js +++ b/src/hocs/withData/index.js @@ -236,10 +236,5 @@ export function withData(conf) { }; } -// wait with refetch spinner -// wait with initial spinner -// -// minimum time for spinner - withData.PAGINATION = PAGINATION; From 52b8edf0a54a30f459c99482ac9e9825664d6b67 Mon Sep 17 00:00:00 2001 From: LFDM <1986gh@gmail.com> Date: Sun, 18 Mar 2018 10:38:46 +0100 Subject: [PATCH 10/15] refactor(with-data) Remove double declaration --- src/hocs/withData/index.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/hocs/withData/index.js b/src/hocs/withData/index.js index 5fa87b3..8886668 100644 --- a/src/hocs/withData/index.js +++ b/src/hocs/withData/index.js @@ -112,7 +112,6 @@ class Container extends Component { clearTimeout(type) { const { timeouts } = this; if (timeouts[type]) { - const { timeouts } = this; clearTimeout(timeouts[type]); timeouts[type] = null; } From b1d84dc403737572074fa9d4a68fda5fbadf0df7 Mon Sep 17 00:00:00 2001 From: LFDM <1986gh@gmail.com> Date: Sun, 18 Mar 2018 10:41:49 +0100 Subject: [PATCH 11/15] refactor(with-data) Extract constant fns --- src/hocs/withData/index.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/hocs/withData/index.js b/src/hocs/withData/index.js index 8886668..7080869 100644 --- a/src/hocs/withData/index.js +++ b/src/hocs/withData/index.js @@ -6,6 +6,9 @@ import { PAGINATION } from './retrievers'; +const TRUE_FN = () => true; +const NULL_FN = () => null; + class Container extends Component { constructor(props) { super(props); @@ -44,7 +47,7 @@ class Container extends Component { } componentWillReceiveProps(newProps) { - const { shouldRefetch = (() => true) } = newProps; + const { shouldRefetch = TRUE_FN } = newProps; if (!shouldRefetch(this.props.originalProps, newProps.originalProps)) { return; } @@ -162,7 +165,7 @@ class Container extends Component { publishError: publishError(key), getProps, getter: poll[key].resolve, - interval: (poll[key].interval || (() => null))(originalProps) + interval: (poll[key].interval || NULL_FN)(originalProps) }); }); From eddfd72da6ac76a35c37bcc380c016c48f99220c Mon Sep 17 00:00:00 2001 From: LFDM <1986gh@gmail.com> Date: Sun, 18 Mar 2018 10:46:10 +0100 Subject: [PATCH 12/15] refactor(with-data) Decrease waiting time for faster specs in minimumPendingTime spec --- src/hocs/withData/index.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hocs/withData/index.spec.js b/src/hocs/withData/index.spec.js index 77e2b7d..6e7fcaf 100644 --- a/src/hocs/withData/index.spec.js +++ b/src/hocs/withData/index.spec.js @@ -331,7 +331,7 @@ describe('withData', () => { }); it('allows to specify a mininumPendingTime to reduce flicker', () => { - const minimumPendingTime = 10; + const minimumPendingTime = 5; const minimumPendingTimeWithThreshold = minimumPendingTime + 2; const api = build(createConfig()); const { spy } = createSpyComponent(); From 9192175420e106ac0043cb9c1523cc5fd07ad824 Mon Sep 17 00:00:00 2001 From: LFDM <1986gh@gmail.com> Date: Sun, 18 Mar 2018 10:53:33 +0100 Subject: [PATCH 13/15] feat(with-data) Make sure minimumPendingTime also applies to errors --- src/hocs/withData/index.js | 46 +++++++++++++++++++++++--------------- 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/src/hocs/withData/index.js b/src/hocs/withData/index.js index 7080869..baba988 100644 --- a/src/hocs/withData/index.js +++ b/src/hocs/withData/index.js @@ -17,7 +17,8 @@ class Container extends Component { this.resolvedDataTargetSize = 0; this.timeouts = { - pendingScheduled: null + pendingScheduled: null, + minimumPendingTime: null }; this.retrievers = {}; @@ -74,32 +75,41 @@ class Container extends Component { addResolvedData(field, data) { this.resolvedData[field] = data; if (this.resolvedDataTargetSize === Object.keys(this.resolvedData).length) { - const { minimumPendingTime } = this.props.delays; + this.publish(); + } + } - if (this.state.pending && minimumPendingTime) { - this.setTimeout('minimumPendingTime', () => this.publish(), minimumPendingTime); - } else { - this.publish(); - } + withMinimumPendingTime(fn) { + const { minimumPendingTime } = this.props.delays; + if (this.state.pending && minimumPendingTime) { + this.setTimeout('minimumPendingTime', () => { + this.clearTimeout('minimumPendingTime'); + fn(); + }, minimumPendingTime); + } else { + fn(); } } publish() { - this.clearTimeout('pendingScheduled'); - this.clearTimeout('minimumPendingTime'); - this.rerender = true; - this.safeSetState({ - pending: false, - resolvedProps: { ...this.resolvedData }, - error: null + this.withMinimumPendingTime(() => { + this.clearTimeout('pendingScheduled'); + this.rerender = true; + this.safeSetState({ + pending: false, + resolvedProps: { ...this.resolvedData }, + error: null + }); }); } setError(field, error) { - this.clearTimeout('pendingScheduled'); - this.safeSetState({ - pending: false, - error + this.withMinimumPendingTime(() => { + this.clearTimeout('pendingScheduled'); + this.safeSetState({ + pending: false, + error + }); }); } From e7e6659ff673026516b1997d3c610dd8ce56bb6a Mon Sep 17 00:00:00 2001 From: LFDM <1986gh@gmail.com> Date: Sun, 18 Mar 2018 11:20:25 +0100 Subject: [PATCH 14/15] feat(with-data) Allow to react to props when defining delays --- src/hocs/withData/index.js | 25 +++++++++++++++---------- src/hocs/withData/index.spec.js | 16 ++++++++-------- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/src/hocs/withData/index.js b/src/hocs/withData/index.js index baba988..99a4a9a 100644 --- a/src/hocs/withData/index.js +++ b/src/hocs/withData/index.js @@ -8,6 +8,7 @@ import { const TRUE_FN = () => true; const NULL_FN = () => null; +const EMPTY_OBJ_FN = () => {}; class Container extends Component { constructor(props) { @@ -44,17 +45,17 @@ class Container extends Component { componentWillMount() { this.setupRetrievers(this.props); - this.trigger({}); + this.trigger(this.props, true); } - componentWillReceiveProps(newProps) { - const { shouldRefetch = TRUE_FN } = newProps; - if (!shouldRefetch(this.props.originalProps, newProps.originalProps)) { + componentWillReceiveProps(nextProps) { + const { shouldRefetch = TRUE_FN } = nextProps; + if (!shouldRefetch(this.props.originalProps, nextProps.originalProps)) { return; } this.destroy(); - this.setupRetrievers(newProps); - this.trigger(newProps.delays); + this.setupRetrievers(nextProps); + this.trigger(nextProps, false); } componentWillUnmount() { @@ -80,7 +81,7 @@ class Container extends Component { } withMinimumPendingTime(fn) { - const { minimumPendingTime } = this.props.delays; + const { minimumPendingTime } = this.props.delays(this.props.originalProps); if (this.state.pending && minimumPendingTime) { this.setTimeout('minimumPendingTime', () => { this.clearTimeout('minimumPendingTime'); @@ -182,14 +183,15 @@ class Container extends Component { this.resolvedDataTargetSize = resolveKeys.length + observeKeys.length + pollKeys.length; } - trigger(delays) { + trigger(props, firstRender) { this.rerender = false; const update = () => { this.rerender = true; this.resolvedData = {}; this.safeSetState({ pending: true, pendingScheduled: false, error: null }); }; - if (delays.refetch) { + const delays = props.delays(props.originalProps); + if (!firstRender && delays.refetch) { this.setTimeout('pendingScheduled', () => { if (this.hasTimeout('pendingScheduled')) { update(); @@ -232,7 +234,10 @@ const DEFAULT_DELAYS = { export function withData(conf) { return component => { - const delays = { ...DEFAULT_DELAYS, ...(conf.delays || {}) }; + const delays = (props) => ({ + ...DEFAULT_DELAYS, + ...(conf.delays || EMPTY_OBJ_FN)(props) + }); class WithDataWrapper extends PureComponent { render() { const props = { diff --git a/src/hocs/withData/index.spec.js b/src/hocs/withData/index.spec.js index 6e7fcaf..6e4c19b 100644 --- a/src/hocs/withData/index.spec.js +++ b/src/hocs/withData/index.spec.js @@ -241,9 +241,9 @@ describe('withData', () => { user: ({ userId }) => api.user.getUser(userId) }, pendingComponent: pendingSpy, - delays: { + delays: () => ({ refetch: 0 - } + }) })(spy); let stateContainer = null; @@ -280,9 +280,9 @@ describe('withData', () => { user: ({ userId }) => api.user.getUser(userId) }, pendingComponent: pendingSpy, - delays: { + delays: () => ({ refetch: 100 - } + }) })(spy); let stateContainer = null; @@ -308,9 +308,9 @@ describe('withData', () => { resolve: { user: ({ userId }) => api.user.getUser(userId) }, - delays: { + delays: () => ({ refetch: 100 - } + }) })(spy); let stateContainer = null; @@ -341,10 +341,10 @@ describe('withData', () => { user: ({ userId }) => api.user.getUser(userId) }, pendingComponent: pendingSpy, - delays: { + delays: () => ({ refetch: 0, minimumPendingTime - } + }) })(spy); let stateContainer = null; From c7a6de7f73d83b3570788c5bb4167cd7a3be875f Mon Sep 17 00:00:00 2001 From: LFDM <1986gh@gmail.com> Date: Mon, 19 Mar 2018 08:44:41 +0100 Subject: [PATCH 15/15] feat(with-data) Add comment about somewhat broken test --- src/hocs/withData/index.spec.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/hocs/withData/index.spec.js b/src/hocs/withData/index.spec.js index 6e4c19b..4513ac1 100644 --- a/src/hocs/withData/index.spec.js +++ b/src/hocs/withData/index.spec.js @@ -351,6 +351,8 @@ describe('withData', () => { render(comp, { userId: 'peter' }, c => { stateContainer = c; }, ({ userId }) => ({ userId })); + // the first render ideally shouldn't wait, if the promise resolves + // immediately return wait(minimumPendingTimeWithThreshold).then(() => { stateContainer.setState({ userId: 'gernot' }); return wait(minimumPendingTimeWithThreshold).then(() => {