szaranger / firebase-saga

A library for connecting redux-saga middleware to Firebase.
58 stars 15 forks source link

`create` requires 'path' 2x? #6

Open brandonmp opened 8 years ago

brandonmp commented 8 years ago

using the create saga & wondering about the source code here (which is also the same source code for push?):

/**
 * Saves new data to the database with `set()`
 *
 * @param path
 * @param fn
 * @example
 * import { create } from 'firebase-saga';
 *
 * yield call(create, 'posts', () => ({
 *              [`posts/1234`]: {
 *                   title: 'My Second Post',
 *                   body: 'Second post details',
 *                   timestamp: +new Date
 *               }
 *           })
 *);
 */
export function* create(path, fn) {
    const key = yield call(newKey, path);
    const payload = yield call(fn, key);
    const opts = newOpts('error');
    const ref = firebase.database().ref();
    const [_, { error }] = yield [
        call([ref, ref.update], payload, opts.handler),
        take(opts)
    ];
    return error;
}

Why does the API require declaring 'posts' as the 2nd arg, and 'posts' in the prop name of the anon set function?

szaranger commented 8 years ago

@brandonmp Regarding your question around create and push, there are slightly different in sense that push passes path into the firebase reference where create doesn't.

const ref = firebase.database().ref(path);

Also both create and push wrap the consecutive Firebase helpers.

Why does the API require declaring 'posts' as the 2nd arg?

And 'posts' in the prop name of the anon set function?

create requires 'path' 2x?

brandonmp commented 8 years ago

okay, but path is only used in generating a new key (which a push operation). shouldn't the signature for the create saga be create(path, payload)?

It looks like the current flow goes like this:

  1. generate new push key
  2. pass key to the fn function that was passed to create (which isn't used in the example)
  3. make a ref, then run update with the payload

why not:

export function* create(path, payload /*obj*/) {
    const opts = newOpts('error');
    const ref = firebase.database().ref(path);
    const [_, { error }] = yield [
        call([ref, ref.set], payload, opts.handler),
        take(opts)
    ];
    return error;
}

then the signature is simply:

yield call(create, 'path/to/items/item#', payload)

this (a) avoids having to write a function that just returns an object, (b) avoids repeating the path twice, and (c) gives users the option to use set or update under the hood (since both operate differently)