From 70ebd3a77aa30198597c71967eebc19dfbc0c0b8 Mon Sep 17 00:00:00 2001 From: Joe Torreggiani Date: Wed, 27 May 2020 15:42:52 -0400 Subject: [PATCH 01/10] Implement basic lazy load --- dist/mobx-async-store.cjs.js | 39 +++++++++++++++++- dist/mobx-async-store.esm.js | 39 +++++++++++++++++- spec/Store.spec.js | 80 ++++++++++++++++++++++++++++++++++++ src/Store.js | 24 ++++++++++- 4 files changed, 179 insertions(+), 3 deletions(-) diff --git a/dist/mobx-async-store.cjs.js b/dist/mobx-async-store.cjs.js index 9a52ce63..c3946e08 100644 --- a/dist/mobx-async-store.cjs.js +++ b/dist/mobx-async-store.cjs.js @@ -1410,7 +1410,16 @@ function () { this.findAll = function (type) { var options = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : {}; var fromServer = options.fromServer, - queryParams = options.queryParams; + queryParams = options.queryParams, + lazyLoad = options.lazyLoad; + + if (lazyLoad) { + var lazyLoadOption = _objectSpread$2({}, options, { + lazyLoad: false + }); + + return _this.lazyLoad(type, lazyLoadOption); + } if (fromServer === true) { // If fromServer is true always fetch the data and return @@ -1423,6 +1432,34 @@ function () { } }; + this.lazyLoad = function (type) { + var options = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : {}; + + var records = _this.findAll(type, _objectSpread$2({}, options, { + fromServer: false + })); + + var beforeRefetch = options.beforeRefetch, + afterRefetch = options.afterRefetch; + + if (records.length > 0) { + beforeRefetch && beforeRefetch(records); + + _this.findAll(type, _objectSpread$2({}, options, { + fromServer: true + })).then(function (result) { + // console.log('yea', result) + afterRefetch && afterRefetch(result); + }); + + return records; + } else { + return _this.findAll(type, _objectSpread$2({}, options, { + fromServer: true + })); + } + }; + this.findOrFetchAll = function (type, queryParams) { // Get any matching records var records = _this.getMatchingRecords(type, queryParams); // If any records are present diff --git a/dist/mobx-async-store.esm.js b/dist/mobx-async-store.esm.js index 50c4f682..a95c0174 100644 --- a/dist/mobx-async-store.esm.js +++ b/dist/mobx-async-store.esm.js @@ -1404,7 +1404,16 @@ function () { this.findAll = function (type) { var options = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : {}; var fromServer = options.fromServer, - queryParams = options.queryParams; + queryParams = options.queryParams, + lazyLoad = options.lazyLoad; + + if (lazyLoad) { + var lazyLoadOption = _objectSpread$2({}, options, { + lazyLoad: false + }); + + return _this.lazyLoad(type, lazyLoadOption); + } if (fromServer === true) { // If fromServer is true always fetch the data and return @@ -1417,6 +1426,34 @@ function () { } }; + this.lazyLoad = function (type) { + var options = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : {}; + + var records = _this.findAll(type, _objectSpread$2({}, options, { + fromServer: false + })); + + var beforeRefetch = options.beforeRefetch, + afterRefetch = options.afterRefetch; + + if (records.length > 0) { + beforeRefetch && beforeRefetch(records); + + _this.findAll(type, _objectSpread$2({}, options, { + fromServer: true + })).then(function (result) { + // console.log('yea', result) + afterRefetch && afterRefetch(result); + }); + + return records; + } else { + return _this.findAll(type, _objectSpread$2({}, options, { + fromServer: true + })); + } + }; + this.findOrFetchAll = function (type, queryParams) { // Get any matching records var records = _this.getMatchingRecords(type, queryParams); // If any records are present diff --git a/spec/Store.spec.js b/spec/Store.spec.js index bf1429a6..a79c774f 100644 --- a/spec/Store.spec.js +++ b/spec/Store.spec.js @@ -532,4 +532,84 @@ describe('Store', () => { expect(typeof todos[1]).toBe('undefined') }) }) + + describe('lazy loading', () => { + let requestOptions + let lazyLoadOptions + let mockAfterRefetch = jest.fn() + let mockBeforeRefetch = jest.fn() + + beforeEach(() => { + jest.resetAllMocks() + + requestOptions = { + queryParams: { + filter: { + title: 'Do taxes' + } + } + } + + lazyLoadOptions = { + ...requestOptions, + lazyLoad: true, + afterRefetch: mockAfterRefetch, + beforeRefetch: mockBeforeRefetch + } + }) + + it('triggers a fetch if no cached data is found', async () => { + fetch.mockResponse(mockTodosResponse) + + const result = await store.findAll('todos', lazyLoadOptions) + + expect(result).toHaveLength(1) + }) + + it('returns cached data before refetching', async () => { + fetch.mockResponse(mockTodosResponse) + + await store.findAll('todos', requestOptions) + const result = store.findAll('todos', lazyLoadOptions) + + expect(result).toHaveLength(1) + expect(fetch.mock.calls).toHaveLength(2) + }) + + it('calls beforeRefetch callback with prefetch result', async () => { + fetch.mockResponse(mockTodosResponse) + await store.findAll('todos', requestOptions) + + const result = store.findAll('todos', lazyLoadOptions) + + expect(result).toHaveLength(1) + expect(mockBeforeRefetch).toHaveBeenCalledWith(result) + }) + + it('calls afterRefetch callback with refetch result', async (done) => { + const mockTodosResponse2 = JSON.stringify({ + data: [ + mockTodoData.data, + { ...mockTodoData.data, id: 2, title: 'Test' } + ] + }) + + fetch.mockResponses( + [mockTodosResponse, { status: 200 }], + [mockTodosResponse2, { status: 200 }] + ) + + // Trigger another request + await store.findAll('todos', requestOptions) + + const result = await store.findAll('todos', lazyLoadOptions) + + expect(result).toHaveLength(1) + + setImmediate(() => { + expect(mockAfterRefetch.mock.calls[0][0]).toHaveLength(2) + done() + }) + }) + }) }) diff --git a/src/Store.js b/src/Store.js index 6cf2184e..290eaa38 100644 --- a/src/Store.js +++ b/src/Store.js @@ -216,7 +216,12 @@ class Store { * @param {Object} options */ findAll = (type, options = {}) => { - const { fromServer, queryParams } = options + const { fromServer, queryParams, lazyLoad } = options + + if (lazyLoad) { + const lazyLoadOption = { ...options, lazyLoad: false } + return this.lazyLoad(type, lazyLoadOption) + } if (fromServer === true) { // If fromServer is true always fetch the data and return @@ -229,6 +234,23 @@ class Store { } } + lazyLoad = (type, options = {}) => { + let records = this.findAll(type, { ...options, fromServer: false }) + + const { beforeRefetch, afterRefetch } = options + + if (records.length > 0) { + beforeRefetch && beforeRefetch(records) + this.findAll(type, { ...options, fromServer: true }).then((result) => { + // console.log('yea', result) + afterRefetch && afterRefetch(result) + }) + return records + } else { + return this.findAll(type, { ...options, fromServer: true }) + } + } + /** * returns cache if exists, returns promise if not * From 9e40d7270e09a06e1d8e517494a8cfa00dd766b1 Mon Sep 17 00:00:00 2001 From: Joe Torreggiani Date: Thu, 28 May 2020 10:10:21 -0400 Subject: [PATCH 02/10] Add documentation and clean up tests --- spec/Store.spec.js | 2 +- src/Store.js | 10 +++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/spec/Store.spec.js b/spec/Store.spec.js index a79c774f..3d91f275 100644 --- a/spec/Store.spec.js +++ b/spec/Store.spec.js @@ -602,7 +602,7 @@ describe('Store', () => { // Trigger another request await store.findAll('todos', requestOptions) - const result = await store.findAll('todos', lazyLoadOptions) + const result = store.findAll('todos', lazyLoadOptions) expect(result).toHaveLength(1) diff --git a/src/Store.js b/src/Store.js index 290eaa38..7eb11e77 100644 --- a/src/Store.js +++ b/src/Store.js @@ -220,7 +220,7 @@ class Store { if (lazyLoad) { const lazyLoadOption = { ...options, lazyLoad: false } - return this.lazyLoad(type, lazyLoadOption) + return this._lazyLoad(type, lazyLoadOption) } if (fromServer === true) { @@ -234,7 +234,12 @@ class Store { } } - lazyLoad = (type, options = {}) => { + /** + * @method _lazyLoad + * @param {String} type the type to find + * @param {Object} options + */ + _lazyLoad = (type, options = {}) => { let records = this.findAll(type, { ...options, fromServer: false }) const { beforeRefetch, afterRefetch } = options @@ -242,7 +247,6 @@ class Store { if (records.length > 0) { beforeRefetch && beforeRefetch(records) this.findAll(type, { ...options, fromServer: true }).then((result) => { - // console.log('yea', result) afterRefetch && afterRefetch(result) }) return records From d7425992cf8e576912f4753314c1c69698b675ce Mon Sep 17 00:00:00 2001 From: Joe Torreggiani Date: Fri, 29 May 2020 09:58:30 -0400 Subject: [PATCH 03/10] Changes based on CR feedback --- dist/mobx-async-store.cjs.js | 14 ++------------ dist/mobx-async-store.esm.js | 14 ++------------ spec/Store.spec.js | 9 ++++----- src/Store.js | 12 ++++-------- 4 files changed, 12 insertions(+), 37 deletions(-) diff --git a/dist/mobx-async-store.cjs.js b/dist/mobx-async-store.cjs.js index c3946e08..bcd16676 100644 --- a/dist/mobx-async-store.cjs.js +++ b/dist/mobx-async-store.cjs.js @@ -1410,16 +1410,7 @@ function () { this.findAll = function (type) { var options = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : {}; var fromServer = options.fromServer, - queryParams = options.queryParams, - lazyLoad = options.lazyLoad; - - if (lazyLoad) { - var lazyLoadOption = _objectSpread$2({}, options, { - lazyLoad: false - }); - - return _this.lazyLoad(type, lazyLoadOption); - } + queryParams = options.queryParams; if (fromServer === true) { // If fromServer is true always fetch the data and return @@ -1432,7 +1423,7 @@ function () { } }; - this.lazyLoad = function (type) { + this.findAndFetchAll = function (type) { var options = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : {}; var records = _this.findAll(type, _objectSpread$2({}, options, { @@ -1448,7 +1439,6 @@ function () { _this.findAll(type, _objectSpread$2({}, options, { fromServer: true })).then(function (result) { - // console.log('yea', result) afterRefetch && afterRefetch(result); }); diff --git a/dist/mobx-async-store.esm.js b/dist/mobx-async-store.esm.js index a95c0174..e749c46f 100644 --- a/dist/mobx-async-store.esm.js +++ b/dist/mobx-async-store.esm.js @@ -1404,16 +1404,7 @@ function () { this.findAll = function (type) { var options = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : {}; var fromServer = options.fromServer, - queryParams = options.queryParams, - lazyLoad = options.lazyLoad; - - if (lazyLoad) { - var lazyLoadOption = _objectSpread$2({}, options, { - lazyLoad: false - }); - - return _this.lazyLoad(type, lazyLoadOption); - } + queryParams = options.queryParams; if (fromServer === true) { // If fromServer is true always fetch the data and return @@ -1426,7 +1417,7 @@ function () { } }; - this.lazyLoad = function (type) { + this.findAndFetchAll = function (type) { var options = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : {}; var records = _this.findAll(type, _objectSpread$2({}, options, { @@ -1442,7 +1433,6 @@ function () { _this.findAll(type, _objectSpread$2({}, options, { fromServer: true })).then(function (result) { - // console.log('yea', result) afterRefetch && afterRefetch(result); }); diff --git a/spec/Store.spec.js b/spec/Store.spec.js index 3d91f275..e7a28279 100644 --- a/spec/Store.spec.js +++ b/spec/Store.spec.js @@ -533,7 +533,7 @@ describe('Store', () => { }) }) - describe('lazy loading', () => { + describe('findAndFetchAll', () => { let requestOptions let lazyLoadOptions let mockAfterRefetch = jest.fn() @@ -552,7 +552,6 @@ describe('Store', () => { lazyLoadOptions = { ...requestOptions, - lazyLoad: true, afterRefetch: mockAfterRefetch, beforeRefetch: mockBeforeRefetch } @@ -570,7 +569,7 @@ describe('Store', () => { fetch.mockResponse(mockTodosResponse) await store.findAll('todos', requestOptions) - const result = store.findAll('todos', lazyLoadOptions) + const result = store.findAndFetchAll('todos', lazyLoadOptions) expect(result).toHaveLength(1) expect(fetch.mock.calls).toHaveLength(2) @@ -580,7 +579,7 @@ describe('Store', () => { fetch.mockResponse(mockTodosResponse) await store.findAll('todos', requestOptions) - const result = store.findAll('todos', lazyLoadOptions) + const result = store.findAndFetchAll('todos', lazyLoadOptions) expect(result).toHaveLength(1) expect(mockBeforeRefetch).toHaveBeenCalledWith(result) @@ -602,7 +601,7 @@ describe('Store', () => { // Trigger another request await store.findAll('todos', requestOptions) - const result = store.findAll('todos', lazyLoadOptions) + const result = store.findAndFetchAll('todos', lazyLoadOptions) expect(result).toHaveLength(1) diff --git a/src/Store.js b/src/Store.js index 7eb11e77..93893b74 100644 --- a/src/Store.js +++ b/src/Store.js @@ -216,12 +216,7 @@ class Store { * @param {Object} options */ findAll = (type, options = {}) => { - const { fromServer, queryParams, lazyLoad } = options - - if (lazyLoad) { - const lazyLoadOption = { ...options, lazyLoad: false } - return this._lazyLoad(type, lazyLoadOption) - } + const { fromServer, queryParams } = options if (fromServer === true) { // If fromServer is true always fetch the data and return @@ -235,11 +230,12 @@ class Store { } /** - * @method _lazyLoad + * @method findAndFetchAll * @param {String} type the type to find * @param {Object} options + * @return {Array} */ - _lazyLoad = (type, options = {}) => { + findAndFetchAll = (type, options = {}) => { let records = this.findAll(type, { ...options, fromServer: false }) const { beforeRefetch, afterRefetch } = options From 48a88aa67a7f2442c2cc24cbf73337e9022e3723 Mon Sep 17 00:00:00 2001 From: Joe Torreggiani Date: Mon, 1 Jun 2020 08:59:25 -0400 Subject: [PATCH 04/10] Fix spec --- spec/Store.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/Store.spec.js b/spec/Store.spec.js index e7a28279..36dce757 100644 --- a/spec/Store.spec.js +++ b/spec/Store.spec.js @@ -560,7 +560,7 @@ describe('Store', () => { it('triggers a fetch if no cached data is found', async () => { fetch.mockResponse(mockTodosResponse) - const result = await store.findAll('todos', lazyLoadOptions) + const result = await store.findAndFetchAll('todos', lazyLoadOptions) expect(result).toHaveLength(1) }) From f6dc8990e2a50dfe5d566be836383b8ce72f233c Mon Sep 17 00:00:00 2001 From: Joe Torreggiani Date: Mon, 1 Jun 2020 12:52:49 -0400 Subject: [PATCH 05/10] WIP --- dist/mobx-async-store.cjs.js | 47 ++++++++--------- dist/mobx-async-store.esm.js | 47 ++++++++--------- spec/Store.spec.js | 93 +++++++++++++++++++++++---------- src/Store.js | 26 +++++---- src/decorators/relationships.js | 7 +-- 5 files changed, 123 insertions(+), 97 deletions(-) diff --git a/dist/mobx-async-store.cjs.js b/dist/mobx-async-store.cjs.js index bcd16676..60089129 100644 --- a/dist/mobx-async-store.cjs.js +++ b/dist/mobx-async-store.cjs.js @@ -1417,43 +1417,35 @@ function () { return _this.fetchAll(type, queryParams); } else if (fromServer === false) { // If fromServer is false never fetch the data and return - return _this.getMatchingRecords(type, queryParams); + return _this.getMatchingRecords(type, queryParams) || []; } else { - return _this.findOrFetchAll(type, queryParams); + return _this.findOrFetchAll(type, queryParams) || []; } }; this.findAndFetchAll = function (type) { var options = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : {}; + var beforeFetch = options.beforeFetch, + afterFetch = options.afterFetch, + afterError = options.afterError, + queryParams = options.queryParams; - var records = _this.findAll(type, _objectSpread$2({}, options, { - fromServer: false - })); - - var beforeRefetch = options.beforeRefetch, - afterRefetch = options.afterRefetch; + var records = _this.getMatchingRecords(type, queryParams); - if (records.length > 0) { - beforeRefetch && beforeRefetch(records); + beforeFetch && beforeFetch(records); - _this.findAll(type, _objectSpread$2({}, options, { - fromServer: true - })).then(function (result) { - afterRefetch && afterRefetch(result); - }); + _this.fetchAll(type, queryParams).then(function (result) { + afterFetch && afterFetch(result); + }).catch(function (error) { + afterError(error); + }); - return records; - } else { - return _this.findAll(type, _objectSpread$2({}, options, { - fromServer: true - })); - } + return records || []; }; this.findOrFetchAll = function (type, queryParams) { // Get any matching records - var records = _this.getMatchingRecords(type, queryParams); // If any records are present - + var records = _this.getMatchingRecords(type, queryParams); if (records.length > 0) { // Return data @@ -2365,9 +2357,12 @@ function getRelatedRecords(record, property) { }); } else { var foreignId = "".concat(singularizeType(record.type), "_id"); - relatedRecords = record.store.getRecords(relationType).filter(function (rel) { - return String(rel[foreignId]) === String(record.id); - }); + + if (record.store.getRecords(relationType)) { + relatedRecords = record.store.getRecords(relationType).filter(function (rel) { + return String(rel[foreignId]) === String(record.id); + }); + } } return new RelatedRecordsArray(relatedRecords, record, relationType); diff --git a/dist/mobx-async-store.esm.js b/dist/mobx-async-store.esm.js index e749c46f..7832cbd7 100644 --- a/dist/mobx-async-store.esm.js +++ b/dist/mobx-async-store.esm.js @@ -1411,43 +1411,35 @@ function () { return _this.fetchAll(type, queryParams); } else if (fromServer === false) { // If fromServer is false never fetch the data and return - return _this.getMatchingRecords(type, queryParams); + return _this.getMatchingRecords(type, queryParams) || []; } else { - return _this.findOrFetchAll(type, queryParams); + return _this.findOrFetchAll(type, queryParams) || []; } }; this.findAndFetchAll = function (type) { var options = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : {}; + var beforeFetch = options.beforeFetch, + afterFetch = options.afterFetch, + afterError = options.afterError, + queryParams = options.queryParams; - var records = _this.findAll(type, _objectSpread$2({}, options, { - fromServer: false - })); - - var beforeRefetch = options.beforeRefetch, - afterRefetch = options.afterRefetch; + var records = _this.getMatchingRecords(type, queryParams); - if (records.length > 0) { - beforeRefetch && beforeRefetch(records); + beforeFetch && beforeFetch(records); - _this.findAll(type, _objectSpread$2({}, options, { - fromServer: true - })).then(function (result) { - afterRefetch && afterRefetch(result); - }); + _this.fetchAll(type, queryParams).then(function (result) { + afterFetch && afterFetch(result); + }).catch(function (error) { + afterError(error); + }); - return records; - } else { - return _this.findAll(type, _objectSpread$2({}, options, { - fromServer: true - })); - } + return records || []; }; this.findOrFetchAll = function (type, queryParams) { // Get any matching records - var records = _this.getMatchingRecords(type, queryParams); // If any records are present - + var records = _this.getMatchingRecords(type, queryParams); if (records.length > 0) { // Return data @@ -2359,9 +2351,12 @@ function getRelatedRecords(record, property) { }); } else { var foreignId = "".concat(singularizeType(record.type), "_id"); - relatedRecords = record.store.getRecords(relationType).filter(function (rel) { - return String(rel[foreignId]) === String(record.id); - }); + + if (record.store.getRecords(relationType)) { + relatedRecords = record.store.getRecords(relationType).filter(function (rel) { + return String(rel[foreignId]) === String(record.id); + }); + } } return new RelatedRecordsArray(relatedRecords, record, relationType); diff --git a/spec/Store.spec.js b/spec/Store.spec.js index 36dce757..b4c11fd4 100644 --- a/spec/Store.spec.js +++ b/spec/Store.spec.js @@ -211,9 +211,7 @@ describe('Store', () => { describe('if records of the specified type do not exist', () => { it('returns an empty array', () => { expect.assertions(1) - const todos = store.findAll('todos', { - fromServer: false - }) + const todos = store.findAll('todos', { fromServer: false }) expect(todos).toHaveLength(0) }) }) @@ -234,9 +232,7 @@ describe('Store', () => { it('fetches data from server', async () => { expect.assertions(4) fetch.mockResponse(mockTodosResponse) - const todos = await store.findAll('todos', { - fromServer: true - }) + const todos = await store.findAll('todos', { fromServer: true }) expect(todos).toHaveLength(1) expect(todos[0].title).toEqual('Do taxes') expect(fetch.mock.calls).toHaveLength(1) @@ -338,8 +334,7 @@ describe('Store', () => { expect(todos).toHaveLength(1) expect(todos[0].title).toEqual('Do taxes') expect(fetch.mock.calls).toHaveLength(1) - expect(fetch.mock.calls[0][0]) - .toEqual('/example_api/todos') + expect(fetch.mock.calls[0][0]).toEqual('/example_api/todos') }) }) @@ -536,8 +531,10 @@ describe('Store', () => { describe('findAndFetchAll', () => { let requestOptions let lazyLoadOptions - let mockAfterRefetch = jest.fn() - let mockBeforeRefetch = jest.fn() + let mockAfterFetch = jest.fn() + let mockBeforeFetch = jest.fn() + let mockTodosResponse2 + let mockAfterError = jest.fn() beforeEach(() => { jest.resetAllMocks() @@ -552,40 +549,44 @@ describe('Store', () => { lazyLoadOptions = { ...requestOptions, - afterRefetch: mockAfterRefetch, - beforeRefetch: mockBeforeRefetch + afterFetch: mockAfterFetch, + beforeFetch: mockBeforeFetch, + afterError: mockAfterError } - }) - - it('triggers a fetch if no cached data is found', async () => { - fetch.mockResponse(mockTodosResponse) - const result = await store.findAndFetchAll('todos', lazyLoadOptions) - - expect(result).toHaveLength(1) + mockTodosResponse2 = JSON.stringify({ + data: [ + mockTodoData.data, + { ...mockTodoData.data, id: 2, title: 'Test' } + ] + }) }) - it('returns cached data before refetching', async () => { + it('triggers a fetch if no cached data is found', async (done) => { fetch.mockResponse(mockTodosResponse) - await store.findAll('todos', requestOptions) + lazyLoadOptions.afterFetch = jest.fn((result) => { + expect(result).toHaveLength(1) + done() + }) + const result = store.findAndFetchAll('todos', lazyLoadOptions) - expect(result).toHaveLength(1) - expect(fetch.mock.calls).toHaveLength(2) + expect(result).toHaveLength(0) + expect(fetch).toHaveBeenCalled() }) - it('calls beforeRefetch callback with prefetch result', async () => { + it('calls beforeFetch callback with prefetch result', async () => { fetch.mockResponse(mockTodosResponse) await store.findAll('todos', requestOptions) const result = store.findAndFetchAll('todos', lazyLoadOptions) expect(result).toHaveLength(1) - expect(mockBeforeRefetch).toHaveBeenCalledWith(result) + expect(mockBeforeFetch).toHaveBeenCalledWith(result) }) - it('calls afterRefetch callback with refetch result', async (done) => { + it('calls afterFetch callback with refetch result', async (done) => { const mockTodosResponse2 = JSON.stringify({ data: [ mockTodoData.data, @@ -601,14 +602,50 @@ describe('Store', () => { // Trigger another request await store.findAll('todos', requestOptions) + lazyLoadOptions.afterFetch = jest.fn((result) => { + // The refetch result is different then the cached result, because + // mockTodosResponse2 has 2 records + expect(result).toHaveLength(2) + done() + }) + + store.findAndFetchAll('todos', lazyLoadOptions) + }) + + it('returns cached data before refetching', async (done) => { + fetch.mockResponses( + [mockTodosResponse, { status: 200 }], + [mockTodosResponse2, { status: 200 }] + ) + + await store.findAll('todos', requestOptions) + + lazyLoadOptions.afterFetch = jest.fn((result) => { + // The refetch result is different then the cached result, because + // mockTodosResponse2 has 2 records + expect(result).toHaveLength(2) + done() + }) + const result = store.findAndFetchAll('todos', lazyLoadOptions) + // mockTodosResponse has only one record expect(result).toHaveLength(1) + // fetch was called twice: once from the findAll and once from findAndFetchAll + // refetching + expect(fetch.mock.calls).toHaveLength(2) + }) - setImmediate(() => { - expect(mockAfterRefetch.mock.calls[0][0]).toHaveLength(2) + it('calls afterError if bad request', (done) => { + fetch.mockResponses([mockTodosResponse, { status: 400 }]) + + lazyLoadOptions.afterError = jest.fn((error) => { + // NOTE: We should have better errors than this. + expect(error).toEqual(400) done() }) + + store.findAndFetchAll('todos', lazyLoadOptions) }) }) }) diff --git a/src/Store.js b/src/Store.js index 93893b74..7ea1d118 100644 --- a/src/Store.js +++ b/src/Store.js @@ -223,9 +223,9 @@ class Store { return this.fetchAll(type, queryParams) } else if (fromServer === false) { // If fromServer is false never fetch the data and return - return this.getMatchingRecords(type, queryParams) + return this.getMatchingRecords(type, queryParams) || [] } else { - return this.findOrFetchAll(type, queryParams) + return this.findOrFetchAll(type, queryParams) || [] } } @@ -236,19 +236,18 @@ class Store { * @return {Array} */ findAndFetchAll = (type, options = {}) => { - let records = this.findAll(type, { ...options, fromServer: false }) + const { beforeFetch, afterFetch, afterError, queryParams } = options + const records = this.getMatchingRecords(type, queryParams) - const { beforeRefetch, afterRefetch } = options + beforeFetch && beforeFetch(records) - if (records.length > 0) { - beforeRefetch && beforeRefetch(records) - this.findAll(type, { ...options, fromServer: true }).then((result) => { - afterRefetch && afterRefetch(result) - }) - return records - } else { - return this.findAll(type, { ...options, fromServer: true }) - } + this.fetchAll(type, queryParams).then((result) => { + afterFetch && afterFetch(result) + }).catch((error) => { + afterError(error) + }) + + return records || [] } /** @@ -262,7 +261,6 @@ class Store { // Get any matching records const records = this.getMatchingRecords(type, queryParams) - // If any records are present if (records.length > 0) { // Return data return records diff --git a/src/decorators/relationships.js b/src/decorators/relationships.js index 4b993726..58a616d6 100644 --- a/src/decorators/relationships.js +++ b/src/decorators/relationships.js @@ -118,9 +118,10 @@ export function getRelatedRecords (record, property, modelType = null) { }) } else { const foreignId = `${singularizeType(record.type)}_id` - relatedRecords = record.store - .getRecords(relationType) - .filter(rel => String(rel[foreignId]) === String(record.id)) + if (record.store.getRecords(relationType)) { + relatedRecords = record.store.getRecords(relationType) + .filter(rel => String(rel[foreignId]) === String(record.id)) + } } return new RelatedRecordsArray(relatedRecords, record, relationType) From c2f8c3befee1b83196dc69e9abc01e75d8d3d7e7 Mon Sep 17 00:00:00 2001 From: Joe Torreggiani Date: Mon, 1 Jun 2020 13:43:09 -0400 Subject: [PATCH 06/10] Add comment --- src/Store.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Store.js b/src/Store.js index 7ea1d118..65c48986 100644 --- a/src/Store.js +++ b/src/Store.js @@ -261,6 +261,11 @@ class Store { // Get any matching records const records = this.getMatchingRecords(type, queryParams) + // NOTE: A broader RFC is in development to improve how we keep data in sync + // with the server. We likely will want to getMatchingRecords and getRecords + // to return null if nothing is found. However, this causes several regressions + // in portal we will need to address in a larger PR for mobx-async-store updates. + if (records.length > 0) { // Return data return records From 91f73b05497573c99e8a64617203875722be2300 Mon Sep 17 00:00:00 2001 From: Joe Torreggiani Date: Mon, 1 Jun 2020 13:45:09 -0400 Subject: [PATCH 07/10] Commit build --- dist/mobx-async-store.cjs.js | 6 +++++- dist/mobx-async-store.esm.js | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/dist/mobx-async-store.cjs.js b/dist/mobx-async-store.cjs.js index 60089129..b7de5dc2 100644 --- a/dist/mobx-async-store.cjs.js +++ b/dist/mobx-async-store.cjs.js @@ -1445,7 +1445,11 @@ function () { this.findOrFetchAll = function (type, queryParams) { // Get any matching records - var records = _this.getMatchingRecords(type, queryParams); + var records = _this.getMatchingRecords(type, queryParams); // NOTE: A broader RFC is in development to improve how we keep data in sync + // with the server. We likely will want to getMatchingRecords and getRecords + // to return null if nothing is found. However, this causes several regressions + // in portal we will need to address in a larger PR for mobx-async-store updates. + if (records.length > 0) { // Return data diff --git a/dist/mobx-async-store.esm.js b/dist/mobx-async-store.esm.js index 7832cbd7..c13ccaf3 100644 --- a/dist/mobx-async-store.esm.js +++ b/dist/mobx-async-store.esm.js @@ -1439,7 +1439,11 @@ function () { this.findOrFetchAll = function (type, queryParams) { // Get any matching records - var records = _this.getMatchingRecords(type, queryParams); + var records = _this.getMatchingRecords(type, queryParams); // NOTE: A broader RFC is in development to improve how we keep data in sync + // with the server. We likely will want to getMatchingRecords and getRecords + // to return null if nothing is found. However, this causes several regressions + // in portal we will need to address in a larger PR for mobx-async-store updates. + if (records.length > 0) { // Return data From bc0d026eaba05184c1914f77841d48eb51091a33 Mon Sep 17 00:00:00 2001 From: Joe Torreggiani Date: Mon, 1 Jun 2020 14:14:52 -0400 Subject: [PATCH 08/10] More fixes --- spec/Store.spec.js | 15 ++++++++------- src/Store.js | 29 +++++++++++++++++++++-------- 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/spec/Store.spec.js b/spec/Store.spec.js index b4c11fd4..e56fa9a7 100644 --- a/spec/Store.spec.js +++ b/spec/Store.spec.js @@ -549,8 +549,8 @@ describe('Store', () => { lazyLoadOptions = { ...requestOptions, - afterFetch: mockAfterFetch, - beforeFetch: mockBeforeFetch, + afterRefetch: mockAfterFetch, + beforeRefetch: mockBeforeFetch, afterError: mockAfterError } @@ -565,18 +565,19 @@ describe('Store', () => { it('triggers a fetch if no cached data is found', async (done) => { fetch.mockResponse(mockTodosResponse) - lazyLoadOptions.afterFetch = jest.fn((result) => { + lazyLoadOptions.afterRefetch = jest.fn((result) => { expect(result).toHaveLength(1) done() }) + await store.findAll('todos', requestOptions) const result = store.findAndFetchAll('todos', lazyLoadOptions) expect(result).toHaveLength(0) expect(fetch).toHaveBeenCalled() }) - it('calls beforeFetch callback with prefetch result', async () => { + it('calls beforeRefetch callback with prefetch result', async () => { fetch.mockResponse(mockTodosResponse) await store.findAll('todos', requestOptions) @@ -586,7 +587,7 @@ describe('Store', () => { expect(mockBeforeFetch).toHaveBeenCalledWith(result) }) - it('calls afterFetch callback with refetch result', async (done) => { + it('calls afterRefetch callback with refetch result', async (done) => { const mockTodosResponse2 = JSON.stringify({ data: [ mockTodoData.data, @@ -602,7 +603,7 @@ describe('Store', () => { // Trigger another request await store.findAll('todos', requestOptions) - lazyLoadOptions.afterFetch = jest.fn((result) => { + lazyLoadOptions.afterRefetch = jest.fn((result) => { // The refetch result is different then the cached result, because // mockTodosResponse2 has 2 records expect(result).toHaveLength(2) @@ -620,7 +621,7 @@ describe('Store', () => { await store.findAll('todos', requestOptions) - lazyLoadOptions.afterFetch = jest.fn((result) => { + lazyLoadOptions.afterRefetch = jest.fn((result) => { // The refetch result is different then the cached result, because // mockTodosResponse2 has 2 records expect(result).toHaveLength(2) diff --git a/src/Store.js b/src/Store.js index 65c48986..f1f1ded2 100644 --- a/src/Store.js +++ b/src/Store.js @@ -236,16 +236,29 @@ class Store { * @return {Array} */ findAndFetchAll = (type, options = {}) => { - const { beforeFetch, afterFetch, afterError, queryParams } = options - const records = this.getMatchingRecords(type, queryParams) + const { + beforeFetch, + afterFetch, + beforeRefetch, + afterRefetch, + afterError, + queryParams + } = options - beforeFetch && beforeFetch(records) + const records = this.getMatchingRecords(type, queryParams) - this.fetchAll(type, queryParams).then((result) => { - afterFetch && afterFetch(result) - }).catch((error) => { - afterError(error) - }) + // NOTE: See note findOrFetchAll about this conditional logic. + if (records.length > 0) { + beforeRefetch && beforeRefetch(records) + this.fetchAll(type, queryParams) + .then((result) => afterRefetch && afterRefetch(result)) + .catch((error) => afterError && afterError(error)) + } else { + beforeFetch && beforeFetch(records) + this.fetchAll(type, queryParams) + .then((result) => afterFetch && afterFetch(result)) + .catch((error) => afterError && afterError(error)) + } return records || [] } From 2b498f57513dfa2bba0625d9df5395056eee5544 Mon Sep 17 00:00:00 2001 From: Joe Torreggiani Date: Mon, 1 Jun 2020 14:15:06 -0400 Subject: [PATCH 09/10] Commit build --- dist/mobx-async-store.cjs.js | 27 ++++++++++++++++++++------- dist/mobx-async-store.esm.js | 27 ++++++++++++++++++++------- 2 files changed, 40 insertions(+), 14 deletions(-) diff --git a/dist/mobx-async-store.cjs.js b/dist/mobx-async-store.cjs.js index b7de5dc2..8295e541 100644 --- a/dist/mobx-async-store.cjs.js +++ b/dist/mobx-async-store.cjs.js @@ -1427,18 +1427,31 @@ function () { var options = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : {}; var beforeFetch = options.beforeFetch, afterFetch = options.afterFetch, + beforeRefetch = options.beforeRefetch, + afterRefetch = options.afterRefetch, afterError = options.afterError, queryParams = options.queryParams; - var records = _this.getMatchingRecords(type, queryParams); + var records = _this.getMatchingRecords(type, queryParams); // NOTE: See note findOrFetchAll about this conditional logic. - beforeFetch && beforeFetch(records); - _this.fetchAll(type, queryParams).then(function (result) { - afterFetch && afterFetch(result); - }).catch(function (error) { - afterError(error); - }); + if (records.length > 0) { + beforeRefetch && beforeRefetch(records); + + _this.fetchAll(type, queryParams).then(function (result) { + return afterRefetch && afterRefetch(result); + }).catch(function (error) { + return afterError && afterError(error); + }); + } else { + beforeFetch && beforeFetch(records); + + _this.fetchAll(type, queryParams).then(function (result) { + return afterFetch && afterFetch(result); + }).catch(function (error) { + return afterError && afterError(error); + }); + } return records || []; }; diff --git a/dist/mobx-async-store.esm.js b/dist/mobx-async-store.esm.js index c13ccaf3..47059427 100644 --- a/dist/mobx-async-store.esm.js +++ b/dist/mobx-async-store.esm.js @@ -1421,18 +1421,31 @@ function () { var options = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : {}; var beforeFetch = options.beforeFetch, afterFetch = options.afterFetch, + beforeRefetch = options.beforeRefetch, + afterRefetch = options.afterRefetch, afterError = options.afterError, queryParams = options.queryParams; - var records = _this.getMatchingRecords(type, queryParams); + var records = _this.getMatchingRecords(type, queryParams); // NOTE: See note findOrFetchAll about this conditional logic. - beforeFetch && beforeFetch(records); - _this.fetchAll(type, queryParams).then(function (result) { - afterFetch && afterFetch(result); - }).catch(function (error) { - afterError(error); - }); + if (records.length > 0) { + beforeRefetch && beforeRefetch(records); + + _this.fetchAll(type, queryParams).then(function (result) { + return afterRefetch && afterRefetch(result); + }).catch(function (error) { + return afterError && afterError(error); + }); + } else { + beforeFetch && beforeFetch(records); + + _this.fetchAll(type, queryParams).then(function (result) { + return afterFetch && afterFetch(result); + }).catch(function (error) { + return afterError && afterError(error); + }); + } return records || []; }; From 57879b33df4ad5503fc807a97c7c10ed701fbffc Mon Sep 17 00:00:00 2001 From: Joe Torreggiani Date: Mon, 1 Jun 2020 15:07:51 -0400 Subject: [PATCH 10/10] Bump version number --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 17b48d6b..4a60d965 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@artemisag/mobx-async-store", - "version": "1.0.23", + "version": "1.0.24", "module": "dist/mobx-async-store.esm.js", "browser": "dist/mobx-async-store.cjs.js", "main": "dist/mobx-async-store.cjs.js",