Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Data fetching across multiple facets #1

Closed
idream3 opened this issue May 11, 2015 · 6 comments
Closed

Data fetching across multiple facets #1

idream3 opened this issue May 11, 2015 · 6 comments

Comments

@idream3
Copy link
Contributor

idream3 commented May 11, 2015

Hey @christianalfoni,

Cerebral looks fantastic! I don't have to learn Elm after all :)

A question that I raised on your blog site about data fetching inside facets is relevant to this library so I thought I should post it here.

What happens when you have multiple facets that rely on the same data path eg. users - where would you put put the signal call without having to duplicate code throughout your facets?

cerebral.facet('visibleTodos', ['todos', 'authors'], function (cerebral, uuid) {
   let todo = cerebral.get('todos')[uuid].toJS();
    todo.author = cerebral.get('authors')[todo.authorId];

    if (!todo.author) {
      todo.author = {
        id: todo.authorId,
        $isLoading: true
      };
      cerebral.signals.missingAuthor(todo.authorId);
    }

})

cerebral.facet('visibleNotes', ['notes', 'authors'], function (cerebral, uuid) {
    let note = cerebral.get('notes')[uuid].toJS();
    note.author = cerebral.get('authors')[todo.authorId];

    if (!notes.author) {
      notes.author = {
        id: notes.authorId,
        $isLoading: true
      };
      cerebral.signals.missingAuthor(notes.authorId);
    }

})

It would be nice if the data fetching was executed when the actual raw data is being accessed. In this case it is authors path.

I havn't thought this though completely, but what if we could do something like this:

// first argument is data which is returned from the get method.
// the rest of the arguments are the paths you pass to get method.
cerebral.watch('authors', function(data [, uuid, path2...]) {

  if (!data) {
    data = {
      id: uuid, 
      $isLoading: true
    };
    cerebral.signals.missingAuthor(uuid);
  }

  return data
})


// and this is how you would use it inside the facet or wherever
var author = cerebral.get('authors', uuid)
@idream3
Copy link
Contributor Author

idream3 commented May 11, 2015

more mind dumps...

It would actually need to be closer to this:

cerebral.watch('authors', function(data [, uuid, path2...]) {

 // ok, no data. We need to get it from server so add isLoading
  if (!data) {
    let data = {
      $isLoading: true
    };

    // we have an extra keypath so we need to be more specific
    if (uuid) {
      data.id: uuid
      cerebral.signals.missingAuthor(uuid);
      return data
    }

    // else we make the basic call
    cerebral.signals.missingAllAuthors();
    return data
  }

  return data
})


// then it would cater for each use case
var author = cerebral.get('authors', uuid)

var authors = cerebral.get('authors')

@christianalfoni
Copy link
Member

Hi @idream3 ,

You raise an important question here. What I value most of all is to keep the code readable and easy to compose in your head. Adding a concept of watch would make it harder to compose how the application works. You have your cerebral state, your maps and also this watcher. How they interoperate is just difficult I think, as they are very decoupled.

I do see your concern of keeping DRY code, but you could quite easily do something like:

var setOrGetAuthor = function (cerebral, id) {
    let author = cerebral.get('authors')[id];
    if (!author) {
     author = {
        id: id,
        $isLoading: true
      };
      cerebral.signals.missingAuthor(id);
    }
    return author;
};
cerebral.map('visibleTodos', ['todos', 'authors'], function (cerebral, uuid) {
   let todo = cerebral.get('todos')[uuid].toJS();
   todo.author = setOrGetAuthor(cerebral , todo.authorId);
})

And you could just reuse that function across all mapping.

I think we need more experience before implementing a brand new concept at least, but thanks a lot for the input :-)

@idream3
Copy link
Contributor Author

idream3 commented May 15, 2015

Ok, I agree that another concept abstraction is probably not the way to go.

Your code above illustrates nicely how you could inline for a single use case and wrap into a generic function for multiple cases. I would be inclined to have a general file called getOrFetch.

And then:

cerebral.map('visibleTodos', ['todos', 'authors'], function (cerebral, uuid) {
   let todo = cerebral.get('todos')[uuid].toJS();
   todo.author = getOrFetch.author(cerebral , todo.authorId);
  return todo
})

The benefit of leaving it how it is gives us developers more flexibility :)

This does, however, remind me of another concern I have about data fetching. Literally all of the data in my application is fetched from a server and I suspect this will be the case for most applications (or local storage etc...). Data fetching works well within Maps but how to fetch data for a single path elegantly?

Take this for example:

var state = {
  projects: {},
  users: {}
  templates: {}
  notes: {}
}

All data needs to be fetched from the server. In my current Flux implementation most of the requested data fetching is handled by the store if the data does not exist (exactly like the Map function above). You can do this because you define the get methods on the store like getProjects, getUsers etc...

But for single key paths in Cerebral how do you do this with out having a pseudo getter for each one:

Cerebral.Map('getProjects', ['projects'], (cerebral, state) => {
  if (!state.projects) return getOrFetch.projects();
  return state.projects
})

Hmmm, but then this wouldn't even work because you are concerned with the initial load of data and in this case 'getProjects' Map wouldn't even trigger because the 'projects' path will not fire a change when the (React) component is mounted.

Now then, I see two options as a start:

  1. add a signal into componentWillMount (initialize)
componentWillMount() {
  if (!this.state.projects) this.signals.missingProjects()
  if (!this.state.users) this.signals.missingUsers()
},

render() {
  if (!this.state.projects || !this.state.users) return <Loading/>
}

But it seems to me that the components should just ask for the data and not be concerned about requesting for missing data.

Also the code above is something that should go into Higher-order components. Perhaps we implement something like Baobab-react. (create a separate issue?).

  1. Implement something akin to my first comment - a function that runs after 'getting' a path.
Cerebral.afterGet('projects', value => {
  if (!value) signals.missingProjects()
  return value
})  

Or, perhaps we can use the Map here. The Map function runs when either of the paths have changed/update, maybe if the second paramter is a function it will run the Map function after the using the 'get' function.

Cerebral.Map('projects', value => {
  if (!value) signals.missingProjects()
  return value
})  

Not sure if any of this feels like the right way to go about it... I will keep experimenting with this aspect.

@christianalfoni
Copy link
Member

Hi @idream3 and again thanks for input :-)

In this scenario I would suggest this:

let getInitialData = function () {
  return Promise.all([getProjects, getTemplates, getUsers]);
};
cerebral.signal('applicationRendered`, getInitialData, setInitialData);

And this signal would trigger on the componentDidMount of the root component. This way you can reuse the getProjects etc. function on other signals. I am skeptic to creating too much automation as it is difficult to reason about when things trigger. I think it is an important concept in Cerebral that all state change is handled with a signal.

This actually got me to thinking:

cerebral.signal('applicationRendered`, [getProjects, getTemplates, getUsers], setInitialData);

It would be possible to indicate that these actions should run in parallell and when all are done, go to next action. What do you think?

Anyways, your suggestion on using map would also work.

@idream3
Copy link
Contributor Author

idream3 commented May 16, 2015

Great stuff, I like the parallel addition.

I actually think now that it does make sense for a component to send a signal on mount that it needs data if missing.

I am going to experiment with this in my higher order components and see how we go.

Thanks for your feedback :)

@christianalfoni
Copy link
Member

Cool! I will just close this for now. Create a new issue if new questions related to this pops up :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants