From 67757c257aae635642bda39e3df2c305dc5bacf9 Mon Sep 17 00:00:00 2001 From: LFDM <1986gh@gmail.com> Date: Sat, 17 Mar 2018 09:39:01 +0100 Subject: [PATCH 01/15] feat(with-data) Also export PAGINATION helpers --- src/hocs/withData/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hocs/withData/index.js b/src/hocs/withData/index.js index 8b7c9c5..ad00e98 100644 --- a/src/hocs/withData/index.js +++ b/src/hocs/withData/index.js @@ -1,6 +1,6 @@ import { createElement, Component, PureComponent } from 'react'; -const PAGINATION = { +export const PAGINATION = { TYPE: { OFFSET_AND_LIMIT: 'offsetAndLimit', CURSOR: 'cursor' From 6dfdb8a9af6ecd8cd2c323aaa103c5cdbcf103e3 Mon Sep 17 00:00:00 2001 From: LFDM <1986gh@gmail.com> Date: Sat, 17 Mar 2018 09:57:43 +0100 Subject: [PATCH 02/15] refactor(with-data) Move retrievers out of main file --- src/hocs/withData/index.js | 336 ++------------------------------ src/hocs/withData/retrievers.js | 324 ++++++++++++++++++++++++++++++ 2 files changed, 335 insertions(+), 325 deletions(-) create mode 100644 src/hocs/withData/retrievers.js diff --git a/src/hocs/withData/index.js b/src/hocs/withData/index.js index ad00e98..4911eb1 100644 --- a/src/hocs/withData/index.js +++ b/src/hocs/withData/index.js @@ -1,324 +1,10 @@ import { createElement, Component, PureComponent } from 'react'; - -export const PAGINATION = { - TYPE: { - OFFSET_AND_LIMIT: 'offsetAndLimit', - CURSOR: 'cursor' - }, - MODE: { - INFINITE: 'infinite' - }, - PRESET: { - infiniteOffsetAndLimit: (initialSize, nextSize = initialSize) => ({ - type: PAGINATION.TYPE.OFFSET_AND_LIMIT, - mode: PAGINATION.MODE.INFINITE, - getNextPage: ({ limit, offset }) => { - if (offset === null) { - return { offset: 0, limit: initialSize }; - } - return { offset: offset + limit, limit: nextSize }; - } - }), - infiniteCursor: (startingCursor) => ({ - type: PAGINATION.TYPE.CURSOR, - mode: PAGINATION.MODE.INFINITE, - startingCursor, - getCursor: (r) => r.cursor, - reduceResults: (mem, l) => [...mem, ...l] - }) - } -}; - -const every = (predicate, collection) => { - for (let i = 0; i < collection.length; i++) { - if (!predicate(collection[i])) { - return false; - } - } - return true; -}; - -const any = (predicate, collection) => { - for (let i = 0; i < collection.length; i++) { - if (predicate(collection[i])) { - return true; - } - } - return false; -}; - -class Retriever { - constructor({ type, name, getter, getProps, publishData, publishError }) { - this.type = type; - this.name = name; - this.getter = getter; - this.getProps = getProps; - this.publishData = publishData; - this.publishError = publishError; - } - - get() { - const promise = this.getter(this.getProps()); - if (!promise || !promise.then) { - throw new Error(`${this.type} for ${this.name} did not return a promise!`); - } - return promise.then(this.publishData, this.publishError); - } - - // eslint-disable-next-line class-methods-use-this - mergeProps(props) { - return props; - } - - // eslint-disable-next-line class-methods-use-this - onDestroy() {} -} - -class ResolveRetriever extends Retriever { - constructor(args) { - super({ type: 'resolve', ...args }); - } -} - -class PollRetriever extends Retriever { - constructor(args) { - super({ type: 'poll', ...args }); - this.interval = args.interval ? setInterval(() => this.get(), args.interval) : null; - } - - onDestroy() { - if (this.interval) { - clearInterval(this.interval); - this.interval = null; - } - } -} - -class ObserveRetriever extends Retriever { - constructor(args) { - super({ type: 'observe', ...args }); - - this.subscription = null; - } - - get() { - const observable = this.getter(this.getProps()); - if (!observable || !observable) { - throw new Error(`${this.type} for ${this.name} did not expose a subscribe function`); - } - this.subscription = observable.subscribe(this.publishData, this.publishError); - return Promise.resolve(); - } - - onDestroy() { - if (this.subscription && this.subscription.unsubscribe) { - this.subscription.unsubscribe(); - } - } -} - -const mergePaginateProps = (props, name, obj) => ({ - ...props, - paginate: { - ...props.paginate, - [name]: obj - } -}); - -class PaginatedInfiniteCursorRetriever extends Retriever { - constructor({ pagerConfig, ...args}) { - super({ type: 'resolve with cursor', ...args }); - - this.pagerConfig = pagerConfig; - - this.pastCursors = []; - this.nextCursors = pagerConfig.startingCursor || null; - - this.hasNext = true; - - this.pending = null; - } - - get() { - if (this.pending) { - return this.pending; - } - - const props = this.getProps(); - - this.pending = Promise.all([ - ...this.pastCursors.map((cursor) => this.getter(props, cursor)), - this.getter(props, this.nextCursor).then(response => { - const nextCursor = this.pagerConfig.getCursor(response); - - this.pastCursors.push(this.nextCursor); - this.nextCursor = nextCursor; - this.hasNext = !!nextCursor; - this.pending = null; - - return response; - }) - ]).then((results) => this.publishData(results.reduce(this.pagerConfig.reduceResults))); - - this.pending.catch((err) => { - this.pending = null; - return Promise.reject(err); - }); - - return this.pending; - } - - mergeProps(props) { - return mergePaginateProps(props, this.name, { - getNext: () => this.get(), - hasNext: this.hasNext - }); - } -} - -class PaginatedInfiniteOffsetAndLimitResolveRetriever extends Retriever { - constructor({ pagerConfig, ...args }) { - super({ type: 'resolve paginated', ...args }); - this.pagerConfig = pagerConfig; - - this.pagers = []; - - this.queueNext(); - } - - get() { - const props = this.getProps(); - return Promise.all(this.pagers.map((pager) => this.getter(props, pager))).then( - (lists) => this.publishData(lists.reduce((mem, list) => [...mem, ...list], [])), - (err) => Promise.reject(err) - ); - } - - queueNext() { - const prevPager = this.pagers[this.pagers.length - 1] || { limit: null, offset: null }; - const nextPager = this.pagerConfig.getNextPage(prevPager); - this.pagers.push(nextPager); - } - - mergeProps(props) { - return mergePaginateProps(props, this.name, { - getNext: () => { - this.queueNext(); - return this.get(); - } - }); - } -} - -class PaginatedInfiniteOffsetAndLimitObserveRetriever extends Retriever { - constructor({ pagerConfig, ...args }) { - super({ type: 'observe paginated', ...args }); - this.pagerConfig = pagerConfig; - - this.pagerSubscriptions = []; - - this.queueNext(); - } - - get() { - const props = this.getProps(); - return new Promise((resolve) => { - const tryResolve = (d) => { - if (every(p_ => p_.data, this.pagerSubscriptions)) { - resolve(d); - } - }; - this.pagerSubscriptions = this.pagerSubscriptions.map((p) => { - if (!p.subscription) { - p.firstLoad = true; - p.subscription = this.getter(props, p.pager).subscribe( - (data) => { - p.data = data; - p.firstLoad = false; - this.tryToPublish(); - tryResolve(data); - }, - this.publishError - ); - } - return p; - }); - }); - } - - queueNext() { - const pagers = this.pagerSubscriptions.map((p) => p.pager); - const prevPager = pagers[pagers.length - 1] || { limit: null, offset: null }; - const nextPager = this.pagerConfig.getNextPage(prevPager); - const p = { - pager: nextPager, - subscription: null, - data: null, - error: null - }; - this.pagerSubscriptions.push(p); - } - - tryToPublish() { - const result = []; - for (let i = 0; i < this.pagerSubscriptions.length; i++) { - const p = this.pagerSubscriptions[i]; - if (!p.data) { - return; - } - result.push(...p.data); - } - this.publishData(result); - } - - mergeProps(props) { - return mergePaginateProps(props, this.name, { - getNext: () => { - this.queueNext(); - return this.get(); - }, - isLoading: any(p => !p.data, this.pagerSubscriptions) - }); - } - - onDestroy() { - this.pagerSubscriptions.forEach((p) => { - if (p.subscription && p.subscription.unsubscribe) { - p.subscription.unsubscribe(); - } - }); - } -} - -const isInfiniteOffsetAndLimitPager = ({ mode, type }) => { - return mode === PAGINATION.MODE.INFINITE && type === PAGINATION.TYPE.OFFSET_AND_LIMIT; -}; - -const isInfiniteCursor = ({ mode, type }) => { - return mode === PAGINATION.MODE.INFINITE && type === PAGINATION.TYPE.CURSOR; -}; - -const getResolveRetriever = (pagerConfig) => { - if (pagerConfig) { - if (isInfiniteOffsetAndLimitPager(pagerConfig)) { - return PaginatedInfiniteOffsetAndLimitResolveRetriever; - } - - if (isInfiniteCursor(pagerConfig)) { - return PaginatedInfiniteCursorRetriever; - } - } - return ResolveRetriever; -}; - -const getObserveRetriever = (pagerConfig) => { - if (pagerConfig) { - if (isInfiniteOffsetAndLimitPager(pagerConfig)) { - return PaginatedInfiniteOffsetAndLimitObserveRetriever; - } - } - return ObserveRetriever; -}; +import { + getResolveRetriever, + getObserveRetriever, + getPollRetriever, + PAGINATION +} from './retrievers'; class Container extends Component { constructor(props) { @@ -426,15 +112,15 @@ class Container extends Component { }); pollKeys.forEach((key) => { - const getter = poll[key].resolve; - const interval = (poll[key].interval || (() => null))(originalProps); - this.retrievers[key] = new PollRetriever({ + const pagerConfig = paginate[key]; + const Constructor = getPollRetriever(pagerConfig); + this.retrievers[key] = new Constructor({ name: key, publishData: publishData(key), publishError: publishError(key), getProps, - getter, - interval + getter: poll[key].resolve, + interval: (poll[key].interval || (() => null))(originalProps) }); }); diff --git a/src/hocs/withData/retrievers.js b/src/hocs/withData/retrievers.js new file mode 100644 index 0000000..07c08b8 --- /dev/null +++ b/src/hocs/withData/retrievers.js @@ -0,0 +1,324 @@ +export const PAGINATION = { + TYPE: { + OFFSET_AND_LIMIT: 'offsetAndLimit', + CURSOR: 'cursor' + }, + MODE: { + INFINITE: 'infinite' + }, + PRESET: { + infiniteOffsetAndLimit: (initialSize, nextSize = initialSize) => ({ + type: PAGINATION.TYPE.OFFSET_AND_LIMIT, + mode: PAGINATION.MODE.INFINITE, + getNextPage: ({ limit, offset }) => { + if (offset === null) { + return { offset: 0, limit: initialSize }; + } + return { offset: offset + limit, limit: nextSize }; + } + }), + infiniteCursor: (startingCursor) => ({ + type: PAGINATION.TYPE.CURSOR, + mode: PAGINATION.MODE.INFINITE, + startingCursor, + getCursor: (r) => r.cursor, + reduceResults: (mem, l) => [...mem, ...l] + }) + } +}; + +const every = (predicate, collection) => { + for (let i = 0; i < collection.length; i++) { + if (!predicate(collection[i])) { + return false; + } + } + return true; +}; + +const any = (predicate, collection) => { + for (let i = 0; i < collection.length; i++) { + if (predicate(collection[i])) { + return true; + } + } + return false; +}; + +class Retriever { + constructor({ type, name, getter, getProps, publishData, publishError }) { + this.type = type; + this.name = name; + this.getter = getter; + this.getProps = getProps; + this.publishData = publishData; + this.publishError = publishError; + } + + get() { + const promise = this.getter(this.getProps()); + if (!promise || !promise.then) { + throw new Error(`${this.type} for ${this.name} did not return a promise!`); + } + return promise.then(this.publishData, this.publishError); + } + + // eslint-disable-next-line class-methods-use-this + mergeProps(props) { + return props; + } + + // eslint-disable-next-line class-methods-use-this + onDestroy() {} +} + +class ResolveRetriever extends Retriever { + constructor(args) { + super({ type: 'resolve', ...args }); + } +} + +class PollRetriever extends Retriever { + constructor(args) { + super({ type: 'poll', ...args }); + this.interval = args.interval ? setInterval(() => this.get(), args.interval) : null; + } + + onDestroy() { + if (this.interval) { + clearInterval(this.interval); + this.interval = null; + } + } +} + +class ObserveRetriever extends Retriever { + constructor(args) { + super({ type: 'observe', ...args }); + + this.subscription = null; + } + + get() { + const observable = this.getter(this.getProps()); + if (!observable || !observable) { + throw new Error(`${this.type} for ${this.name} did not expose a subscribe function`); + } + this.subscription = observable.subscribe(this.publishData, this.publishError); + return Promise.resolve(); + } + + onDestroy() { + if (this.subscription && this.subscription.unsubscribe) { + this.subscription.unsubscribe(); + } + } +} + +const mergePaginateProps = (props, name, obj) => ({ + ...props, + paginate: { + ...props.paginate, + [name]: obj + } +}); + +class PaginatedInfiniteCursorRetriever extends Retriever { + constructor({ pagerConfig, ...args}) { + super({ type: 'resolve with cursor', ...args }); + + this.pagerConfig = pagerConfig; + + this.pastCursors = []; + this.nextCursors = pagerConfig.startingCursor || null; + + this.hasNext = true; + + this.pending = null; + } + + get() { + if (this.pending) { + return this.pending; + } + + const props = this.getProps(); + + this.pending = Promise.all([ + ...this.pastCursors.map((cursor) => this.getter(props, cursor)), + this.getter(props, this.nextCursor).then(response => { + const nextCursor = this.pagerConfig.getCursor(response); + + this.pastCursors.push(this.nextCursor); + this.nextCursor = nextCursor; + this.hasNext = !!nextCursor; + this.pending = null; + + return response; + }) + ]).then((results) => this.publishData(results.reduce(this.pagerConfig.reduceResults))); + + this.pending.catch((err) => { + this.pending = null; + return Promise.reject(err); + }); + + return this.pending; + } + + mergeProps(props) { + return mergePaginateProps(props, this.name, { + getNext: () => this.get(), + hasNext: this.hasNext + }); + } +} + +class PaginatedInfiniteOffsetAndLimitResolveRetriever extends Retriever { + constructor({ pagerConfig, ...args }) { + super({ type: 'resolve paginated', ...args }); + this.pagerConfig = pagerConfig; + + this.pagers = []; + + this.queueNext(); + } + + get() { + const props = this.getProps(); + return Promise.all(this.pagers.map((pager) => this.getter(props, pager))).then( + (lists) => this.publishData(lists.reduce((mem, list) => [...mem, ...list], [])), + (err) => Promise.reject(err) + ); + } + + queueNext() { + const prevPager = this.pagers[this.pagers.length - 1] || { limit: null, offset: null }; + const nextPager = this.pagerConfig.getNextPage(prevPager); + this.pagers.push(nextPager); + } + + mergeProps(props) { + return mergePaginateProps(props, this.name, { + getNext: () => { + this.queueNext(); + return this.get(); + } + }); + } +} + +class PaginatedInfiniteOffsetAndLimitObserveRetriever extends Retriever { + constructor({ pagerConfig, ...args }) { + super({ type: 'observe paginated', ...args }); + this.pagerConfig = pagerConfig; + + this.pagerSubscriptions = []; + + this.queueNext(); + } + + get() { + const props = this.getProps(); + return new Promise((resolve) => { + const tryResolve = (d) => { + if (every(p_ => p_.data, this.pagerSubscriptions)) { + resolve(d); + } + }; + this.pagerSubscriptions = this.pagerSubscriptions.map((p) => { + if (!p.subscription) { + p.firstLoad = true; + p.subscription = this.getter(props, p.pager).subscribe( + (data) => { + p.data = data; + p.firstLoad = false; + this.tryToPublish(); + tryResolve(data); + }, + this.publishError + ); + } + return p; + }); + }); + } + + queueNext() { + const pagers = this.pagerSubscriptions.map((p) => p.pager); + const prevPager = pagers[pagers.length - 1] || { limit: null, offset: null }; + const nextPager = this.pagerConfig.getNextPage(prevPager); + const p = { + pager: nextPager, + subscription: null, + data: null, + error: null + }; + this.pagerSubscriptions.push(p); + } + + tryToPublish() { + const result = []; + for (let i = 0; i < this.pagerSubscriptions.length; i++) { + const p = this.pagerSubscriptions[i]; + if (!p.data) { + return; + } + result.push(...p.data); + } + this.publishData(result); + } + + mergeProps(props) { + return mergePaginateProps(props, this.name, { + getNext: () => { + this.queueNext(); + return this.get(); + }, + isLoading: any(p => !p.data, this.pagerSubscriptions) + }); + } + + onDestroy() { + this.pagerSubscriptions.forEach((p) => { + if (p.subscription && p.subscription.unsubscribe) { + p.subscription.unsubscribe(); + } + }); + } +} + +const isInfiniteOffsetAndLimitPager = ({ mode, type }) => { + return mode === PAGINATION.MODE.INFINITE && type === PAGINATION.TYPE.OFFSET_AND_LIMIT; +}; + +const isInfiniteCursor = ({ mode, type }) => { + return mode === PAGINATION.MODE.INFINITE && type === PAGINATION.TYPE.CURSOR; +}; + +export const getResolveRetriever = (pagerConfig) => { + if (pagerConfig) { + if (isInfiniteOffsetAndLimitPager(pagerConfig)) { + return PaginatedInfiniteOffsetAndLimitResolveRetriever; + } + + if (isInfiniteCursor(pagerConfig)) { + return PaginatedInfiniteCursorRetriever; + } + } + return ResolveRetriever; +}; + +export const getObserveRetriever = (pagerConfig) => { + if (pagerConfig) { + if (isInfiniteOffsetAndLimitPager(pagerConfig)) { + return PaginatedInfiniteOffsetAndLimitObserveRetriever; + } + } + return ObserveRetriever; +}; + +export const getPollRetriever = () => { + return PollRetriever; +}; + From 98a6a7ed7d6148e55dbca997e549a4097c87ae83 Mon Sep 17 00:00:00 2001 From: LFDM <1986gh@gmail.com> Date: Sat, 17 Mar 2018 21:51:54 +0100 Subject: [PATCH 03/15] feat(with-data) Don't immediately go to pending mode when delay is specified --- src/hocs/withData/index.js | 63 +++++++++++++++++++++++++++------ src/hocs/withData/index.spec.js | 32 +++++++++++++++++ 2 files changed, 85 insertions(+), 10 deletions(-) diff --git a/src/hocs/withData/index.js b/src/hocs/withData/index.js index 4911eb1..e7fb0f3 100644 --- a/src/hocs/withData/index.js +++ b/src/hocs/withData/index.js @@ -6,6 +6,8 @@ import { PAGINATION } from './retrievers'; +const incrementVersion = version => (version > 99 ? 0 : version + 1); + class Container extends Component { constructor(props) { super(props); @@ -13,6 +15,8 @@ class Container extends Component { this.resolvedData = {}; this.resolvedDataTargetSize = 0; + this.pendingTimeout = null; + this.retrievers = {}; this.subscriptions = []; @@ -23,7 +27,8 @@ class Container extends Component { this.state = { pending: false, error: null, - resolvedProps: null + resolvedProps: null, + version: 0 }; } @@ -36,7 +41,7 @@ class Container extends Component { componentWillMount() { this.setupRetrievers(this.props); - this.trigger(); + this.trigger({}); } componentWillReceiveProps(newProps) { @@ -46,7 +51,7 @@ class Container extends Component { } this.destroy(); this.setupRetrievers(newProps); - this.trigger(); + this.trigger(newProps.delays); } componentWillUnmount() { @@ -63,16 +68,30 @@ class Container extends Component { addResolvedData(field, data) { this.resolvedData[field] = data; if (this.resolvedDataTargetSize === Object.keys(this.resolvedData).length) { - this.safeSetState({ + this.clearPendingTimeout(); + this.safeSetState(prevState => ({ pending: false, resolvedProps: { ...this.resolvedData }, - error: null - }); + error: null, + version: incrementVersion(prevState.version) + })); } } setError(field, error) { - this.safeSetState({ pending: false, error }); + this.clearPendingTimeout(); + this.safeSetState(prevState => ({ + pending: false, + error, + version: incrementVersion(prevState.version) + })); + } + + clearPendingTimeout() { + if (this.pendingTimeout) { + clearTimeout(this.pendingTimeout); + this.pendingTimeout = null; + } } setupRetrievers(props) { @@ -127,8 +146,18 @@ class Container extends Component { this.resolvedDataTargetSize = resolveKeys.length + observeKeys.length + pollKeys.length; } - trigger() { - this.safeSetState({ pending: true, error: null }); + trigger(delays) { + const update = () => this.safeSetState({ pending: true, error: null }); + if (delays.refetch) { + this.pendingTimeout = setTimeout(() => { + if (this.pendingTimeout) { + update(); + this.pendingTimeout = null; + } + }, delays.refetch); + } else { + update(); + } Object.keys(this.retrievers).forEach((key) => { this.retrievers[key].get(); @@ -155,11 +184,20 @@ class Container extends Component { } } +const DEFAULT_DELAYS = { + refetch: 0 +}; + export function withData(conf) { return component => { class WithDataWrapper extends PureComponent { render() { - const props = { ...conf, originalProps: this.props, component }; + const props = { + ...conf, + delays: conf.delays || DEFAULT_DELAYS, + originalProps: this.props, + component + }; return createElement(Container, props); } } @@ -167,5 +205,10 @@ export function withData(conf) { }; } +// wait with refetch spinner +// wait with initial spinner +// +// minimum time for spinner + withData.PAGINATION = PAGINATION; diff --git a/src/hocs/withData/index.spec.js b/src/hocs/withData/index.spec.js index 4cba2ef..a8190a4 100644 --- a/src/hocs/withData/index.spec.js +++ b/src/hocs/withData/index.spec.js @@ -161,6 +161,38 @@ describe('withData', () => { }); }); + describe('delay', () => { + it('does not show pending state immediately when delay is requested', () => { + const api = build(createConfig()); + const spy = createSpyComponent(); + const pendingSpy = createSpyComponent(); + const comp = withData({ + resolve: { + user: ({ userId }) => api.user.getUser(userId) + }, + pendingComponent: pendingSpy, + delays: { + refetch: 100 + } + })(spy); + + let stateContainer = null; + + // prefill the cache + render(comp, { userId: 'peter' }, c => { stateContainer = c; }, ({ userId }) => ({ userId })); + + return delay().then(() => { + expect(spy).to.have.been.calledOnce; + expect(pendingSpy).to.have.been.calledOne; + stateContainer.setState({ userId: 'gernot' }); + return delay().then(() => { + expect(pendingSpy).to.have.been.calledOnce; + expect(spy).to.have.been.calledThrice; + }); + }); + }); + }); + describe('pagination', () => { it('allows to paginate with limit and offset (resolve)', () => { const api = build(createConfig(), [observable()]); From 1681769641e9bc7c2d1e4c761d4eef95043a953a Mon Sep 17 00:00:00 2001 From: LFDM <1986gh@gmail.com> Date: Sat, 17 Mar 2018 21:56:05 +0100 Subject: [PATCH 04/15] refactor(with-data) Improve naming to support more use cases --- src/hocs/withData/index.js | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/hocs/withData/index.js b/src/hocs/withData/index.js index e7fb0f3..27b662d 100644 --- a/src/hocs/withData/index.js +++ b/src/hocs/withData/index.js @@ -15,7 +15,9 @@ class Container extends Component { this.resolvedData = {}; this.resolvedDataTargetSize = 0; - this.pendingTimeout = null; + this.timeouts = { + pendingScheduled: null + }; this.retrievers = {}; @@ -88,9 +90,10 @@ class Container extends Component { } clearPendingTimeout() { - if (this.pendingTimeout) { - clearTimeout(this.pendingTimeout); - this.pendingTimeout = null; + const { timeouts } = this; + if (timeouts.pendingScheduled) { + clearTimeout(timeouts.pendingScheduled); + timeouts.pendingScheduled = null; } } @@ -149,10 +152,11 @@ class Container extends Component { trigger(delays) { const update = () => this.safeSetState({ pending: true, error: null }); if (delays.refetch) { - this.pendingTimeout = setTimeout(() => { - if (this.pendingTimeout) { + const { timeouts } = this; + timeouts.pendingScheduled = setTimeout(() => { + if (timeouts.pendingScheduled) { update(); - this.pendingTimeout = null; + this.clearPendingTimeout(); } }, delays.refetch); } else { From d455315eeaabbc24ef6618ee252f8a773ee9db46 Mon Sep 17 00:00:00 2001 From: LFDM <1986gh@gmail.com> Date: Sat, 17 Mar 2018 22:24:26 +0100 Subject: [PATCH 05/15] feat(with-data) Build a more comprehensive own Spy component --- src/hocs/withData/index.spec.js | 91 +++++++++++++++++++++++++-------- 1 file changed, 70 insertions(+), 21 deletions(-) diff --git a/src/hocs/withData/index.spec.js b/src/hocs/withData/index.spec.js index a8190a4..483250e 100644 --- a/src/hocs/withData/index.spec.js +++ b/src/hocs/withData/index.spec.js @@ -69,8 +69,57 @@ const createConfig = () => { }; }; +class Logger { + constructor() { + this.logs = []; + } + + log(l) { + this.logs.push({ ...l, t: Date.now() }); + } + + getByType(t) { + return this.logs.filter(l => l.type === t); + } + + getRenders() { + return this.getByType('render'); + } + + getCallSequence() { + return this.logs.map(l => l.type); + } + + expectRenderCount(count) { + expect(this.getRenders().length).to.equal(count); + } +} + const createSpyComponent = () => { - return sinon.stub().returns(null); + const logger = new Logger(); + const log = (type, props) => logger.log({ type, props }); + + class SpyComponent extends Component { + componentWillMount() { + log('componentWillMount', this.props); + } + + componentWillUnmount() { + log('componentWillUnmount', this.props); + } + + // eslint-disable-next-line class-methods-use-this + componentWillReceiveProps(nextProps) { + log('componentWillReceiveProps', nextProps); + } + + render() { + log('render', this.props); + return null; + } + } + + return { spy: SpyComponent, logger }; }; class StateContainer extends Component { @@ -91,7 +140,7 @@ const render = (component, componentProps, ref, mapState = (t => t)) => { describe('withData', () => { it('passes the original properties down', () => { const api = build(createConfig()); - const spy = createSpyComponent(); + const { spy, logger } = createSpyComponent(); const comp = withData({ resolve: { users: () => api.user.getUsers(), @@ -102,15 +151,15 @@ describe('withData', () => { render(comp, { userId: 'peter' }); return delay().then(() => { - expect(spy).to.have.been.called; - const props = spy.args[0][0]; + logger.expectRenderCount(1); + const { props } = logger.getRenders()[0]; expect(props.userId).to.equal('peter'); }); }); it('does not re-render when no props to it have changed', () => { const api = build(createConfig()); - const spy = createSpyComponent(); + const { spy, logger } = createSpyComponent(); const comp = withData({ resolve: { users: () => api.user.getUsers(), @@ -123,13 +172,13 @@ describe('withData', () => { render(comp, { userId: 'peter' }, c => { stateContainer = c; }, ({ userId }) => ({ userId })); return delay().then(() => { - expect(spy).to.have.been.calledOnce; + logger.expectRenderCount(1); stateContainer.setState({ x: 'x' }); return delay().then(() => { - expect(spy).to.have.been.calledOnce; + logger.expectRenderCount(1); stateContainer.setState({ userId: 'gernot' }); return delay().then(() => { - expect(spy).to.have.been.calledThrice; + logger.expectRenderCount(3); }); }); }); @@ -137,7 +186,7 @@ describe('withData', () => { it('allows to observe changes', () => { const api = build(createConfig(), [observable()]); - const spy = createSpyComponent(); + const { spy, logger } = createSpyComponent(); const comp = withData({ observe: { user: ({ userId }) => api.user.getUser.createObservable(userId) @@ -147,14 +196,14 @@ describe('withData', () => { render(comp, { userId: 'peter' }); return delay().then(() => { - expect(spy).to.have.been.calledOnce; - const firstProps = spy.args[0][0]; + logger.expectRenderCount(1); + const { props: firstProps } = logger.getRenders()[0]; expect(firstProps.user).to.deep.equal(peter); return api.user.updateUser({ id: 'peter', name: 'crona' }).then((nextUser) => { return delay().then(() => { - expect(spy).to.have.been.calledTwice; - const secondProps = spy.args[1][0]; + logger.expectRenderCount(2); + const { props: secondProps } = logger.getRenders()[1]; expect(secondProps.user).to.deep.equal(nextUser); }); }); @@ -164,7 +213,7 @@ describe('withData', () => { describe('delay', () => { it('does not show pending state immediately when delay is requested', () => { const api = build(createConfig()); - const spy = createSpyComponent(); + const { spy } = createSpyComponent(); const pendingSpy = createSpyComponent(); const comp = withData({ resolve: { @@ -196,7 +245,7 @@ describe('withData', () => { describe('pagination', () => { it('allows to paginate with limit and offset (resolve)', () => { const api = build(createConfig(), [observable()]); - const spy = createSpyComponent(); + const { spy } = createSpyComponent(); const comp = withData({ resolve: { users: (props, { limit, offset }) => api.user.getUsersPaginated({ limit, offset }) @@ -225,7 +274,7 @@ describe('withData', () => { it('allows to paginate with limit and offset (observe)', () => { const api = build(createConfig(), [observable()]); - const spy = createSpyComponent(); + const { spy } = createSpyComponent(); const comp = withData({ observe: { users: (props, { limit, offset }) => api.user.getUsersPaginated.createObservable({ @@ -262,7 +311,7 @@ describe('withData', () => { it('allows to paginate with a cursor', () => { const api = build(createConfig(), [observable()]); - const spy = createSpyComponent(); + const { spy } = createSpyComponent(); const comp = withData({ resolve: { users: (props, cursor) => api.user.getUsersWithCursor({ cursor, limit: 2 }) @@ -306,7 +355,7 @@ describe('withData', () => { describe('poll', () => { it('does not poll when interval is set to a falsy value', () => { const api = build(createConfig()); - const spy = createSpyComponent(); + const { spy } = createSpyComponent(); const comp = withData({ poll: { users: { @@ -328,7 +377,7 @@ describe('withData', () => { it('does not poll when interval is not defined', () => { const api = build(createConfig()); - const spy = createSpyComponent(); + const { spy } = createSpyComponent(); const comp = withData({ poll: { users: { @@ -352,7 +401,7 @@ describe('withData', () => { // to make this really robust! const api = build(createConfig()); - const spy = createSpyComponent(); + const { spy } = createSpyComponent(); const comp = withData({ poll: { users: { @@ -378,7 +427,7 @@ describe('withData', () => { describe('shouldRefetch', () => { it('does not trigger callbacks when returning false for new props', () => { - const spy = createSpyComponent(); + const { spy } = createSpyComponent(); const spyResolve = sinon.stub().returns(Promise.resolve({})); const comp = withData({ From 0e6ff502694fc043cfebb86777e5e577a626a9fd Mon Sep 17 00:00:00 2001 From: LFDM <1986gh@gmail.com> Date: Sat, 17 Mar 2018 22:32:25 +0100 Subject: [PATCH 06/15] fix(with-data) Update all specs to new system --- src/hocs/withData/index.spec.js | 76 ++++++++++++++++----------------- 1 file changed, 38 insertions(+), 38 deletions(-) diff --git a/src/hocs/withData/index.spec.js b/src/hocs/withData/index.spec.js index 483250e..e3acd82 100644 --- a/src/hocs/withData/index.spec.js +++ b/src/hocs/withData/index.spec.js @@ -213,8 +213,8 @@ describe('withData', () => { describe('delay', () => { it('does not show pending state immediately when delay is requested', () => { const api = build(createConfig()); - const { spy } = createSpyComponent(); - const pendingSpy = createSpyComponent(); + const { spy, logger } = createSpyComponent(); + const { spy: pendingSpy, logger: pendingLogger } = createSpyComponent(); const comp = withData({ resolve: { user: ({ userId }) => api.user.getUser(userId) @@ -231,12 +231,12 @@ describe('withData', () => { render(comp, { userId: 'peter' }, c => { stateContainer = c; }, ({ userId }) => ({ userId })); return delay().then(() => { - expect(spy).to.have.been.calledOnce; - expect(pendingSpy).to.have.been.calledOne; + pendingLogger.expectRenderCount(1); + logger.expectRenderCount(1); stateContainer.setState({ userId: 'gernot' }); return delay().then(() => { - expect(pendingSpy).to.have.been.calledOnce; - expect(spy).to.have.been.calledThrice; + pendingLogger.expectRenderCount(1); + logger.expectRenderCount(3); }); }); }); @@ -245,7 +245,7 @@ describe('withData', () => { describe('pagination', () => { it('allows to paginate with limit and offset (resolve)', () => { const api = build(createConfig(), [observable()]); - const { spy } = createSpyComponent(); + const { spy, logger } = createSpyComponent(); const comp = withData({ resolve: { users: (props, { limit, offset }) => api.user.getUsersPaginated({ limit, offset }) @@ -258,14 +258,14 @@ describe('withData', () => { render(comp, {}); return delay().then(() => { - expect(spy).to.have.been.called; - const firstProps = spy.args[0][0]; + logger.expectRenderCount(1); + const { props: firstProps } = logger.getRenders()[0]; expect(firstProps.users).to.deep.equal([peter, gernot]); return firstProps.paginate.users.getNext().then(() => { return delay().then(() => { - expect(spy).to.have.been.calledTwice; - const secondProps = spy.args[1][0]; + logger.expectRenderCount(2); + const { props: secondProps } = logger.getRenders()[1]; expect(secondProps.users).to.deep.equal([peter, gernot, robin]); }); }); @@ -274,7 +274,7 @@ describe('withData', () => { it('allows to paginate with limit and offset (observe)', () => { const api = build(createConfig(), [observable()]); - const { spy } = createSpyComponent(); + const { spy, logger } = createSpyComponent(); const comp = withData({ observe: { users: (props, { limit, offset }) => api.user.getUsersPaginated.createObservable({ @@ -290,18 +290,18 @@ describe('withData', () => { render(comp, {}); return delay().then(() => { - expect(spy).to.have.been.called; - const firstProps = spy.args[0][0]; + logger.expectRenderCount(1); + const { props: firstProps } = logger.getRenders()[0]; expect(firstProps.users).to.deep.equal([peter, gernot]); return firstProps.paginate.users.getNext().then(() => { - expect(spy).to.have.been.calledTwice; - const secondProps = spy.args[1][0]; + logger.expectRenderCount(2); + const { props: secondProps } = logger.getRenders()[1]; expect(secondProps.users).to.deep.equal([peter, gernot, robin]); return api.user.updateUser({ id: 'peter', name: 'crona' }).then((nextUser) => { return delay().then(() => { - const thirdProps = spy.args[2][0]; + const { props: thirdProps } = logger.getRenders()[2]; expect(thirdProps.users[0]).to.deep.equal(nextUser); }); }); @@ -311,7 +311,7 @@ describe('withData', () => { it('allows to paginate with a cursor', () => { const api = build(createConfig(), [observable()]); - const { spy } = createSpyComponent(); + const { spy, logger } = createSpyComponent(); const comp = withData({ resolve: { users: (props, cursor) => api.user.getUsersWithCursor({ cursor, limit: 2 }) @@ -327,22 +327,22 @@ describe('withData', () => { render(comp, {}); return delay().then(() => { - expect(spy).to.have.been.called; - const firstProps = spy.args[0][0]; + logger.expectRenderCount(1); + const { props: firstProps } = logger.getRenders()[0]; expect(firstProps.users.length).to.equal(2); expect(firstProps.users).to.contain(peter); expect(firstProps.users).to.contain(gernot); return firstProps.paginate.users.getNext().then(() => { return delay().then(() => { - expect(spy).to.have.been.calledTwice; - const secondProps = spy.args[1][0]; + logger.expectRenderCount(2); + const { props: secondProps } = logger.getRenders()[1]; expect(secondProps.users).to.deep.equal([peter, gernot, robin, paulo]); expect(secondProps.paginate.users.hasNext).to.be.true; return secondProps.paginate.users.getNext().then(() => { - expect(spy).to.have.been.calledThrice; - const thirdProps = spy.args[2][0]; + logger.expectRenderCount(3); + const { props: thirdProps } = logger.getRenders()[2]; expect(thirdProps.users).to.deep.equal([peter, gernot, robin, paulo, timur]); expect(thirdProps.paginate.users.hasNext).to.be.false; }); @@ -355,7 +355,7 @@ describe('withData', () => { describe('poll', () => { it('does not poll when interval is set to a falsy value', () => { const api = build(createConfig()); - const { spy } = createSpyComponent(); + const { spy, logger } = createSpyComponent(); const comp = withData({ poll: { users: { @@ -368,16 +368,16 @@ describe('withData', () => { render(comp, {}); return delay().then(() => { - expect(spy).to.have.been.calledOnce; + logger.expectRenderCount(1); return delay(10).then(() => { - expect(spy).to.have.been.calledOnce; + logger.expectRenderCount(1); }); }); }); it('does not poll when interval is not defined', () => { const api = build(createConfig()); - const { spy } = createSpyComponent(); + const { spy, logger } = createSpyComponent(); const comp = withData({ poll: { users: { @@ -389,9 +389,9 @@ describe('withData', () => { render(comp, {}); return delay().then(() => { - expect(spy).to.have.been.calledOnce; + logger.expectRenderCount(1); return delay(10).then(() => { - expect(spy).to.have.been.calledOnce; + logger.expectRenderCount(1); }); }); }); @@ -401,7 +401,7 @@ describe('withData', () => { // to make this really robust! const api = build(createConfig()); - const { spy } = createSpyComponent(); + const { spy, logger } = createSpyComponent(); const comp = withData({ poll: { users: { @@ -414,11 +414,11 @@ describe('withData', () => { render(comp, {}); return delay().then(() => { - expect(spy).to.have.been.calledOnce; + logger.expectRenderCount(1); return delay(10).then(() => { - expect(spy).to.have.been.calledTwice; + logger.expectRenderCount(2); return delay(10).then(() => { - expect(spy).to.have.been.calledThrice; + logger.expectRenderCount(3); }); }); }); @@ -427,7 +427,7 @@ describe('withData', () => { describe('shouldRefetch', () => { it('does not trigger callbacks when returning false for new props', () => { - const { spy } = createSpyComponent(); + const { spy, logger } = createSpyComponent(); const spyResolve = sinon.stub().returns(Promise.resolve({})); const comp = withData({ @@ -444,18 +444,18 @@ describe('withData', () => { render(comp, { userId: 'peter' }, c => { stateContainer = c; }); return delay().then(() => { - expect(spy).to.have.been.calledOnce; + logger.expectRenderCount(1); expect(spyResolve).to.have.been.calledOnce; stateContainer.setState({ userId: 'robin' }); return delay().then(() => { expect(spyResolve).to.have.been.calledOnce; - expect(spy).to.have.been.calledTwice; + logger.expectRenderCount(2); stateContainer.setState({ userId: 'gernot' }); return delay().then(() => { expect(spyResolve).to.have.been.calledTwice; - expect(spy).to.have.been.calledThrice; + logger.expectRenderCount(3); }); }); }); From b4dff0a82bce9dd9689aac2df1ec601fbac1e0fc Mon Sep 17 00:00:00 2001 From: LFDM <1986gh@gmail.com> Date: Sun, 18 Mar 2018 07:57:29 +0100 Subject: [PATCH 07/15] feat(with-data) Also show component generation in spec --- src/hocs/withData/index.spec.js | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/hocs/withData/index.spec.js b/src/hocs/withData/index.spec.js index e3acd82..6b5aa0d 100644 --- a/src/hocs/withData/index.spec.js +++ b/src/hocs/withData/index.spec.js @@ -97,24 +97,31 @@ class Logger { const createSpyComponent = () => { const logger = new Logger(); - const log = (type, props) => logger.log({ type, props }); + const log = (type, props, generation) => logger.log({ type, props, generation }); + + let generation = 0; class SpyComponent extends Component { + constructor() { + super(); + this.generation = generation++; + } + componentWillMount() { - log('componentWillMount', this.props); + log('componentWillMount', this.props, this.generation); } componentWillUnmount() { - log('componentWillUnmount', this.props); + log('componentWillUnmount', this.props, this.generation); } // eslint-disable-next-line class-methods-use-this componentWillReceiveProps(nextProps) { - log('componentWillReceiveProps', nextProps); + log('componentWillReceiveProps', nextProps, this.generation); } render() { - log('render', this.props); + log('render', this.props, this.generation); return null; } } @@ -157,7 +164,7 @@ describe('withData', () => { }); }); - it('does not re-render when no props to it have changed', () => { + fit('does not re-render when no props to it have changed', () => { const api = build(createConfig()); const { spy, logger } = createSpyComponent(); const comp = withData({ From a5770d35a3298694ed171eeccbbafc7eba5738c7 Mon Sep 17 00:00:00 2001 From: LFDM <1986gh@gmail.com> Date: Sun, 18 Mar 2018 08:11:35 +0100 Subject: [PATCH 08/15] feat(refactor) Prevent excessive re-rendering when multiple resolves are present --- src/hocs/withData/index.js | 5 ++++- src/hocs/withData/index.spec.js | 5 ++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/hocs/withData/index.js b/src/hocs/withData/index.js index 27b662d..63fd468 100644 --- a/src/hocs/withData/index.js +++ b/src/hocs/withData/index.js @@ -150,7 +150,10 @@ class Container extends Component { } trigger(delays) { - const update = () => this.safeSetState({ pending: true, error: null }); + const update = () => { + this.resolvedData = {}; + this.safeSetState({ pending: true, error: null }); + }; if (delays.refetch) { const { timeouts } = this; timeouts.pendingScheduled = setTimeout(() => { diff --git a/src/hocs/withData/index.spec.js b/src/hocs/withData/index.spec.js index 6b5aa0d..fd0811d 100644 --- a/src/hocs/withData/index.spec.js +++ b/src/hocs/withData/index.spec.js @@ -164,7 +164,7 @@ describe('withData', () => { }); }); - fit('does not re-render when no props to it have changed', () => { + it('does not re-render when no props to it have changed', () => { const api = build(createConfig()); const { spy, logger } = createSpyComponent(); const comp = withData({ @@ -185,7 +185,7 @@ describe('withData', () => { logger.expectRenderCount(1); stateContainer.setState({ userId: 'gernot' }); return delay().then(() => { - logger.expectRenderCount(3); + logger.expectRenderCount(2); }); }); }); @@ -234,7 +234,6 @@ describe('withData', () => { let stateContainer = null; - // prefill the cache render(comp, { userId: 'peter' }, c => { stateContainer = c; }, ({ userId }) => ({ userId })); return delay().then(() => { From b9125d70b61ffc101c702c009f1cb5da9805cebe Mon Sep 17 00:00:00 2001 From: LFDM <1986gh@gmail.com> Date: Sun, 18 Mar 2018 08:34:55 +0100 Subject: [PATCH 09/15] refactor(with-data) Remove version increments - were clever, but unnecessary --- src/hocs/withData/index.js | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/hocs/withData/index.js b/src/hocs/withData/index.js index 63fd468..7ef311b 100644 --- a/src/hocs/withData/index.js +++ b/src/hocs/withData/index.js @@ -6,8 +6,6 @@ import { PAGINATION } from './retrievers'; -const incrementVersion = version => (version > 99 ? 0 : version + 1); - class Container extends Component { constructor(props) { super(props); @@ -71,22 +69,22 @@ class Container extends Component { this.resolvedData[field] = data; if (this.resolvedDataTargetSize === Object.keys(this.resolvedData).length) { this.clearPendingTimeout(); - this.safeSetState(prevState => ({ + this.safeSetState({ pending: false, + pendingScheduled: false, resolvedProps: { ...this.resolvedData }, - error: null, - version: incrementVersion(prevState.version) - })); + error: null + }); } } setError(field, error) { this.clearPendingTimeout(); - this.safeSetState(prevState => ({ + this.safeSetState({ pending: false, - error, - version: incrementVersion(prevState.version) - })); + pendingScheduled: false, + error + }); } clearPendingTimeout() { @@ -152,7 +150,7 @@ class Container extends Component { trigger(delays) { const update = () => { this.resolvedData = {}; - this.safeSetState({ pending: true, error: null }); + this.safeSetState({ pending: true, pendingScheduled: false, error: null }); }; if (delays.refetch) { const { timeouts } = this; @@ -162,6 +160,7 @@ class Container extends Component { this.clearPendingTimeout(); } }, delays.refetch); + this.safeSetState({ pendingScheduled: true }); } else { update(); } From 4b76d0c23733b33276415b40d62c4268ca2e3a9f Mon Sep 17 00:00:00 2001 From: LFDM <1986gh@gmail.com> Date: Sun, 18 Mar 2018 08:40:01 +0100 Subject: [PATCH 10/15] refactor(with-data) Update prop access syntax --- src/hocs/withData/index.spec.js | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/src/hocs/withData/index.spec.js b/src/hocs/withData/index.spec.js index fd0811d..aa926c2 100644 --- a/src/hocs/withData/index.spec.js +++ b/src/hocs/withData/index.spec.js @@ -90,6 +90,10 @@ class Logger { return this.logs.map(l => l.type); } + getRenderProps(count) { + return this.getRenders()[count].props; + } + expectRenderCount(count) { expect(this.getRenders().length).to.equal(count); } @@ -159,7 +163,7 @@ describe('withData', () => { return delay().then(() => { logger.expectRenderCount(1); - const { props } = logger.getRenders()[0]; + const props = logger.getRenderProps(0); expect(props.userId).to.equal('peter'); }); }); @@ -204,13 +208,13 @@ describe('withData', () => { return delay().then(() => { logger.expectRenderCount(1); - const { props: firstProps } = logger.getRenders()[0]; + const firstProps = logger.getRenderProps(0); expect(firstProps.user).to.deep.equal(peter); return api.user.updateUser({ id: 'peter', name: 'crona' }).then((nextUser) => { return delay().then(() => { logger.expectRenderCount(2); - const { props: secondProps } = logger.getRenders()[1]; + const secondProps = logger.getRenderProps(1); expect(secondProps.user).to.deep.equal(nextUser); }); }); @@ -240,6 +244,7 @@ describe('withData', () => { pendingLogger.expectRenderCount(1); logger.expectRenderCount(1); stateContainer.setState({ userId: 'gernot' }); + logger.expectRenderCount(2); return delay().then(() => { pendingLogger.expectRenderCount(1); logger.expectRenderCount(3); @@ -265,13 +270,13 @@ describe('withData', () => { return delay().then(() => { logger.expectRenderCount(1); - const { props: firstProps } = logger.getRenders()[0]; + const firstProps = logger.getRenderProps(0); expect(firstProps.users).to.deep.equal([peter, gernot]); return firstProps.paginate.users.getNext().then(() => { return delay().then(() => { logger.expectRenderCount(2); - const { props: secondProps } = logger.getRenders()[1]; + const secondProps = logger.getRenderProps(1); expect(secondProps.users).to.deep.equal([peter, gernot, robin]); }); }); @@ -297,17 +302,18 @@ describe('withData', () => { return delay().then(() => { logger.expectRenderCount(1); - const { props: firstProps } = logger.getRenders()[0]; + const firstProps = logger.getRenderProps(0); expect(firstProps.users).to.deep.equal([peter, gernot]); return firstProps.paginate.users.getNext().then(() => { logger.expectRenderCount(2); - const { props: secondProps } = logger.getRenders()[1]; + const secondProps = logger.getRenderProps(1); expect(secondProps.users).to.deep.equal([peter, gernot, robin]); return api.user.updateUser({ id: 'peter', name: 'crona' }).then((nextUser) => { return delay().then(() => { - const { props: thirdProps } = logger.getRenders()[2]; + logger.expectRenderCount(4); + const thirdProps = logger.getRenderProps(2); expect(thirdProps.users[0]).to.deep.equal(nextUser); }); }); @@ -334,7 +340,7 @@ describe('withData', () => { return delay().then(() => { logger.expectRenderCount(1); - const { props: firstProps } = logger.getRenders()[0]; + const firstProps = logger.getRenderProps(0); expect(firstProps.users.length).to.equal(2); expect(firstProps.users).to.contain(peter); expect(firstProps.users).to.contain(gernot); @@ -342,13 +348,13 @@ describe('withData', () => { return firstProps.paginate.users.getNext().then(() => { return delay().then(() => { logger.expectRenderCount(2); - const { props: secondProps } = logger.getRenders()[1]; + const secondProps = logger.getRenderProps(1); expect(secondProps.users).to.deep.equal([peter, gernot, robin, paulo]); expect(secondProps.paginate.users.hasNext).to.be.true; return secondProps.paginate.users.getNext().then(() => { logger.expectRenderCount(3); - const { props: thirdProps } = logger.getRenders()[2]; + const thirdProps = logger.getRenderProps(2); expect(thirdProps.users).to.deep.equal([peter, gernot, robin, paulo, timur]); expect(thirdProps.paginate.users.hasNext).to.be.false; }); From 5c264078990c5347952f79c1bc1ae05111628e07 Mon Sep 17 00:00:00 2001 From: LFDM <1986gh@gmail.com> Date: Sun, 18 Mar 2018 09:12:42 +0100 Subject: [PATCH 11/15] feat(with-data) Aggressively avoid re-rendering --- src/hocs/withData/index.js | 7 +++++++ src/hocs/withData/index.spec.js | 4 ++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/hocs/withData/index.js b/src/hocs/withData/index.js index 7ef311b..23ccc64 100644 --- a/src/hocs/withData/index.js +++ b/src/hocs/withData/index.js @@ -59,6 +59,10 @@ class Container extends Component { this.destroy(); } + shouldComponentUpdate() { + return this.rerender; + } + safeSetState(...args) { if (!this.isUnmounting) { this.setState(...args); @@ -69,6 +73,7 @@ class Container extends Component { this.resolvedData[field] = data; if (this.resolvedDataTargetSize === Object.keys(this.resolvedData).length) { this.clearPendingTimeout(); + this.rerender = true; this.safeSetState({ pending: false, pendingScheduled: false, @@ -148,7 +153,9 @@ class Container extends Component { } trigger(delays) { + this.rerender = false; const update = () => { + this.rerender = true; this.resolvedData = {}; this.safeSetState({ pending: true, pendingScheduled: false, error: null }); }; diff --git a/src/hocs/withData/index.spec.js b/src/hocs/withData/index.spec.js index aa926c2..c420113 100644 --- a/src/hocs/withData/index.spec.js +++ b/src/hocs/withData/index.spec.js @@ -244,10 +244,10 @@ describe('withData', () => { pendingLogger.expectRenderCount(1); logger.expectRenderCount(1); stateContainer.setState({ userId: 'gernot' }); - logger.expectRenderCount(2); + logger.expectRenderCount(1); return delay().then(() => { pendingLogger.expectRenderCount(1); - logger.expectRenderCount(3); + logger.expectRenderCount(2); }); }); }); From 309b223abbc3fb55af0fe65455ca06e8c88898d8 Mon Sep 17 00:00:00 2001 From: LFDM <1986gh@gmail.com> Date: Sun, 18 Mar 2018 09:37:07 +0100 Subject: [PATCH 12/15] feat(with-data) Add future TODO --- src/hocs/withData/index.spec.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/hocs/withData/index.spec.js b/src/hocs/withData/index.spec.js index c420113..65aaf35 100644 --- a/src/hocs/withData/index.spec.js +++ b/src/hocs/withData/index.spec.js @@ -312,6 +312,7 @@ describe('withData', () => { return api.user.updateUser({ id: 'peter', name: 'crona' }).then((nextUser) => { return delay().then(() => { + // TODO: Why is there another render call here? logger.expectRenderCount(4); const thirdProps = logger.getRenderProps(2); expect(thirdProps.users[0]).to.deep.equal(nextUser); From dd4739bbf94dcb6e51651e178f0e0ec55615d468 Mon Sep 17 00:00:00 2001 From: LFDM <1986gh@gmail.com> Date: Sun, 18 Mar 2018 09:37:55 +0100 Subject: [PATCH 13/15] refactor(with-data) Rename delay to wait in specs --- src/hocs/withData/index.spec.js | 50 ++++++++++++++++----------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/src/hocs/withData/index.spec.js b/src/hocs/withData/index.spec.js index 65aaf35..78708c2 100644 --- a/src/hocs/withData/index.spec.js +++ b/src/hocs/withData/index.spec.js @@ -7,7 +7,7 @@ import sinon from 'sinon'; import { withData } from '.'; -const delay = (t = 1) => new Promise(res => setTimeout(() => res(), t)); +const wait = (t = 1) => new Promise(res => setTimeout(() => res(), t)); const peter = { id: 'peter', name: 'peter' }; const gernot = { id: 'gernot', name: 'gernot' }; @@ -161,7 +161,7 @@ describe('withData', () => { render(comp, { userId: 'peter' }); - return delay().then(() => { + return wait().then(() => { logger.expectRenderCount(1); const props = logger.getRenderProps(0); expect(props.userId).to.equal('peter'); @@ -182,13 +182,13 @@ describe('withData', () => { render(comp, { userId: 'peter' }, c => { stateContainer = c; }, ({ userId }) => ({ userId })); - return delay().then(() => { + return wait().then(() => { logger.expectRenderCount(1); stateContainer.setState({ x: 'x' }); - return delay().then(() => { + return wait().then(() => { logger.expectRenderCount(1); stateContainer.setState({ userId: 'gernot' }); - return delay().then(() => { + return wait().then(() => { logger.expectRenderCount(2); }); }); @@ -206,13 +206,13 @@ describe('withData', () => { render(comp, { userId: 'peter' }); - return delay().then(() => { + return wait().then(() => { logger.expectRenderCount(1); const firstProps = logger.getRenderProps(0); expect(firstProps.user).to.deep.equal(peter); return api.user.updateUser({ id: 'peter', name: 'crona' }).then((nextUser) => { - return delay().then(() => { + return wait().then(() => { logger.expectRenderCount(2); const secondProps = logger.getRenderProps(1); expect(secondProps.user).to.deep.equal(nextUser); @@ -240,12 +240,12 @@ describe('withData', () => { render(comp, { userId: 'peter' }, c => { stateContainer = c; }, ({ userId }) => ({ userId })); - return delay().then(() => { + return wait().then(() => { pendingLogger.expectRenderCount(1); logger.expectRenderCount(1); stateContainer.setState({ userId: 'gernot' }); logger.expectRenderCount(1); - return delay().then(() => { + return wait().then(() => { pendingLogger.expectRenderCount(1); logger.expectRenderCount(2); }); @@ -268,13 +268,13 @@ describe('withData', () => { render(comp, {}); - return delay().then(() => { + return wait().then(() => { logger.expectRenderCount(1); const firstProps = logger.getRenderProps(0); expect(firstProps.users).to.deep.equal([peter, gernot]); return firstProps.paginate.users.getNext().then(() => { - return delay().then(() => { + return wait().then(() => { logger.expectRenderCount(2); const secondProps = logger.getRenderProps(1); expect(secondProps.users).to.deep.equal([peter, gernot, robin]); @@ -300,7 +300,7 @@ describe('withData', () => { render(comp, {}); - return delay().then(() => { + return wait().then(() => { logger.expectRenderCount(1); const firstProps = logger.getRenderProps(0); expect(firstProps.users).to.deep.equal([peter, gernot]); @@ -311,7 +311,7 @@ describe('withData', () => { expect(secondProps.users).to.deep.equal([peter, gernot, robin]); return api.user.updateUser({ id: 'peter', name: 'crona' }).then((nextUser) => { - return delay().then(() => { + return wait().then(() => { // TODO: Why is there another render call here? logger.expectRenderCount(4); const thirdProps = logger.getRenderProps(2); @@ -339,7 +339,7 @@ describe('withData', () => { render(comp, {}); - return delay().then(() => { + return wait().then(() => { logger.expectRenderCount(1); const firstProps = logger.getRenderProps(0); expect(firstProps.users.length).to.equal(2); @@ -347,7 +347,7 @@ describe('withData', () => { expect(firstProps.users).to.contain(gernot); return firstProps.paginate.users.getNext().then(() => { - return delay().then(() => { + return wait().then(() => { logger.expectRenderCount(2); const secondProps = logger.getRenderProps(1); expect(secondProps.users).to.deep.equal([peter, gernot, robin, paulo]); @@ -380,9 +380,9 @@ describe('withData', () => { render(comp, {}); - return delay().then(() => { + return wait().then(() => { logger.expectRenderCount(1); - return delay(10).then(() => { + return wait(10).then(() => { logger.expectRenderCount(1); }); }); @@ -401,9 +401,9 @@ describe('withData', () => { render(comp, {}); - return delay().then(() => { + return wait().then(() => { logger.expectRenderCount(1); - return delay(10).then(() => { + return wait(10).then(() => { logger.expectRenderCount(1); }); }); @@ -426,11 +426,11 @@ describe('withData', () => { render(comp, {}); - return delay().then(() => { + return wait().then(() => { logger.expectRenderCount(1); - return delay(10).then(() => { + return wait(10).then(() => { logger.expectRenderCount(2); - return delay(10).then(() => { + return wait(10).then(() => { logger.expectRenderCount(3); }); }); @@ -456,17 +456,17 @@ describe('withData', () => { render(comp, { userId: 'peter' }, c => { stateContainer = c; }); - return delay().then(() => { + return wait().then(() => { logger.expectRenderCount(1); expect(spyResolve).to.have.been.calledOnce; stateContainer.setState({ userId: 'robin' }); - return delay().then(() => { + return wait().then(() => { expect(spyResolve).to.have.been.calledOnce; logger.expectRenderCount(2); stateContainer.setState({ userId: 'gernot' }); - return delay().then(() => { + return wait().then(() => { expect(spyResolve).to.have.been.calledTwice; logger.expectRenderCount(3); }); From f5671af9e6b714b2da8213985ca924453464cf39 Mon Sep 17 00:00:00 2001 From: LFDM <1986gh@gmail.com> Date: Sun, 18 Mar 2018 09:48:21 +0100 Subject: [PATCH 14/15] feat(with-data) Add more specs to prove mount and unmount behavior --- src/hocs/withData/index.spec.js | 78 ++++++++++++++++++++++++++++++++- 1 file changed, 77 insertions(+), 1 deletion(-) diff --git a/src/hocs/withData/index.spec.js b/src/hocs/withData/index.spec.js index 78708c2..d6e2c5a 100644 --- a/src/hocs/withData/index.spec.js +++ b/src/hocs/withData/index.spec.js @@ -95,7 +95,15 @@ class Logger { } expectRenderCount(count) { - expect(this.getRenders().length).to.equal(count); + this.expectCountByType('render', count); + } + + expectCountByType(type, count) { + expect(this.getByType(type).length).to.equal(count); + } + + expectCallSequence(sequence) { + expect(this.getCallSequence()).to.deep.equal(sequence); } } @@ -222,6 +230,45 @@ describe('withData', () => { }); describe('delay', () => { + it('unmounts and remounts view component when no delay is specified on refecth', () => { + const api = build(createConfig()); + const { spy, logger } = createSpyComponent(); + const { spy: pendingSpy, logger: pendingLogger } = createSpyComponent(); + const comp = withData({ + resolve: { + user: ({ userId }) => api.user.getUser(userId) + }, + pendingComponent: pendingSpy, + delays: { + refetch: 0 + } + })(spy); + + let stateContainer = null; + + render(comp, { userId: 'peter' }, c => { stateContainer = c; }, ({ userId }) => ({ userId })); + + return wait().then(() => { + pendingLogger.expectRenderCount(1); + logger.expectRenderCount(1); + stateContainer.setState({ userId: 'gernot' }); + logger.expectRenderCount(1); + logger.expectCountByType('componentWillUnmount', 1); + return wait().then(() => { + logger.expectCountByType('componentWillMount', 2); + pendingLogger.expectRenderCount(2); + logger.expectRenderCount(2); + logger.expectCallSequence([ + 'componentWillMount', + 'render', + 'componentWillUnmount', + 'componentWillMount', + 'render' + ]); + }); + }); + }); + it('does not show pending state immediately when delay is requested', () => { const api = build(createConfig()); const { spy, logger } = createSpyComponent(); @@ -251,6 +298,35 @@ describe('withData', () => { }); }); }); + + it('does not needlessly unmount immediately when delay is requested', () => { + const api = build(createConfig()); + const { spy, logger } = createSpyComponent(); + const comp = withData({ + resolve: { + user: ({ userId }) => api.user.getUser(userId) + }, + delays: { + refetch: 100 + } + })(spy); + + let stateContainer = null; + + render(comp, { userId: 'peter' }, c => { stateContainer = c; }, ({ userId }) => ({ userId })); + + return wait().then(() => { + stateContainer.setState({ userId: 'gernot' }); + return wait().then(() => { + logger.expectCallSequence([ + 'componentWillMount', + 'render', + 'componentWillReceiveProps', + 'render' + ]); + }); + }); + }); }); describe('pagination', () => { From 3edd3ce457b1bcc10bc55a8b8b92fe0911bcbc1b Mon Sep 17 00:00:00 2001 From: LFDM <1986gh@gmail.com> Date: Sun, 18 Mar 2018 09:54:38 +0100 Subject: [PATCH 15/15] refactor(with-data) Remove remnants of version field --- src/hocs/withData/index.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/hocs/withData/index.js b/src/hocs/withData/index.js index 23ccc64..8078d5b 100644 --- a/src/hocs/withData/index.js +++ b/src/hocs/withData/index.js @@ -27,8 +27,7 @@ class Container extends Component { this.state = { pending: false, error: null, - resolvedProps: null, - version: 0 + resolvedProps: null }; }