From 8f411af21763e0e645b0128632c20be45c9d8344 Mon Sep 17 00:00:00 2001 From: Alex Mattson Date: Mon, 27 Aug 2018 09:43:13 -0700 Subject: [PATCH 01/12] Warning for not using allowMultipleListeners config option --- src/actions/firestore.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/actions/firestore.js b/src/actions/firestore.js index 394dbdf3..ed892abd 100644 --- a/src/actions/firestore.js +++ b/src/actions/firestore.js @@ -326,6 +326,16 @@ export function setListeners(firebase, dispatch, listeners) { // truthy value if (!listenerExists(firebase, listener) || multipleListenersEnabled) { setListener(firebase, dispatch, listener); + } else { + const path = getQueryName(listener); + + /* eslint-disable no-console */ + console.warn( + `There are multiple listeners attempting to be set to ${path}. ` + + `If you are trying to do this intentionally please set the ` + + `configuration option 'allowMultipleListeners' to 'true'.`, + ); + /* eslint-enable no-console */ } }); } From 66865e2ac534c49e1dadfd7780669a3a3e074e40 Mon Sep 17 00:00:00 2001 From: Alex Mattson Date: Mon, 27 Aug 2018 13:49:43 -0700 Subject: [PATCH 02/12] Consolidate oneListenerPerPath and allowMultipleListeners logic --- README.md | 7 +-- src/actions/firestore.js | 69 +++++++++-------------------- src/constants.js | 2 +- test/unit/actions/firestore.spec.js | 12 ++--- 4 files changed, 29 insertions(+), 61 deletions(-) diff --git a/README.md b/README.md index 4e5ac022..8276d641 100644 --- a/README.md +++ b/README.md @@ -251,7 +251,7 @@ After setting a listener/multiple listeners, you can unset them with the followi ```js store.firestore.unsetListener({ collection: 'cities' }), // of for any number of listeners at once : -store.firestore.unsetListeners([query1Options, query2Options]), +store.firestore.unsetListeners([query1Options, query2Options]), // here query1Options as in { collection: 'cities' } for example ``` @@ -480,11 +480,6 @@ Default: `false` Whether or not to allow multiple listeners to be attached for the same query. If a function is passed the arguments it receives are `listenerToAttach`, `currentListeners`, and the function should return a boolean. -#### oneListenerPerPath -Default: `false` - -If set to true redux-firestore will attach a listener on the same path just once & will count how many the listener was set. When you try to unset the listener, it won't unset until you have less than 1 listeners on this path - #### preserveOnDelete Default: `null` diff --git a/src/actions/firestore.js b/src/actions/firestore.js index ed892abd..4866ee78 100644 --- a/src/actions/firestore.js +++ b/src/actions/firestore.js @@ -4,7 +4,6 @@ import { actionTypes } from '../constants'; import { attachListener, detachListener, - listenerExists, orderedFromSnap, dataByIdSnapshot, getQueryConfig, @@ -298,44 +297,20 @@ export function setListeners(firebase, dispatch, listeners) { } const { config } = firebase._; - - // Only attach one listener (count of matching listener path calls is tracked) - if (config.oneListenerPerPath) { - return listeners.forEach(listener => { - const path = getQueryName(listener); - const oldListenerCount = pathListenerCounts[path] || 0; - pathListenerCounts[path] = oldListenerCount + 1; - - // If we already have an attached listener exit here - if (oldListenerCount > 0) { - return; - } - - setListener(firebase, dispatch, listener); - }); - } + const { allowMultipleListeners } = config; return listeners.forEach(listener => { - // Config for supporting attaching of multiple listener callbacks - const multipleListenersEnabled = isFunction(config.allowMultipleListeners) - ? config.allowMultipleListeners(listener, firebase._.listeners) - : config.allowMultipleListeners; + const path = getQueryName(listener); + const oldListenerCount = pathListenerCounts[path] || 0; + const multipleListenersEnabled = isFunction(allowMultipleListeners) + ? allowMultipleListeners(listener, firebase._.listeners) + : allowMultipleListeners; - // Only attach listener if it does not already exist or - // if multiple listeners config is true or is a function which returns - // truthy value - if (!listenerExists(firebase, listener) || multipleListenersEnabled) { - setListener(firebase, dispatch, listener); - } else { - const path = getQueryName(listener); + pathListenerCounts[path] = oldListenerCount + 1; - /* eslint-disable no-console */ - console.warn( - `There are multiple listeners attempting to be set to ${path}. ` + - `If you are trying to do this intentionally please set the ` + - `configuration option 'allowMultipleListeners' to 'true'.`, - ); - /* eslint-enable no-console */ + // If we already have an attached listener exit here + if (oldListenerCount === 0 || multipleListenersEnabled) { + setListener(firebase, dispatch, listener); } }); } @@ -368,25 +343,23 @@ export function unsetListeners(firebase, dispatch, listeners) { ); } const { config } = firebase._; + const { allowMultipleListeners } = config; // Keep one listener path even when detaching - if (config.oneListenerPerPath) { - listeners.forEach(listener => { - const path = getQueryName(listener); - pathListenerCounts[path] -= 1; + listeners.forEach(listener => { + const path = getQueryName(listener); + const listenerExists = pathListenerCounts[path] >= 1; + const multipleListenersEnabled = isFunction(allowMultipleListeners) + ? allowMultipleListeners(listener, firebase._.listeners) + : allowMultipleListeners; + if (listenerExists) { + pathListenerCounts[path] -= 1; // If we aren't supposed to have listners for this path, then remove them - if (pathListenerCounts[path] === 0) { + if (pathListenerCounts[path] === 0 || multipleListenersEnabled) { unsetListener(firebase, dispatch, listener); } - }); - - return; - } - - listeners.forEach(listener => { - // Remove listener only if it exists - unsetListener(firebase, dispatch, listener); + } }); } diff --git a/src/constants.js b/src/constants.js index dd9ee3e0..788cf13e 100644 --- a/src/constants.js +++ b/src/constants.js @@ -100,7 +100,7 @@ export const actionTypes = { * preserve from state when LISTENER_ERROR action is dispatched. * @property {Boolean} enhancerNamespace - `'firestore'` Namespace under which * enhancer places internal instance on redux store (i.e. store.firestore). - * @property {Boolean|Function} allowMultipleListeners - `null` Whether or not + * @property {Boolean|Function} allowMultipleListeners - `false` Whether or not * to allow multiple listeners to be attached for the same query. If a function * is passed the arguments it receives are `listenerToAttach`, * `currentListeners`, and the function should return a boolean. diff --git a/test/unit/actions/firestore.spec.js b/test/unit/actions/firestore.spec.js index fe8daa5a..58a1c770 100644 --- a/test/unit/actions/firestore.spec.js +++ b/test/unit/actions/firestore.spec.js @@ -654,12 +654,12 @@ describe('firestoreActions', () => { ); }); - describe('oneListenerPerPath', () => { + describe('allowMultipleListeners', () => { it('works with one listener', async () => { const fakeFirebaseWithOneListener = { _: { listeners: {}, - config: { ...defaultConfig, oneListenerPerPath: true }, + config: { ...defaultConfig, allowMultipleListeners: false }, }, firestore: () => ({ collection: collectionClass, @@ -688,7 +688,7 @@ describe('firestoreActions', () => { const fakeFirebaseWithOneListener = { _: { listeners: {}, - config: { ...defaultConfig, oneListenerPerPath: true }, + config: { ...defaultConfig, allowMultipleListeners: false }, }, firestore: () => ({ collection: collectionClass, @@ -769,12 +769,12 @@ describe('firestoreActions', () => { }); }); - describe('oneListenerPerPath option enabled', () => { + describe('allowMultipleListeners option enabled', () => { it('dispatches UNSET_LISTENER action', async () => { const fakeFirebaseWithOneListener = { _: { listeners: {}, - config: { ...defaultConfig, oneListenerPerPath: true }, + config: { ...defaultConfig, allowMultipleListeners: false }, }, firestore: () => ({ collection: collectionClass, @@ -793,7 +793,7 @@ describe('firestoreActions', () => { const fakeFirebaseWithOneListener = { _: { listeners: {}, - config: { ...defaultConfig, oneListenerPerPath: true }, + config: { ...defaultConfig, allowMultipleListeners: false }, }, firestore: () => ({ collection: collectionClass, From ffb5092836b54efa464623a3a564ffe9669205dd Mon Sep 17 00:00:00 2001 From: Daniel Dimitrov Date: Thu, 11 Oct 2018 10:16:16 +0200 Subject: [PATCH 03/12] fix for storeAs and documents that contain a dot https://github.com/prescottprue/redux-firestore/issues/139 --- .gitignore | 1 + package.json | 1 + src/utils/reducers.js | 3 ++- 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 37842882..a825647e 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,4 @@ es *.log coverage .idea +.DS_Store diff --git a/package.json b/package.json index ba349e07..99e6ac48 100644 --- a/package.json +++ b/package.json @@ -22,6 +22,7 @@ "build:umd:min": "cross-env BABEL_ENV=commonjs NODE_ENV=production webpack", "build": "npm run build:commonjs && npm run build:es && npm run build:umd && npm run build:umd:min", "watch": "npm run build:es -- --watch", + "watch:lib": "npm run build:lib -- --watch", "watch:commonjs": "npm run build:commonjs -- --watch", "test": "mocha -R spec ./test/unit/**", "test:cov": "istanbul cover $(npm bin)/_mocha ./test/unit/**", diff --git a/src/utils/reducers.js b/src/utils/reducers.js index 972dba52..ba343e9a 100644 --- a/src/utils/reducers.js +++ b/src/utils/reducers.js @@ -78,7 +78,8 @@ export function pathFromMeta(meta) { } const { collection, doc, subcollections, storeAs } = meta; if (storeAs) { - return doc ? `${storeAs}.${doc}` : storeAs; + // Use array here - if we don't we end up in trouble with docs that contain a dot + return doc ? [storeAs, doc] : storeAs; } if (meta.path) { return meta.path.split('/'); From bf04e64c5039fa16cf57af6ac296bcea59fcef73 Mon Sep 17 00:00:00 2001 From: Daniel Dimitrov Date: Thu, 11 Oct 2018 14:37:42 +0200 Subject: [PATCH 04/12] fix dot documents even if there is no storeAs or meta.path --- src/utils/reducers.js | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/utils/reducers.js b/src/utils/reducers.js index ba343e9a..a6e58b0c 100644 --- a/src/utils/reducers.js +++ b/src/utils/reducers.js @@ -69,7 +69,7 @@ export function combineReducers(reducers) { * @param {String} meta.storeAs - Another key within redux store that the * action associates with (used for storing data under a path different * from its collection/document) - * @return {String} String path to be used within reducer + * @return {Array} Array with path segments * @private */ export function pathFromMeta(meta) { @@ -84,18 +84,23 @@ export function pathFromMeta(meta) { if (meta.path) { return meta.path.split('/'); } + if (!collection) { throw new Error('Collection is required to construct reducer path.'); } - let basePath = collection; + + let basePath = [collection]; + if (doc) { - basePath += `.${doc}`; + basePath = [...basePath, doc]; } + if (!subcollections) { return basePath; } + const mappedCollections = subcollections.map(pathFromMeta); - return basePath.concat(`.${mappedCollections.join('.')}`); + return [...basePath, ...mappedCollections.flat()]; } /** From 988470099eedbb99a26bb59e7549acf87273e007 Mon Sep 17 00:00:00 2001 From: Daniel Dimitrov Date: Thu, 11 Oct 2018 16:21:12 +0200 Subject: [PATCH 05/12] =?UTF-8?q?we=20should=20always=20use=20array=20with?= =?UTF-8?q?=20setWith=20to=20avoid=20shit=20hitting=20the=20fan=E2=80=A6.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/reducers/dataReducer.js | 6 +++--- src/utils/reducers.js | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/reducers/dataReducer.js b/src/reducers/dataReducer.js index 9c27b321..b7f4ba22 100644 --- a/src/reducers/dataReducer.js +++ b/src/reducers/dataReducer.js @@ -46,18 +46,18 @@ export default function dataReducer(state = {}, action) { // Data to set to state is doc if doc name exists within meta const data = docName ? get(payload.data, docName) : payload.data; // Get previous data at path to check for existence - const previousData = get(state, meta.storeAs || pathFromMeta(meta)); + const previousData = get(state, meta.storeAs ? [meta.storeAs] : pathFromMeta(meta)); // Set data (without merging) if no previous data exists or if there are subcollections if (!previousData || meta.subcollections) { // Set data to state immutabily (lodash/fp's setWith creates copy) - return setWith(Object, meta.storeAs || pathFromMeta(meta), data, state); + return setWith(Object, meta.storeAs ? [meta.storeAs] : pathFromMeta(meta), data, state); } // Otherwise merge with existing data const mergedData = assign(previousData, data); // Set data to state (with merge) immutabily (lodash/fp's setWith creates copy) return setWith( Object, - meta.storeAs || pathFromMeta(meta), + meta.storeAs ? [meta.storeAs] : pathFromMeta(meta), mergedData, state, ); diff --git a/src/utils/reducers.js b/src/utils/reducers.js index a6e58b0c..d7a9f677 100644 --- a/src/utils/reducers.js +++ b/src/utils/reducers.js @@ -79,7 +79,7 @@ export function pathFromMeta(meta) { const { collection, doc, subcollections, storeAs } = meta; if (storeAs) { // Use array here - if we don't we end up in trouble with docs that contain a dot - return doc ? [storeAs, doc] : storeAs; + return doc ? [storeAs, doc] : [storeAs]; } if (meta.path) { return meta.path.split('/'); From ab249911ec9f307b31c521513093856f3cfff713 Mon Sep 17 00:00:00 2001 From: Daniel Dimitrov Date: Thu, 11 Oct 2018 16:22:37 +0200 Subject: [PATCH 06/12] =?UTF-8?q?lint=E2=80=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/reducers/dataReducer.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/reducers/dataReducer.js b/src/reducers/dataReducer.js index b7f4ba22..8505f320 100644 --- a/src/reducers/dataReducer.js +++ b/src/reducers/dataReducer.js @@ -46,11 +46,19 @@ export default function dataReducer(state = {}, action) { // Data to set to state is doc if doc name exists within meta const data = docName ? get(payload.data, docName) : payload.data; // Get previous data at path to check for existence - const previousData = get(state, meta.storeAs ? [meta.storeAs] : pathFromMeta(meta)); + const previousData = get( + state, + meta.storeAs ? [meta.storeAs] : pathFromMeta(meta), + ); // Set data (without merging) if no previous data exists or if there are subcollections if (!previousData || meta.subcollections) { // Set data to state immutabily (lodash/fp's setWith creates copy) - return setWith(Object, meta.storeAs ? [meta.storeAs] : pathFromMeta(meta), data, state); + return setWith( + Object, + meta.storeAs ? [meta.storeAs] : pathFromMeta(meta), + data, + state, + ); } // Otherwise merge with existing data const mergedData = assign(previousData, data); From 6849735e5a9e21b47f692a1cfccf75e0588037e7 Mon Sep 17 00:00:00 2001 From: Daniel Dimitrov Date: Fri, 26 Oct 2018 11:15:22 +0200 Subject: [PATCH 07/12] =?UTF-8?q?use=20flatten=20from=20lodash=20-=20for?= =?UTF-8?q?=20some=20reason=20.flat()=20doesn=E2=80=99t=20work=20on=20rn?= =?UTF-8?q?=20in=20production=20mode?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/utils/reducers.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/utils/reducers.js b/src/utils/reducers.js index d7a9f677..7f867f86 100644 --- a/src/utils/reducers.js +++ b/src/utils/reducers.js @@ -5,6 +5,7 @@ import { pick, replace, trimStart, + flatten } from 'lodash'; /** @@ -100,7 +101,8 @@ export function pathFromMeta(meta) { } const mappedCollections = subcollections.map(pathFromMeta); - return [...basePath, ...mappedCollections.flat()]; + + return [...basePath, ...flatten(mappedCollections)]; } /** From 2370ae62d7c4000324367cf74317bdda91e0a7d5 Mon Sep 17 00:00:00 2001 From: Daniel Dimitrov Date: Fri, 26 Oct 2018 11:17:25 +0200 Subject: [PATCH 08/12] eslint --- src/utils/reducers.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/reducers.js b/src/utils/reducers.js index 7f867f86..ac624835 100644 --- a/src/utils/reducers.js +++ b/src/utils/reducers.js @@ -5,7 +5,7 @@ import { pick, replace, trimStart, - flatten + flatten, } from 'lodash'; /** From 0bd0025ec0fa8d3e94ca85e12904cc37be3d1153 Mon Sep 17 00:00:00 2001 From: Scott Prue Date: Mon, 29 Oct 2018 17:29:30 -0700 Subject: [PATCH 09/12] v0.6.0-alpha.3 --- package-lock.json | 149 ++++++++++++---------------------------------- package.json | 5 +- 2 files changed, 41 insertions(+), 113 deletions(-) diff --git a/package-lock.json b/package-lock.json index 4976306d..fbfc4691 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "redux-firestore", - "version": "0.6.0-alpha.2", + "version": "0.6.0-alpha.3", "lockfileVersion": 1, "requires": true, "dependencies": { @@ -454,11 +454,6 @@ "integrity": "sha1-iYUI2iIm84DfkEcoRWhJwVAaSw0=", "dev": true }, - "asap": { - "version": "2.0.6", - "resolved": "https://registry.npmjs.org/asap/-/asap-2.0.6.tgz", - "integrity": "sha1-5QNHYR1+aQlDIIu9r+vLwvuGbUY=" - }, "asn1": { "version": "0.2.3", "resolved": "https://registry.npmjs.org/asn1/-/asn1-0.2.3.tgz", @@ -571,7 +566,7 @@ "convert-source-map": "1.5.0", "fs-readdir-recursive": "1.0.0", "glob": "7.1.2", - "lodash": "4.17.10", + "lodash": "4.17.11", "output-file-sync": "1.1.2", "path-is-absolute": "1.0.1", "slash": "1.0.0", @@ -609,7 +604,7 @@ "convert-source-map": "1.5.0", "debug": "2.6.9", "json5": "0.5.1", - "lodash": "4.17.10", + "lodash": "4.17.11", "minimatch": "3.0.4", "path-is-absolute": "1.0.1", "private": "0.1.8", @@ -669,7 +664,7 @@ "debug": "3.1.0", "globals": "10.1.0", "invariant": "2.2.2", - "lodash": "4.17.10" + "lodash": "4.17.11" } }, "babel-types": { @@ -679,7 +674,7 @@ "dev": true, "requires": { "esutils": "2.0.2", - "lodash": "4.17.10", + "lodash": "4.17.11", "to-fast-properties": "2.0.0" } }, @@ -743,7 +738,7 @@ "babel-types": "6.26.0", "detect-indent": "4.0.0", "jsesc": "1.3.0", - "lodash": "4.17.10", + "lodash": "4.17.11", "source-map": "0.5.7", "trim-right": "1.0.1" } @@ -791,7 +786,7 @@ "babel-helper-function-name": "6.24.1", "babel-runtime": "6.26.0", "babel-types": "6.26.0", - "lodash": "4.17.10" + "lodash": "4.17.11" }, "dependencies": { "babel-helper-function-name": { @@ -877,7 +872,7 @@ "babel-traverse": "7.0.0-beta.0", "babel-types": "7.0.0-beta.0", "babylon": "7.0.0-beta.22", - "lodash": "4.17.10" + "lodash": "4.17.11" } }, "babel-traverse": { @@ -894,7 +889,7 @@ "debug": "3.1.0", "globals": "10.1.0", "invariant": "2.2.2", - "lodash": "4.17.10" + "lodash": "4.17.11" } }, "babel-types": { @@ -904,7 +899,7 @@ "dev": true, "requires": { "esutils": "2.0.2", - "lodash": "4.17.10", + "lodash": "4.17.11", "to-fast-properties": "2.0.0" } }, @@ -973,7 +968,7 @@ "dev": true, "requires": { "esutils": "2.0.2", - "lodash": "4.17.10", + "lodash": "4.17.11", "to-fast-properties": "2.0.0" } }, @@ -1013,7 +1008,7 @@ "requires": { "babel-runtime": "6.26.0", "babel-types": "6.26.0", - "lodash": "4.17.10" + "lodash": "4.17.11" } }, "babel-helper-remap-async-to-generator": { @@ -1114,7 +1109,7 @@ "dev": true, "requires": { "glob": "7.1.2", - "lodash": "4.17.10" + "lodash": "4.17.11" } }, "babel-plugin-module-resolver": { @@ -1205,7 +1200,7 @@ "babel-template": "6.26.0", "babel-traverse": "6.26.0", "babel-types": "6.26.0", - "lodash": "4.17.10" + "lodash": "4.17.11" } }, "babel-plugin-transform-es2015-classes": { @@ -1683,7 +1678,7 @@ "babel-runtime": "6.26.0", "core-js": "2.5.6", "home-or-tmp": "2.0.0", - "lodash": "4.17.10", + "lodash": "4.17.11", "mkdirp": "0.5.1", "source-map-support": "0.4.18" }, @@ -1724,7 +1719,7 @@ "babel-traverse": "6.26.0", "babel-types": "6.26.0", "babylon": "6.18.0", - "lodash": "4.17.10" + "lodash": "4.17.11" } }, "babel-traverse": { @@ -1741,7 +1736,7 @@ "debug": "2.6.9", "globals": "9.18.0", "invariant": "2.2.2", - "lodash": "4.17.10" + "lodash": "4.17.11" } }, "babel-types": { @@ -1752,7 +1747,7 @@ "requires": { "babel-runtime": "6.26.0", "esutils": "2.0.2", - "lodash": "4.17.10", + "lodash": "4.17.11", "to-fast-properties": "1.0.3" } }, @@ -2424,11 +2419,6 @@ "integrity": "sha1-Z29us8OZl8LuGsOpJP1hJHSPV40=", "dev": true }, - "core-js": { - "version": "1.2.7", - "resolved": "https://registry.npmjs.org/core-js/-/core-js-1.2.7.tgz", - "integrity": "sha1-ZSKUwUZR2yj6k70tX/KYOk8IxjY=" - }, "core-util-is": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/core-util-is/-/core-util-is-1.0.2.tgz", @@ -2817,14 +2807,6 @@ "integrity": "sha1-TapNnbAPmBmIDHn6RXrlsJof04k=", "dev": true }, - "encoding": { - "version": "0.1.12", - "resolved": "https://registry.npmjs.org/encoding/-/encoding-0.1.12.tgz", - "integrity": "sha1-U4tm8+5izRq1HsMjgp0flIDHS+s=", - "requires": { - "iconv-lite": "0.4.23" - } - }, "end-of-stream": { "version": "1.4.1", "resolved": "https://registry.npmjs.org/end-of-stream/-/end-of-stream-1.4.1.tgz", @@ -3002,7 +2984,7 @@ "js-yaml": "3.10.0", "json-stable-stringify": "1.0.1", "levn": "0.3.0", - "lodash": "4.17.10", + "lodash": "4.17.11", "mkdirp": "0.5.1", "natural-compare": "1.4.0", "optionator": "0.8.2", @@ -3396,20 +3378,6 @@ "integrity": "sha1-PYpcZog6FqMMqGQ+hR8Zuqd5eRc=", "dev": true }, - "fbjs": { - "version": "0.8.16", - "resolved": "https://registry.npmjs.org/fbjs/-/fbjs-0.8.16.tgz", - "integrity": "sha1-XmdDL1UNxBtXK/VYR7ispk5TN9s=", - "requires": { - "core-js": "1.2.7", - "isomorphic-fetch": "2.2.1", - "loose-envify": "1.3.1", - "object-assign": "4.1.1", - "promise": "7.3.1", - "setimmediate": "1.0.5", - "ua-parser-js": "0.7.18" - } - }, "figures": { "version": "1.7.0", "resolved": "https://registry.npmjs.org/figures/-/figures-1.7.0.tgz", @@ -4518,6 +4486,7 @@ "version": "0.4.23", "resolved": "https://registry.npmjs.org/iconv-lite/-/iconv-lite-0.4.23.tgz", "integrity": "sha512-neyTUVFtahjf0mB3dZT77u+8O0QB89jFdnBkd5P1JgYPbPaia3gXXOVL2fq8VyU2gMMD7SaN7QukTB/pmXYvDA==", + "dev": true, "requires": { "safer-buffer": "2.1.2" } @@ -4595,7 +4564,7 @@ "cli-cursor": "1.0.2", "cli-width": "2.2.0", "figures": "1.7.0", - "lodash": "4.17.10", + "lodash": "4.17.11", "readline2": "1.0.1", "run-async": "0.1.0", "rx-lite": "3.1.2", @@ -4896,7 +4865,8 @@ "is-stream": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/is-stream/-/is-stream-1.1.0.tgz", - "integrity": "sha1-EtSj3U5o4Lec6428hBc66A2RykQ=" + "integrity": "sha1-EtSj3U5o4Lec6428hBc66A2RykQ=", + "dev": true }, "is-symbol": { "version": "1.0.1", @@ -4944,15 +4914,6 @@ "isarray": "1.0.0" } }, - "isomorphic-fetch": { - "version": "2.2.1", - "resolved": "https://registry.npmjs.org/isomorphic-fetch/-/isomorphic-fetch-2.2.1.tgz", - "integrity": "sha1-YRrhrPFPXoH3KVB0coGf6XM1WKk=", - "requires": { - "node-fetch": "1.7.3", - "whatwg-fetch": "2.0.4" - } - }, "isstream": { "version": "0.1.2", "resolved": "https://registry.npmjs.org/isstream/-/isstream-0.1.2.tgz", @@ -5000,7 +4961,7 @@ "integrity": "sha512-e+lJAJeNWuPCNyxZKOBdaJGyLGHugXVQtrAwtuAe2vhxTYxFTKE73p8JuTmdH0qdQZtDvI4dhJwjZc5zsfIsYw==", "dev": true, "requires": { - "lodash": "4.17.10" + "lodash": "4.17.11" } } } @@ -5106,7 +5067,8 @@ "js-tokens": { "version": "3.0.2", "resolved": "https://registry.npmjs.org/js-tokens/-/js-tokens-3.0.2.tgz", - "integrity": "sha1-mGbfOVECEw449/mWvOtlRDIJwls=" + "integrity": "sha1-mGbfOVECEw449/mWvOtlRDIJwls=", + "dev": true }, "js-yaml": { "version": "3.10.0", @@ -5295,9 +5257,9 @@ } }, "lodash": { - "version": "4.17.10", - "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.10.tgz", - "integrity": "sha512-UejweD1pDoXu+AD825lWwp4ZGtSwgnpZxb3JDViD7StjQz+Nb/6l093lx4OQ0foGWNRoc19mWy7BzL+UAK2iVg==" + "version": "4.17.11", + "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.11.tgz", + "integrity": "sha512-cQKh8igo5QUhZ7lg38DYWAxMvjSAKG0A8wGSVimP07SIUEK2UO+arSRKbRZWtelMtN5V0Hkwh5ryOto/SshYIg==" }, "lodash-es": { "version": "4.17.4", @@ -5339,6 +5301,7 @@ "version": "1.3.1", "resolved": "https://registry.npmjs.org/loose-envify/-/loose-envify-1.3.1.tgz", "integrity": "sha1-0aitM/qc4OcT1l/dCsi3SNR4yEg=", + "dev": true, "requires": { "js-tokens": "3.0.2" } @@ -5701,15 +5664,6 @@ "text-encoding": "0.6.4" } }, - "node-fetch": { - "version": "1.7.3", - "resolved": "https://registry.npmjs.org/node-fetch/-/node-fetch-1.7.3.tgz", - "integrity": "sha512-NhZ4CsKx7cYm2vSrBAr2PvFOe6sWDf0UYLRqA6svUYg7+/TSfVAu49jYC4BvQ4Sms9SZgdqGBgroqfDhJdTyKQ==", - "requires": { - "encoding": "0.1.12", - "is-stream": "1.1.0" - } - }, "node-libs-browser": { "version": "2.1.0", "resolved": "https://registry.npmjs.org/node-libs-browser/-/node-libs-browser-2.1.0.tgz", @@ -5795,7 +5749,8 @@ "object-assign": { "version": "4.1.1", "resolved": "https://registry.npmjs.org/object-assign/-/object-assign-4.1.1.tgz", - "integrity": "sha1-IQmtx5ZYh8/AXLvUQsrIv7s2CGM=" + "integrity": "sha1-IQmtx5ZYh8/AXLvUQsrIv7s2CGM=", + "dev": true }, "object-copy": { "version": "0.1.0", @@ -6242,30 +6197,12 @@ "integrity": "sha1-4mDHj2Fhzdmw5WzD4Khd4Xx6V74=", "dev": true }, - "promise": { - "version": "7.3.1", - "resolved": "https://registry.npmjs.org/promise/-/promise-7.3.1.tgz", - "integrity": "sha512-nolQXZ/4L+bP/UGlkfaIujX9BKxGwmQ9OT4mOt5yvy8iK1h3wqTEJCijzGANTCCl9nWjY41juyAn2K3Q1hLLTg==", - "requires": { - "asap": "2.0.6" - } - }, "promise-inflight": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/promise-inflight/-/promise-inflight-1.0.1.tgz", "integrity": "sha1-mEcocL8igTL8vdhoEputEsPAKeM=", "dev": true }, - "prop-types": { - "version": "15.6.1", - "resolved": "https://registry.npmjs.org/prop-types/-/prop-types-15.6.1.tgz", - "integrity": "sha512-4ec7bY1Y66LymSUOH/zARVYObB23AT2h8cf6e/O6ZALB/N0sqZFEx7rq6EYPX2MkOdKORuooI/H5k9TlR4q7kQ==", - "requires": { - "fbjs": "0.8.16", - "loose-envify": "1.3.1", - "object-assign": "4.1.1" - } - }, "prr": { "version": "0.0.0", "resolved": "https://registry.npmjs.org/prr/-/prr-0.0.0.tgz", @@ -6479,7 +6416,7 @@ "integrity": "sha1-BrcxIyFZAdJdBlvjQusCa8HIU3s=", "dev": true, "requires": { - "lodash": "4.17.10", + "lodash": "4.17.11", "lodash-es": "4.17.4", "loose-envify": "1.3.1", "symbol-observable": "1.0.4" @@ -6781,7 +6718,8 @@ "safer-buffer": { "version": "2.1.2", "resolved": "https://registry.npmjs.org/safer-buffer/-/safer-buffer-2.1.2.tgz", - "integrity": "sha512-YZo3K82SD7Riyi0E1EQPojLz7kpepnSQI9IyPbHHg1XXXevb5dJI7tpyN2ADxGcQbHG7vcyRHk0cbwqcQriUtg==" + "integrity": "sha512-YZo3K82SD7Riyi0E1EQPojLz7kpepnSQI9IyPbHHg1XXXevb5dJI7tpyN2ADxGcQbHG7vcyRHk0cbwqcQriUtg==", + "dev": true }, "samsam": { "version": "1.3.0", @@ -6869,7 +6807,8 @@ "setimmediate": { "version": "1.0.5", "resolved": "https://registry.npmjs.org/setimmediate/-/setimmediate-1.0.5.tgz", - "integrity": "sha1-KQy7Iy4waULX1+qbg3Mqt4VvgoU=" + "integrity": "sha1-KQy7Iy4waULX1+qbg3Mqt4VvgoU=", + "dev": true }, "sha.js": { "version": "2.4.11", @@ -7365,7 +7304,7 @@ "ajv": "4.11.8", "ajv-keywords": "1.5.1", "chalk": "1.1.3", - "lodash": "4.17.10", + "lodash": "4.17.11", "slice-ansi": "0.0.4", "string-width": "2.1.1" }, @@ -7579,11 +7518,6 @@ "integrity": "sha1-hnrHTjhkGHsdPUfZlqeOxciDB3c=", "dev": true }, - "ua-parser-js": { - "version": "0.7.18", - "resolved": "https://registry.npmjs.org/ua-parser-js/-/ua-parser-js-0.7.18.tgz", - "integrity": "sha512-LtzwHlVHwFGTptfNSgezHp7WUlwiqb0gA9AALRbKaERfxwJoiX0A73QbTToxteIAuIaFshhgIZfqK8s7clqgnA==" - }, "uglify-js": { "version": "2.8.29", "resolved": "https://registry.npmjs.org/uglify-js/-/uglify-js-2.8.29.tgz", @@ -8687,7 +8621,7 @@ "cli-width": "2.2.0", "external-editor": "3.0.0", "figures": "2.0.0", - "lodash": "4.17.10", + "lodash": "4.17.11", "mute-stream": "0.0.7", "run-async": "2.3.0", "rxjs": "6.2.1", @@ -8816,11 +8750,6 @@ } } }, - "whatwg-fetch": { - "version": "2.0.4", - "resolved": "https://registry.npmjs.org/whatwg-fetch/-/whatwg-fetch-2.0.4.tgz", - "integrity": "sha512-dcQ1GWpOD/eEQ97k66aiEVpNnapVj90/+R+SXTPYGHpYBBypfKJEQjLrvMZ7YXbKm21gXd4NcuxUTjiv1YtLng==" - }, "which": { "version": "1.3.0", "resolved": "https://registry.npmjs.org/which/-/which-1.3.0.tgz", diff --git a/package.json b/package.json index 90ee278a..a03b9890 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "redux-firestore", - "version": "0.6.0-alpha.2", + "version": "0.6.0-alpha.3", "description": "Redux bindings for Firestore.", "main": "lib/index.js", "module": "es/index.js", @@ -35,8 +35,7 @@ }, "dependencies": { "immer": "1.5.0", - "lodash": "^4.17.10", - "prop-types": "^15.6.1", + "lodash": "^4.17.11", "reduce-reducers": "0.4.3" }, "devDependencies": { From af8944c5e854909ce2df9adf5a69d5e033d0552b Mon Sep 17 00:00:00 2001 From: Scott Prue Date: Tue, 30 Oct 2018 17:54:44 -0700 Subject: [PATCH 10/12] Start of populate support - #48 --- .babelrc | 4 +- .eslintrc | 1 + examples/basic/jsconfig.json | 9 ++ jsconfig.json | 17 ++++ src/actions/firestore.js | 125 +++++++++++++++++-------- src/utils/async.js | 9 ++ src/utils/query.js | 171 ++++++++++++++++++++++++++++++++++- 7 files changed, 295 insertions(+), 41 deletions(-) create mode 100644 examples/basic/jsconfig.json create mode 100644 jsconfig.json create mode 100644 src/utils/async.js diff --git a/.babelrc b/.babelrc index 7be1f013..c3f32013 100644 --- a/.babelrc +++ b/.babelrc @@ -10,8 +10,9 @@ ], "plugins": [ "lodash", + "transform-runtime", "transform-object-rest-spread", - "transform-object-assign", + "transform-object-assign" ], "env": { "es": { @@ -28,7 +29,6 @@ }, "test": { "plugins": [ - "transform-runtime", "transform-async-to-generator", ["module-resolver", { "root": ["./src"] diff --git a/.eslintrc b/.eslintrc index f60ebc66..00ccb4fc 100644 --- a/.eslintrc +++ b/.eslintrc @@ -20,3 +20,4 @@ rules: no-confusing-arrow: 0 no-case-declarations: 0 arrow-parens: [2, "as-needed"] + prefer-default-export: 0 \ No newline at end of file diff --git a/examples/basic/jsconfig.json b/examples/basic/jsconfig.json new file mode 100644 index 00000000..b003f794 --- /dev/null +++ b/examples/basic/jsconfig.json @@ -0,0 +1,9 @@ +{ + "compilerOptions": { + "baseUrl": "./src" + }, + "exclude": [ + "node_modules", + "public", + ] +} \ No newline at end of file diff --git a/jsconfig.json b/jsconfig.json new file mode 100644 index 00000000..b08336fa --- /dev/null +++ b/jsconfig.json @@ -0,0 +1,17 @@ +{ + "compilerOptions": { + "baseUrl": "./src", + "paths": { + "utils/*": [ + "utils/*", + ] + } + }, + "exclude": [ + "node_modules", + "dist", + "es", + "lib", + "coverage" + ] +} \ No newline at end of file diff --git a/src/actions/firestore.js b/src/actions/firestore.js index 83d4cfbd..1161d061 100644 --- a/src/actions/firestore.js +++ b/src/actions/firestore.js @@ -10,7 +10,9 @@ import { getQueryConfig, getQueryName, firestoreRef, + promisesForPopulate, } from '../utils/query'; +import { to } from '../utils/async'; const pathListenerCounts = {}; @@ -26,7 +28,7 @@ const pathListenerCounts = {}; export function add(firebase, dispatch, queryOption, ...args) { const meta = getQueryConfig(queryOption); return wrapInDispatch(dispatch, { - ref: firestoreRef(firebase, dispatch, meta), + ref: firestoreRef(firebase, meta), method: 'add', meta, args, @@ -53,7 +55,7 @@ export function add(firebase, dispatch, queryOption, ...args) { export function set(firebase, dispatch, queryOption, ...args) { const meta = getQueryConfig(queryOption); return wrapInDispatch(dispatch, { - ref: firestoreRef(firebase, dispatch, meta), + ref: firestoreRef(firebase, meta), method: 'set', meta, args, @@ -84,7 +86,7 @@ export function get(firebase, dispatch, queryOption) { } = firebase._.config || {}; return wrapInDispatch(dispatch, { - ref: firestoreRef(firebase, dispatch, meta), + ref: firestoreRef(firebase, meta), method: 'get', meta, types: [ @@ -117,7 +119,7 @@ export function get(firebase, dispatch, queryOption) { export function update(firebase, dispatch, queryOption, ...args) { const meta = getQueryConfig(queryOption); return wrapInDispatch(dispatch, { - ref: firestoreRef(firebase, dispatch, meta), + ref: firestoreRef(firebase, meta), method: 'update', meta, args, @@ -155,7 +157,7 @@ export function deleteRef(firebase, dispatch, queryOption) { return Promise.reject(new Error('Only documents can be deleted.')); } return wrapInDispatch(dispatch, { - ref: firestoreRef(firebase, dispatch, meta), + ref: firestoreRef(firebase, meta), method: 'delete', meta, types: [ @@ -199,6 +201,41 @@ function docChangeEvent(change, originalMeta = {}) { }; } +async function dispatchListenerResponse({ dispatch, docData, meta, firebase }) { + const { + mergeOrdered, + mergeOrderedDocUpdates, + mergeOrderedCollectionUpdates, + } = + firebase._.config || {}; + const docChanges = + typeof docData.docChanges === 'function' + ? docData.docChanges() + : docData.docChanges; + // Dispatch different actions for doc changes (only update doc(s) by key) + if (docChanges && docChanges.length < docData.size) { + // Loop to dispatch for each change if there are multiple + // TODO: Option for dispatching multiple changes in single action + docChanges.forEach(change => { + dispatch(docChangeEvent(change, meta)); + }); + } else { + // Dispatch action for whole collection change + dispatch({ + type: actionTypes.LISTENER_RESPONSE, + meta, + payload: { + data: dataByIdSnapshot(docData), + ordered: orderedFromSnap(docData), + }, + merge: { + docs: mergeOrdered && mergeOrderedDocUpdates, + collections: mergeOrdered && mergeOrderedCollectionUpdates, + }, + }); + } +} + /** * Set listener to Cloud Firestore with the call to the Firebase library * being wrapped in action dispatches.. Internall calls Firebase's onSnapshot() @@ -215,45 +252,59 @@ function docChangeEvent(change, originalMeta = {}) { */ export function setListener(firebase, dispatch, queryOpts, successCb, errorCb) { const meta = getQueryConfig(queryOpts); - const { - mergeOrdered, - mergeOrderedDocUpdates, - mergeOrderedCollectionUpdates, - } = - firebase._.config || {}; + // Create listener - const unsubscribe = firestoreRef(firebase, dispatch, meta).onSnapshot( - docData => { - const docChanges = - typeof docData.docChanges === 'function' - ? docData.docChanges() - : docData.docChanges; - // Dispatch different actions for doc changes (only update doc(s) by key) - if (docChanges && docChanges.length < docData.size) { - // Loop to dispatch for each change if there are multiple - // TODO: Option for dispatching multiple changes in single action - docChanges.forEach(change => { - dispatch(docChangeEvent(change, meta)); - }); - } else { - // Dispatch action for whole collection change + const unsubscribe = firestoreRef(firebase, meta).onSnapshot( + async docData => { + if (!meta.populates) { + dispatchListenerResponse({ dispatch, docData, meta, firebase }); + // Invoke success callback if it exists + if (typeof successCb === 'function') successCb(docData); + return; + } + const [populateErr, populateResults] = await to( + promisesForPopulate( + firebase, + docData.id, + dataByIdSnapshot(docData), + meta.populates, + ), + ); + if (populateErr) { + console.error('Error with populate:', populateErr, meta); // eslint-disable-line no-console + if (typeof errorCb === 'function') errorCb(populateErr); + throw populateErr; + } + // Dispatch for each child collection + Object.keys(populateResults).forEach(resultKey => { dispatch({ type: actionTypes.LISTENER_RESPONSE, - meta, + // TODO: Handle population of subcollection queries + meta: { collection: resultKey }, payload: { - data: dataByIdSnapshot(docData), - ordered: orderedFromSnap(docData), - }, - merge: { - docs: mergeOrdered && mergeOrderedDocUpdates, - collections: mergeOrdered && mergeOrderedCollectionUpdates, + data: populateResults[resultKey], + // TODO: Write ordered here }, + timestamp: Date.now(), + requesting: false, + requested: true, }); - } - // Invoke success callback if it exists - if (successCb) successCb(docData); + }); + // Dispatch results + dispatchListenerResponse({ + dispatch, + docData, + meta, + firebase, + }); }, err => { + const { + mergeOrdered, + mergeOrderedDocUpdates, + mergeOrderedCollectionUpdates, + } = + firebase._.config || {}; // TODO: Look into whether listener is automatically removed in all cases // Log error handling the case of it not existing const { logListenerError, preserveOnListenerError } = @@ -274,6 +325,8 @@ export function setListener(firebase, dispatch, queryOpts, successCb, errorCb) { }, ); attachListener(firebase, dispatch, meta, unsubscribe); + + return unsubscribe; } /** diff --git a/src/utils/async.js b/src/utils/async.js new file mode 100644 index 00000000..68af8673 --- /dev/null +++ b/src/utils/async.js @@ -0,0 +1,9 @@ +/* eslint-disable import/prefer-default-export */ +/** + * Async await wrapper for easy error handling + * @param {Promise} promise - Promise to wrap responses of + * @return {Promise} Resolves and rejects with an array + */ +export function to(promise) { + return promise.then(data => [null, data]).catch(err => [err]); +} diff --git a/src/utils/query.js b/src/utils/query.js index 519f1470..bb3e8c59 100644 --- a/src/utils/query.js +++ b/src/utils/query.js @@ -1,13 +1,18 @@ import { - isObject, isString, isArray, + isFunction, + isObject, size, trim, forEach, has, - isFunction, + map, + get, + set, + some, } from 'lodash'; + import { actionTypes } from '../constants'; /** @@ -106,7 +111,7 @@ function handleSubcollections(ref, subcollectionList) { * @param {Array} meta.where - List of argument arrays * @return {firebase.firestore.Reference} Resolves with results of add call */ -export function firestoreRef(firebase, dispatch, meta) { +export function firestoreRef(firebase, meta) { if (!firebase.firestore) { throw new Error('Firestore must be required and initalized.'); } @@ -402,3 +407,163 @@ export function dataByIdSnapshot(snap) { } return size(data) ? data : null; } + +/** + * @private + * @description Create an array of promises for population of an object or list + * @param {Object} firebase - Internal firebase object + * @param {Object} populate - Object containing root to be populate + * @param {Object} populate.root - Firebase root path from which to load populate item + * @param {String} id - String id + */ +export function getPopulateChild(firebase, populate, id) { + return firestoreRef(firebase, { collection: populate.root, doc: id }) + .get() + .then(snap => snap.data()); +} + +/** + * @private + * @description Populate list of data + * @param {Object} firebase - Internal firebase object + * @param {Object} originalObj - Object to have parameter populated + * @param {Object} populate - Object containing populate information + * @param {Object} results - Object containing results of population from other populates + */ +export function populateList(firebase, list, p, results) { + // Handle root not being defined + if (!results[p.root]) { + set(results, p.root, {}); + } + return Promise.all( + map(list, (id, childKey) => { + // handle list of keys + const populateKey = id === true || p.populateByKey ? childKey : id; + return getPopulateChild(firebase, p, populateKey).then(pc => { + if (pc) { + // write child to result object under root name if it is found + return set(results, `${p.root}.${populateKey}`, pc); + } + return results; + }); + }), + ); +} + +/** + * @private + * @description Create standardized populate object from strings or objects + * @param {String|Object} str - String or Object to standardize into populate object + */ +function getPopulateObj(str) { + if (!isString(str)) { + return str; + } + const strArray = str.split(':'); + // TODO: Handle childParam + return { child: strArray[0], root: strArray[1] }; +} + +/** + * @private + * @description Create standardized populate object from strings or objects + * @param {String|Object} str - String or Object to standardize into populate object + */ +function getPopulateObjs(arr) { + if (!isArray(arr)) { + return arr; + } + return arr.map(o => (isObject(o) ? o : getPopulateObj(o))); +} + +/** + * @private + * @description Create an array of promises for population of an object or list + * @param {Object} firebase - Internal firebase object + * @param {Object} originalObj - Object to have parameter populated + * @param {Object} populateString - String containg population data + */ +export function promisesForPopulate( + firebase, + dataKey, + originalData, + populatesIn, +) { + // TODO: Handle selecting of parameter to populate with (i.e. displayName of users/user) + const promisesArray = []; + const results = {}; + + // test if data is a single object, try generating populates and looking for the child + const populatesForData = getPopulateObjs( + isFunction(populatesIn) ? populatesIn(dataKey, originalData) : populatesIn, + ); + + const dataHasPopulateChilds = some(populatesForData, populate => + has(originalData, populate.child), + ); + + if (dataHasPopulateChilds) { + // Data is a single object, resolve populates directly + forEach(populatesForData, p => { + if (isString(get(originalData, p.child))) { + return promisesArray.push( + getPopulateChild(firebase, p, get(originalData, p.child)).then(v => { + // write child to result object under root name if it is found + if (v) { + set(results, `${p.root}.${get(originalData, p.child)}`, v); + } + }), + ); + } + + // Single Parameter is list + return promisesArray.push( + populateList(firebase, get(originalData, p.child), p, results), + ); + }); + } else { + // Data is a list of objects, each value has parameters to be populated + // { '1': {someobject}, '2': {someobject} } + forEach(originalData, (d, key) => { + // generate populates for this data item if a fn was passed + const populatesForDataItem = getPopulateObj( + isFunction(populatesIn) ? populatesIn(key, d) : populatesIn, + ); + + // resolve each populate for this data item + forEach(populatesForDataItem, p => { + // get value of parameter to be populated (key or list of keys) + const idOrList = get(d, p.child); + + // Parameter/child of list item does not exist + if (!idOrList) { + return; + } + + // Parameter of each list item is single ID + if (isString(idOrList)) { + return promisesArray.push( // eslint-disable-line + getPopulateChild(firebase, p, idOrList).then(v => { + // write child to result object under root name if it is found + if (v) { + set(results, `${p.root}.${idOrList}`, v); + } + return results; + }), + ); + } + + // Parameter of each list item is a list of ids + if (isArray(idOrList) || isObject(idOrList)) { + // Create single promise that includes a promise for each child + return promisesArray.push(// eslint-disable-line + populateList(firebase, idOrList, p, results), + ); + } + }); + }); + } + + // Return original data after population promises run + return Promise.all(promisesArray).then(() => results); +} From 058bf1cf86e5315e9f4d18dd16aee896d2d1d3bb Mon Sep 17 00:00:00 2001 From: Scott Prue Date: Tue, 30 Oct 2018 18:06:12 -0700 Subject: [PATCH 11/12] Tests fixed. --- test/unit/utils/query.spec.js | 48 ++++++++++++++++---------------- test/unit/utils/reducers.spec.js | 27 ++++++++++++------ 2 files changed, 43 insertions(+), 32 deletions(-) diff --git a/test/unit/utils/query.spec.js b/test/unit/utils/query.spec.js index 2f6a345e..eb7ba367 100644 --- a/test/unit/utils/query.spec.js +++ b/test/unit/utils/query.spec.js @@ -369,7 +369,7 @@ describe('query utils', () => { fakeFirebase = { firestore: () => ({ collection: () => ({ doc: docSpy }) }), }; - result = firestoreRef(fakeFirebase, dispatch, meta); + result = firestoreRef(fakeFirebase, meta); expect(result).to.be.an('object'); expect(docSpy).to.be.calledWith(meta.doc); }); @@ -391,7 +391,7 @@ describe('query utils', () => { }), }), }; - expect(() => firestoreRef(fakeFirebase, dispatch, meta)).to.throw( + expect(() => firestoreRef(fakeFirebase, meta)).to.throw( `Collection can only be run on a document. Check that query config for subcollection: "${subcollection}" contains a doc parameter.`, ); }); @@ -411,7 +411,7 @@ describe('query utils', () => { }), }), }; - result = firestoreRef(fakeFirebase, dispatch, meta); + result = firestoreRef(fakeFirebase, meta); expect(result).to.be.an('object'); // expect(docSpy).to.be.calledOnce(meta.subcollections[0].collection); }); @@ -442,7 +442,7 @@ describe('query utils', () => { }), }), }; - result = firestoreRef(fakeFirebase, dispatch, meta); + result = firestoreRef(fakeFirebase, meta); expect(result).to.be.an('object'); expect(result).to.be.deep.equal({ doc: 'data' }); expect(collectionSpy).to.be.calledOnce; @@ -463,7 +463,7 @@ describe('query utils', () => { }), }), }; - result = firestoreRef(fakeFirebase, dispatch, meta); + result = firestoreRef(fakeFirebase, meta); expect(docSpy).to.be.calledWith(meta.subcollections[0].doc); }); @@ -487,7 +487,7 @@ describe('query utils', () => { }), }), }; - result = firestoreRef(fakeFirebase, dispatch, meta); + result = firestoreRef(fakeFirebase, meta); expect(docSpy).to.be.calledWith(meta.subcollections[0].doc); expect(whereSpy).to.be.calledWith(testVal); }); @@ -512,7 +512,7 @@ describe('query utils', () => { }), }), }; - result = firestoreRef(fakeFirebase, dispatch, meta); + result = firestoreRef(fakeFirebase, meta); expect(result).to.be.an('object'); expect(orderBySpy).to.be.calledWith(meta.subcollections[0].orderBy); }); @@ -538,7 +538,7 @@ describe('query utils', () => { }), }), }; - result = firestoreRef(fakeFirebase, dispatch, meta); + result = firestoreRef(fakeFirebase, meta); expect(result).to.be.an('object'); expect(limitSpy).to.be.calledWith(meta.subcollections[0].limit); }); @@ -547,7 +547,7 @@ describe('query utils', () => { describe('startAt', () => { it('calls startAt if valid', () => { const { theFirebase, theSpy, theMeta } = fakeFirebaseWith('startAt'); - result = firestoreRef(theFirebase, dispatch, theMeta); + result = firestoreRef(theFirebase, theMeta); expect(result).to.be.an('object'); expect(theSpy).to.be.calledWith(theMeta.subcollections[0].startAt); }); @@ -558,7 +558,7 @@ describe('query utils', () => { const { theFirebase, theSpy, theMeta } = fakeFirebaseWith( 'startAfter', ); - result = firestoreRef(theFirebase, dispatch, theMeta); + result = firestoreRef(theFirebase, theMeta); expect(result).to.be.an('object'); expect(theSpy).to.be.calledWith(theMeta.subcollections[0].startAfter); }); @@ -574,7 +574,7 @@ describe('query utils', () => { ], }; const { theFirebase, theSpy } = fakeFirebaseWith('endAt'); - result = firestoreRef(theFirebase, dispatch, meta); + result = firestoreRef(theFirebase, meta); expect(result).to.be.an('object'); expect(theSpy).to.be.calledWith(meta.subcollections[0].endAt); }); @@ -590,7 +590,7 @@ describe('query utils', () => { ], }; const { theFirebase, theSpy } = fakeFirebaseWith('endBefore'); - result = firestoreRef(theFirebase, dispatch, meta); + result = firestoreRef(theFirebase, meta); expect(result).to.be.an('object'); expect(theSpy).to.be.calledWith(meta.subcollections[0].endBefore); }); @@ -604,7 +604,7 @@ describe('query utils', () => { fakeFirebase = { firestore: () => ({ collection: () => ({ where: whereSpy }) }), }; - result = firestoreRef(fakeFirebase, dispatch, meta); + result = firestoreRef(fakeFirebase, meta); expect(result).to.be.an('object'); expect(whereSpy).to.be.calledWith(meta.where[0]); }); @@ -615,7 +615,7 @@ describe('query utils', () => { fakeFirebase = { firestore: () => ({ collection: () => ({ where: whereSpy }) }), }; - result = firestoreRef(fakeFirebase, dispatch, meta); + result = firestoreRef(fakeFirebase, meta); expect(result).to.be.an('object'); expect(whereSpy).to.be.calledOnce; }); @@ -625,7 +625,7 @@ describe('query utils', () => { fakeFirebase = { firestore: () => ({ collection: () => ({ where: () => ({}) }) }), }; - expect(() => firestoreRef(fakeFirebase, dispatch, meta)).to.throw( + expect(() => firestoreRef(fakeFirebase, meta)).to.throw( 'where parameter must be an array.', ); }); @@ -636,7 +636,7 @@ describe('query utils', () => { fakeFirebase = { firestore: () => ({ collection: () => ({ where: whereSpy }) }), }; - expect(() => firestoreRef(fakeFirebase, dispatch, meta)).to.throw( + expect(() => firestoreRef(fakeFirebase, meta)).to.throw( 'where parameter must be an array.', ); }); @@ -649,7 +649,7 @@ describe('query utils', () => { fakeFirebase = { firestore: () => ({ collection: () => ({ orderBy: orderBySpy }) }), }; - result = firestoreRef(fakeFirebase, dispatch, meta); + result = firestoreRef(fakeFirebase, meta); expect(result).to.be.an('object'); expect(orderBySpy).to.be.calledWith(meta.orderBy[0]); }); @@ -660,7 +660,7 @@ describe('query utils', () => { fakeFirebase = { firestore: () => ({ collection: () => ({ orderBy: orderBySpy }) }), }; - result = firestoreRef(fakeFirebase, dispatch, meta); + result = firestoreRef(fakeFirebase, meta); expect(result).to.be.an('object'); expect(orderBySpy).to.be.calledWith(meta.orderBy[0][0]); }); @@ -670,7 +670,7 @@ describe('query utils', () => { fakeFirebase = { firestore: () => ({ collection: () => ({ orderBy: () => ({}) }) }), }; - expect(() => firestoreRef(fakeFirebase, dispatch, meta)).to.throw( + expect(() => firestoreRef(fakeFirebase, meta)).to.throw( 'orderBy parameter must be an array or string.', ); }); @@ -683,7 +683,7 @@ describe('query utils', () => { fakeFirebase = { firestore: () => ({ collection: () => ({ limit: limitSpy }) }), }; - result = firestoreRef(fakeFirebase, dispatch, meta); + result = firestoreRef(fakeFirebase, meta); expect(result).to.be.an('object'); expect(limitSpy).to.be.calledWith(meta.limit); }); @@ -696,7 +696,7 @@ describe('query utils', () => { fakeFirebase = { firestore: () => ({ collection: () => ({ startAt: startAtSpy }) }), }; - result = firestoreRef(fakeFirebase, dispatch, meta); + result = firestoreRef(fakeFirebase, meta); expect(result).to.be.an('object'); expect(startAtSpy).to.be.calledWith(meta.startAt); }); @@ -711,7 +711,7 @@ describe('query utils', () => { collection: () => ({ startAfter: startAfterSpy }), }), }; - result = firestoreRef(fakeFirebase, dispatch, meta); + result = firestoreRef(fakeFirebase, meta); expect(result).to.be.an('object'); expect(startAfterSpy).to.be.calledWith(meta.startAfter); }); @@ -724,7 +724,7 @@ describe('query utils', () => { fakeFirebase = { firestore: () => ({ collection: () => ({ endAt: endAtSpy }) }), }; - result = firestoreRef(fakeFirebase, dispatch, meta); + result = firestoreRef(fakeFirebase, meta); expect(result).to.be.an('object'); expect(endAtSpy).to.be.calledWith(meta.endAt); }); @@ -739,7 +739,7 @@ describe('query utils', () => { collection: () => ({ endBefore: endBeforeSpy }), }), }; - result = firestoreRef(fakeFirebase, dispatch, meta); + result = firestoreRef(fakeFirebase, meta); expect(result).to.be.an('object'); expect(endBeforeSpy).to.be.calledWith(meta.endBefore); }); diff --git a/test/unit/utils/reducers.spec.js b/test/unit/utils/reducers.spec.js index 526b7c65..e500f589 100644 --- a/test/unit/utils/reducers.spec.js +++ b/test/unit/utils/reducers.spec.js @@ -59,13 +59,13 @@ describe('reducer utils', () => { }); it('returns collection if provided', () => { - expect(pathFromMeta({ collection: 'test' })).to.equal('test'); + expect(pathFromMeta({ collection: 'test' })).to.have.property(0, 'test'); }); it('returns collection doc combined into dot path if both provided', () => { - expect(pathFromMeta({ collection: 'first', doc: 'second' })).to.equal( - 'first.second', - ); + const result = pathFromMeta({ collection: 'first', doc: 'second' }); + expect(result).to.have.property(0, 'first'); + expect(result).to.have.property(1, 'second'); }); it('uses storeAs as path if provided', () => { @@ -102,7 +102,7 @@ describe('reducer utils', () => { testId, () => 'test', ); - expect(result[0]).to.equal('test'); + expect(result).to.have.property(0, 'test'); }); }); @@ -110,13 +110,20 @@ describe('reducer utils', () => { it('with collection', () => { subcollections = [{ collection: 'third' }]; config = { collection: 'first', doc: 'second', subcollections }; - expect(pathFromMeta(config)).to.equal('first.second.third'); + const result = pathFromMeta(config); + expect(result).to.have.property(0, 'first'); + expect(result).to.have.property(1, 'second'); + expect(result).to.have.property(2, 'third'); }); it('with doc', () => { subcollections = [{ collection: 'third', doc: 'forth' }]; config = { collection: 'first', doc: 'second', subcollections }; - expect(pathFromMeta(config)).to.equal('first.second.third.forth'); + const result = pathFromMeta(config); + expect(result).to.have.property(0, 'first'); + expect(result).to.have.property(1, 'second'); + expect(result).to.have.property(2, 'third'); + expect(result).to.have.property(3, 'forth'); }); }); @@ -126,7 +133,11 @@ describe('reducer utils', () => { { collection: 'fifth' }, ]; config = { collection: 'first', doc: 'second', subcollections }; - expect(pathFromMeta(config)).to.equal('first.second.third.forth.fifth'); + const result = pathFromMeta(config); + expect(result).to.have.property(0, 'first'); + expect(result).to.have.property(1, 'second'); + expect(result).to.have.property(2, 'third'); + expect(result).to.have.property(4, 'fifth'); }); }); From 15eac10e1b31cedcd2711992c679aed9b71fd8a9 Mon Sep 17 00:00:00 2001 From: Scott Prue Date: Thu, 8 Nov 2018 01:03:04 -0800 Subject: [PATCH 12/12] Move over utils. Add docs on populate. --- README.md | 84 +++++++++++++++++++++++++++-- src/actions/firestore.js | 114 ++++++++------------------------------- src/utils/query.js | 105 ++++++++++++++++++++++++++++++++++-- 3 files changed, 203 insertions(+), 100 deletions(-) diff --git a/README.md b/README.md index ce0f35fa..27d65c9f 100644 --- a/README.md +++ b/README.md @@ -114,7 +114,7 @@ const enhance = compose( props.store.firestore.add('todos', { ...newTodo, owner: 'Anonymous' }), }), lifecycle({ - componentWillMount(props) { + componentDidMount(props) { props.loadData() } }), @@ -142,7 +142,7 @@ class Todos extends Component { store: PropTypes.object.isRequired } - componentWillMount () { + componentDidMount () { const { firestore } = this.context.store firestore.get('todos') } @@ -447,16 +447,16 @@ Storing data under a different path within redux is as easy as passing the `stor Other Firebase statics (such as [FieldValue](https://firebase.google.com/docs/reference/js/firebase.firestore.FieldValue)) are available through the firestore instance: ```js +import PropTypes from 'prop-types' import { connect } from 'react-redux' import { compose, withHandlers, - lifecycle, withContext, getContext } from 'recompose' -const withFirestore = compose( +const withStore = compose( withContext({ store: PropTypes.object }, () => {}), getContext({ store: PropTypes.object }), ) @@ -477,6 +477,82 @@ const enhance = compose( export default enhance(SomeComponent) ``` +### Population +Population, made popular in [react-redux-firebase](http://react-redux-firebase.com/docs/recipes/populate.html), also works with firestore. + + +#### Automatic Listeners +```js +import { connect } from 'react-redux' +import { firestoreConnect, populate } from 'react-redux-firebase' +import { + compose, + withHandlers, + lifecycle, + withContext, + getContext +} from 'recompose' + +const populates = [{ child: 'createdBy', root: 'users' }] +const collection = 'projects' + +const withPopulatedProjects = compose( + firestoreConnect((props) => [ + { + collection, + populates + } + ]), + connect((state, props) => ({ + projects: populate(state.firestore, collection, populates) + })) +) +``` + +#### Manually using setListeners +```js +import { withFirestore, populate } from 'react-redux-firebase' +import { connect } from 'react-redux' +import { compose, lifecycle } from 'recompose' + +const collection = 'projects' +const populates = [{ child: 'createdBy', root: 'users' }] + +const enhance = compose( + withFirestore, + lifecycle({ + componentDidMount() { + this.props.firestore.setListener({ collection, populates }) + } + }), + connect(({ firestore }) => ({ // state.firestore + todos: firestore.ordered.todos, + })) +) +``` + +#### Manually using get +```js +import { withFirestore, populate } from 'react-redux-firebase' +import { connect } from 'react-redux' +import { compose, lifecycle } from 'recompose' + +const collection = 'projects' +const populates = [{ child: 'createdBy', root: 'users' }] + +const enhance = compose( + withFirestore, + lifecycle({ + componentDidMount() { + this.props.store.firestore.get({ collection, populates }) + } + }), + connect(({ firestore }) => ({ // state.firestore + todos: firestore.ordered.todos, + })) +) +``` + ## Config Options Optional configuration options for redux-firestore, provided to reduxFirestore enhancer as optional second argument. Combine any of them together in an object. diff --git a/src/actions/firestore.js b/src/actions/firestore.js index b01a046c..a8e3a8bf 100644 --- a/src/actions/firestore.js +++ b/src/actions/firestore.js @@ -9,7 +9,8 @@ import { getQueryConfig, getQueryName, firestoreRef, - promisesForPopulate, + dispatchListenerResponse, + getPopulateActions, } from '../utils/query'; import { to } from '../utils/async'; @@ -170,71 +171,6 @@ export function deleteRef(firebase, dispatch, queryOption) { }); } -const changeTypeToEventType = { - added: actionTypes.DOCUMENT_ADDED, - removed: actionTypes.DOCUMENT_REMOVED, - modified: actionTypes.DOCUMENT_MODIFIED, -}; - -/** - * Action creator for document change event. Used to create action objects - * to be passed to dispatch. - * @param {Object} change - Document change object from Firebase callback - * @param {Object} [originalMeta={}] - Original meta data of action - * @return {Object} [description] - */ -function docChangeEvent(change, originalMeta = {}) { - const meta = { ...originalMeta, path: change.doc.ref.path }; - if (originalMeta.subcollections && !originalMeta.storeAs) { - meta.subcollections[0] = { ...meta.subcollections[0], doc: change.doc.id }; - } else { - meta.doc = change.doc.id; - } - return { - type: changeTypeToEventType[change.type] || actionTypes.DOCUMENT_MODIFIED, - meta, - payload: { - data: change.doc.data(), - ordered: { oldIndex: change.oldIndex, newIndex: change.newIndex }, - }, - }; -} - -async function dispatchListenerResponse({ dispatch, docData, meta, firebase }) { - const { - mergeOrdered, - mergeOrderedDocUpdates, - mergeOrderedCollectionUpdates, - } = - firebase._.config || {}; - const docChanges = - typeof docData.docChanges === 'function' - ? docData.docChanges() - : docData.docChanges; - // Dispatch different actions for doc changes (only update doc(s) by key) - if (docChanges && docChanges.length < docData.size) { - // Loop to dispatch for each change if there are multiple - // TODO: Option for dispatching multiple changes in single action - docChanges.forEach(change => { - dispatch(docChangeEvent(change, meta)); - }); - } else { - // Dispatch action for whole collection change - dispatch({ - type: actionTypes.LISTENER_RESPONSE, - meta, - payload: { - data: dataByIdSnapshot(docData), - ordered: orderedFromSnap(docData), - }, - merge: { - docs: mergeOrdered && mergeOrderedDocUpdates, - collections: mergeOrdered && mergeOrderedCollectionUpdates, - }, - }); - } -} - /** * Set listener to Cloud Firestore with the call to the Firebase library * being wrapped in action dispatches.. Internall calls Firebase's onSnapshot() @@ -255,47 +191,39 @@ export function setListener(firebase, dispatch, queryOpts, successCb, errorCb) { // Create listener const unsubscribe = firestoreRef(firebase, meta).onSnapshot( async docData => { + // Dispatch directly if no populates if (!meta.populates) { dispatchListenerResponse({ dispatch, docData, meta, firebase }); // Invoke success callback if it exists if (typeof successCb === 'function') successCb(docData); return; } - const [populateErr, populateResults] = await to( - promisesForPopulate( - firebase, - docData.id, - dataByIdSnapshot(docData), - meta.populates, - ), + + // Run population and dispatch results + const [populateErr, populateActions] = await to( + getPopulateActions({ firebase, docData, meta }), ); + + // Handle errors in population if (populateErr) { - console.error('Error with populate:', populateErr, meta); // eslint-disable-line no-console + if (firebase._.config.logListenerError) { + // Log error handling the case of it not existing + invoke(console, 'error', `Error populating:`, populateErr); + } if (typeof errorCb === 'function') errorCb(populateErr); - throw populateErr; + return; } - // Dispatch for each child collection - Object.keys(populateResults).forEach(resultKey => { + + // Dispatch each populate action + populateActions.forEach(populateAction => { dispatch({ + ...populateAction, type: actionTypes.LISTENER_RESPONSE, - // TODO: Handle population of subcollection queries - meta: { collection: resultKey }, - payload: { - data: populateResults[resultKey], - // TODO: Write ordered here - }, timestamp: Date.now(), - requesting: false, - requested: true, }); }); - // Dispatch results - dispatchListenerResponse({ - dispatch, - docData, - meta, - firebase, - }); + // Dispatch original action + dispatchListenerResponse({ dispatch, docData, meta, firebase }); }, err => { const { @@ -320,7 +248,7 @@ export function setListener(firebase, dispatch, queryOpts, successCb, errorCb) { preserve: preserveOnListenerError, }); // Invoke error callback if it exists - if (errorCb) errorCb(err); + if (typeof errorCb === 'function') errorCb(err); }, ); attachListener(firebase, dispatch, meta, unsubscribe); diff --git a/src/utils/query.js b/src/utils/query.js index bb3e8c59..f2579119 100644 --- a/src/utils/query.js +++ b/src/utils/query.js @@ -12,7 +12,7 @@ import { set, some, } from 'lodash'; - +import { to } from '../utils/async'; import { actionTypes } from '../constants'; /** @@ -501,7 +501,6 @@ export function promisesForPopulate( const dataHasPopulateChilds = some(populatesForData, populate => has(originalData, populate.child), ); - if (dataHasPopulateChilds) { // Data is a single object, resolve populates directly forEach(populatesForData, p => { @@ -556,7 +555,7 @@ export function promisesForPopulate( // Parameter of each list item is a list of ids if (isArray(idOrList) || isObject(idOrList)) { // Create single promise that includes a promise for each child - return promisesArray.push(// eslint-disable-line + return promisesArray.push( // eslint-disable-line populateList(firebase, idOrList, p, results), ); } @@ -567,3 +566,103 @@ export function promisesForPopulate( // Return original data after population promises run return Promise.all(promisesArray).then(() => results); } + +const changeTypeToEventType = { + added: actionTypes.DOCUMENT_ADDED, + removed: actionTypes.DOCUMENT_REMOVED, + modified: actionTypes.DOCUMENT_MODIFIED, +}; + +/** + * Action creator for document change event. Used to create action objects + * to be passed to dispatch. + * @param {Object} change - Document change object from Firebase callback + * @param {Object} [originalMeta={}] - Original meta data of action + * @return {Object} [description] + */ +function docChangeEvent(change, originalMeta = {}) { + const meta = { ...originalMeta, path: change.doc.ref.path }; + if (originalMeta.subcollections && !originalMeta.storeAs) { + meta.subcollections[0] = { ...meta.subcollections[0], doc: change.doc.id }; + } else { + meta.doc = change.doc.id; + } + return { + type: changeTypeToEventType[change.type] || actionTypes.DOCUMENT_MODIFIED, + meta, + payload: { + data: change.doc.data(), + ordered: { oldIndex: change.oldIndex, newIndex: change.newIndex }, + }, + }; +} + +export function dispatchListenerResponse({ + dispatch, + docData, + meta, + firebase, +}) { + const { + mergeOrdered, + mergeOrderedDocUpdates, + mergeOrderedCollectionUpdates, + } = + firebase._.config || {}; + const docChanges = + typeof docData.docChanges === 'function' + ? docData.docChanges() + : docData.docChanges; + // Dispatch different actions for doc changes (only update doc(s) by key) + if (docChanges && docChanges.length < docData.size) { + // Loop to dispatch for each change if there are multiple + // TODO: Option for dispatching multiple changes in single action + docChanges.forEach(change => { + dispatch(docChangeEvent(change, meta)); + }); + } else { + // Dispatch action for whole collection change + dispatch({ + type: actionTypes.LISTENER_RESPONSE, + meta, + payload: { + data: dataByIdSnapshot(docData), + ordered: orderedFromSnap(docData), + }, + merge: { + docs: mergeOrdered && mergeOrderedDocUpdates, + collections: mergeOrdered && mergeOrderedCollectionUpdates, + }, + }); + } +} + +export async function getPopulateActions({ firebase, docData, meta }) { + // Run promises for population + const [populateErr, populateResults] = await to( + promisesForPopulate( + firebase, + docData.id, + dataByIdSnapshot(docData), + meta.populates, + ), + ); + + // Handle errors populating + if (populateErr) { + console.error('Error with populate:', populateErr, meta); // eslint-disable-line no-console + throw populateErr; + } + + // Dispatch listener results for each child collection + return Object.keys(populateResults).map(resultKey => ({ + // TODO: Handle population of subcollection queries + meta: { collection: resultKey }, + payload: { + data: populateResults[resultKey], + // TODO: Write ordered here + }, + requesting: false, + requested: true, + })); +}