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

Todos not populated until browser refresh #93

Closed
jarbot opened this issue Mar 24, 2017 · 9 comments
Closed

Todos not populated until browser refresh #93

jarbot opened this issue Mar 24, 2017 · 9 comments
Assignees

Comments

@jarbot
Copy link

jarbot commented Mar 24, 2017

I'm using Google Sign-in for auth. This seems to be working as expected. When my app first launches the user must login to Google then the app renders after auth, but Todos are not populated until browser refresh or hot module reload.

I see this in the reactReduxFirebase log when the app starts w/ no auth:
react-redux-firebase.js:7425 event: /todos:cancel

After that events don't fire for todos, only user/profile updates.

You can see the full log before/after auth here: http://pastebin.com/whHWzrQX

Here is my App.js:

import React from 'react';
import './app.css';
import { connect } from 'react-redux';
import TopBar from './TopBar';
import BaseTextInput from './BaseTextInput';
import RegexHelper from '../helpers/RegexHelper';
import { firebaseConnect, isLoaded, isEmpty, dataToJS, pathToJS } from 'react-redux-firebase'
import _ from 'lodash';

class AppComponent extends React.Component {

  constructor(props, context) {
    super(props, context);
    this.onSubmit = this.onSubmit.bind(this);
  }

  onSubmit() {
  }

  render() {
    // todo: list existing filters
    let { firebase, todos } = this.props;
    todos = todos || [];
    return (
      <div className="content App">
        <TopBar actions={this.props.actions} auth={this.props.auth} />
        <form onSubmit={(e)=>{e.preventDefault();this.onSubmit();}}>
          <BaseTextInput
            type="text"
            name="audienceName"
            placeholder="Audience Name"
            validateComplete={RegexHelper.regexText}
            ignoreBlur={false}
            ref={(c) => this._audienceNameInput = c} />

          {todos.map((todo) => {
            return (
              <p key={todo}>{todo}</p>
            )
          })}
        </form>
      </div>
    );
  }
}

AppComponent.defaultProps = {
};

const wrappedTodos = firebaseConnect([
  'todos'
])(AppComponent)

export default connect(
  ({firebase}) => ({
    todos: dataToJS(firebase, 'todos'),
    auth: pathToJS(firebase, 'auth'),
  })
)(wrappedTodos)

Here is where I handle logins:

import React, {PropTypes} from 'react';
import './topbar.css';
import { connect } from 'react-redux';
import { firebaseConnect, isLoaded, isEmpty, dataToJS, pathToJS } from 'react-redux-firebase'
import _ from 'lodash';
import Dialog from './Dialog';

const logo = require('../images/logo.png');

class TopBar extends React.Component {

  constructor(props, context) {
    super(props, context);
    this.onLogout = this.onLogout.bind(this);
    this.onLogin = this.onLogin.bind(this);
    this.onOk = this.onOk.bind(this);
    this.isLoggedIn = false;
    this.loginError = false;
  }

  componentWillReceiveProps({ auth }) {
    this.isLoggedIn = !_.isEmpty(auth) && !_.isEmpty(auth.uid);
    if(this.isLoggedIn && !_.isEmpty(auth.providerData)) {
      console.log('user info: ', auth.providerData[0] );
    } else {
      console.log('user not logged in');
    }
  }

  onLogout() {
    this.props.firebase.logout();
    localStorage.clear();
  }

  onLogin() {
    this.props.firebase.login({
      provider: 'google',
      type: 'popup'
    }).then( result => {
      this.isLoggedIn = true;
      this.props.actions.loggedIn(result.providerData[0].uid, result.displayName, result.avatarUrl)
    }).catch( err => {
      // trigger logout action
      console.log('err: ', err);
      this.props.actions.loggedOut();
      this.loginError = err.message;
      this.isLoggedIn = false;
      this.forceUpdate();
    });
  }

  onOk() {
    this.errorDialog.close();
    this.loginError = "";
    this.forceUpdate();
  }

  render () {
    let { todos } = this.props;

    todos = todos || [];
    console.log('todos isLoaded: ', isLoaded(todos));
    console.log('todos: ', todos.length);

    if(this.loginError) {
      return (
        <div class="contnet">
          <Dialog
            className="small"
            title="Login Error"
            message={(
              <div>
                {this.loginError}

              </div>
            )}
            singleAction="Ok"
            yesAction={this.onOk}
            hideButtons={false}
            ref={c=>{this.errorDialog = c}}
          />
        </div>
      );
    }
    return (
      <div className="TopBar">
        <div className="row">
          <div className="logoContainer col">
            <img src={logo} alt="Flowplay Dashboard" />
          </div>
          <nav className="col">
            <ul>
              <li><button>FAKE</button></li>
              <li><button>FAKE</button></li>
              <li><button>FAKE</button></li>
            </ul>
          </nav>
        </div>
        <div className="loginStatus">
          <p>
            {this.props.localAuth.displayName} &nbsp;
            { this.isLoggedIn ?
              (<button className="small" onClick={this.onLogout}>logout</button>) :
              (<button className="small" onClick={this.onLogin}>google login</button>)
            }
          </p>
          {todos.map((todo) => {
            return (
              <p key={todo}>{todo}</p>
            )
          })}
        </div>
      </div>
    );
  }
}

//export default firebaseConnect()(TopBar)
TopBar.defaultProps = {
  localAuth: PropTypes.shape({}).isRequired
};

const wrappedTodos = firebaseConnect([
  'todos'
])(TopBar);

export default connect(
  ({firebase}) => ({
    todos: dataToJS(firebase, 'todos'),
    auth: pathToJS(firebase, 'auth'),
  })
)(wrappedTodos);

I'm rendering todos in both places for debugging purposes.

Here is my store setup:

import { createStore, compose } from 'redux'
import reducers from '../reducers';
import { reactReduxFirebase } from 'react-redux-firebase';


// Firebase config
var config = {
  apiKey: "fAkeKey",
  authDomain: "fake.firebaseapp.com",
  databaseURL: "https://fake.firebaseio.com",
  storageBucket: "fake.appspot.com",
  messagingSenderId: "blahblah"
};

export default function configureStore (initialState) {

  const createStoreWithFirebase = compose(
    reactReduxFirebase(
      config,
      { userProfile: 'users', enableLogging: true }
    ),
    typeof window === 'object' && typeof window.devToolsExtension !== 'undefined' ? window.devToolsExtension() : f => f
  )(createStore);

  const store = createStoreWithFirebase(reducers, initialState);

  if (module.hot) {
    // Enable Webpack hot module replacement for reducers
    module.hot.accept('../reducers', () => {
      // We need to require for hot reloading to work properly.
      const nextReducer = require('../reducers');  // eslint-disable-line global-require

      store.replaceReducer(nextReducer);
    });
  }

  return store;
}
@prescottprue
Copy link
Owner

prescottprue commented Mar 24, 2017

There are a few things this could be based on your implementation

Use this.setState

You will want to use state for things like this.isLoggedIn because your component will not update and re-render otherwise (This could also be causing your issue).

If you are updating state, you should use this.setState(). For instance use this.setState({ isLoggedIn: true }) if you need it passed into state (which you shouldn't because it will be available in props)

Props Passing and Multiple Connects

This may be due to an error in your handling of logins in the Todos. Either want to pass auth down or connect that component, you are doing both.

So either replace:

const wrappedTodos = firebaseConnect([
  'todos'
])(TopBar);

export default connect(
  ({firebase}) => ({
    todos: dataToJS(firebase, 'todos'),
    auth: pathToJS(firebase, 'auth'),
  })
)(wrappedTodos);

with

export default TopBar

or

keep the component connected and do not pass down an auth prop when calling the component:

 <TopBar actions={this.props.actions} /> // no longer passing auth

Try to closely follow the simple complete example.

Side Notes

  • You set this.isLoggedIn in multiple places. If it is conditionally changing, you might want to look into using a method or a static method that sets state using this.setState
  • Not sure what this is trying to accomplish: this.props.actions.loggedIn(result.providerData[0].uid, result.displayName, result.avatarUrl), but you shouldn't need to pass auth to anywhere else to make things work.
  • Looks like you are referencing the prop localAuth in a few places you will want to change that
  • You will probably want to use .then on logout before clearing local storage (guessing you have your own application stuff being stored in local storage?)

@jarbot
Copy link
Author

jarbot commented Mar 24, 2017

@prescottprue Thanks for all the details. I will clean up this.loggedIn stuff, I was just being lazy trying to get stuff to work. I created localAuth so that I had an easy (documented with my own code) way of accessing profile data for the user.

I actually did follow https://github.com/prescottprue/react-redux-firebase/tree/master/examples/complete/simple pretty closely, but it does not have auth setup.

After auth I notice that there is no request to firebase for /todos. I see a request for /users only.

@jarbot
Copy link
Author

jarbot commented Mar 27, 2017

I pulled down the simple complete example and changed to firebase config to point at my instance and added a login / logout interface. I'm able to repro the issue with this setup. I made minor changes to how the todos render, because my setup is just a simple array of strings. No other changes were made.

To repro with the existing firebase api key thats setup you will have to turn on google signon in firebase console.

import React, { Component, PropTypes } from 'react'
import { connect } from 'react-redux'
import {
  firebaseConnect,
  isLoaded,
  isEmpty,
  dataToJS
} from 'react-redux-firebase'

import logo from './logo.svg'
import TodoItem from './TodoItem'
import './App.css'

class App extends Component {
  static propTypes = {
    todos: PropTypes.array,
    firebase: PropTypes.shape({
      push: PropTypes.func.isRequired
    })
  }

  handleAdd = () => {
    const { firebase } = this.props
    const { newTodo } = this.refs
    firebase.push('/todos', { text: newTodo.value, done: false })
    newTodo.value = ''
  }

  login = () => {
    console.log('open login');
    this.props.firebase.login({
      provider: 'google',
      type: 'popup'
    }).then( result => {
      console.log('login complete');
      this.forceUpdate();
    }).catch( err => {
      // trigger logout action
      console.log('err: ', err);
   });
  }

  logout = () => {
    this.props.firebase.logout().then(() => {
      console.log('logout complete');
    });

  }

  render () {
    const { todos } = this.props

    console.log('todos;', todos)

    const todosList = (!isLoaded(todos))
                        ? 'Loading'
                        : (isEmpty(todos))
                          ? 'Todo list is empty'
                          : Object.keys(todos).map((key) => (
                            <TodoItem key={key} id={key} todo={todos[key]} />
                          ))
    return (
      <div className='App'>
        <div className='App-header'>
          <h2>react-redux-firebase demo</h2>
          <img src={logo} className='App-logo' alt='logo' />
          <button onClick={this.login}>login</button>
          <button onClick={this.logout}>logout</button>
        </div>
        <div className='App-todos'>
          <h4>
            Loaded From
            <span className='App-Url'>
              <a href='https://redux-firebasev3.firebaseio.com/'>
                redux-firebasev3.firebaseio.com
              </a>
            </span>
          </h4>
          <h4>Todos List</h4>
          {todosList}
          <h4>New Todo</h4>
          <input type='text' ref='newTodo' />
          <button onClick={this.handleAdd}>
            Add
          </button>
        </div>
      </div>
    )
  }
}
const fbWrappedComponent = firebaseConnect([
  '/todos'
  // { type: 'once', path: '/todos' } // for loading once instead of binding
  // '/todos#populate=owner:displayNames' // for populating owner parameter from id into string loaded from /displayNames root
  // '/todos#populate=collaborators:users' // for populating owner parameter from id to user object loaded from /users root
  // { path: 'todos', populates: [{ child: 'collaborators', root: 'users' }] } // object notation of population
  // '/todos#populate=owner:users:displayName' // for populating owner parameter from id within to displayName string from user object within users root
])(App)

export default connect(
  ({ firebase }) => ({
    todos: dataToJS(firebase, 'todos'),
  })
)(fbWrappedComponent)

@jarbot
Copy link
Author

jarbot commented Mar 27, 2017

I managed to work around this auth issue by separating the firebaseConntect for 'todos' into its own component. I don't load the todos component until after auth is complete.

@Absolutestunna
Copy link

Absolutestunna commented Apr 7, 2017

@jarbot Do you mind sharing how you ended up separating the todos component?

@jarbot
Copy link
Author

jarbot commented Apr 10, 2017

@Absolutestunna are you experiencing a similar issue? I'm starting to suspect my setup is the issue. Either some React.js, babel or webpack version incompatibility.

The firebase connect pattern does not trigger a re-render when the todo list changes. I'm thinking of manually getting the list now with this.props.firebase.ref('todos').once('value') and managing updates/refresh on my own.

The decorator pattern does not work in my stack either. This may have something to do with it.

To answer your question though I did something like so (this is untested pseudo code):

render() {
	let loggedIn = (this.props.auth && !_.isEmpty(this.props.auth.uid));
	let view = (<p>you need to login</p>);

	if(loggedIn) {
		view = (<yourTodoComponent />);
	}
	return ({view});
}

@prescottprue
Copy link
Owner

@jarbot Is there a reason you didn't want to use isLoaded and isEmpty that is provided by react-redux-firebase?

firebaseConnect re-rendering

firebaseConnect re-renders if any of the props relevant to a listener path has changed (which is not the case when you log into the basic example, since only '/todos' is passed).

Working Pattern

As long as you pass auth as a prop, the component should re-render since one of the props will change (from undefined to an object or null). Something like this should work (untested):

import React, { Component, PropTypes } from 'react'
import { isLoaded, isEmpty, pathToJS, dataToJS } from 'react-redux-firebase'
import { connect } from 'react-redux'

class App extends Component {
  render() {
    const { auth } = this.props
    if (!isLoaded(auth) || isEmpty(auth)) {
      return (<p>you need to login</p>)
    }
    return <TodoComponent todos={this.props.todos} />;
  }
}

const fbWrappedComponent = firebaseConnect([
  '/todos'
])(App)

export default connect(
  ({ firebase }) => ({
    todos: dataToJS(firebase, 'todos'),
    auth: pathToJS(firebase, 'auth')
  })
)(fbWrappedComponent)

I suggest moving your login logic to its own route similar to the material example.

No Decorators

Even if you are not using decorators, you can still use Higher Order Components as indicated in the docs and the Readme:

class SomeComponent extends Component {

}
export default connect()(SomeComponent)

// vs

@connect()
export default class SomeComponent extends Component {

}

@jarbot
Copy link
Author

jarbot commented Apr 10, 2017

Is there a reason you didn't want to use isLoaded and isEmpty that is provided by react-redux-firebase?

I just have a habit of using lodash, no other reason.

Thanks for the working example. I will test it soon. I was able to use the "Higher Order Components" without decorators but re-renders were not being triggered when the data was changing on firebase. I suspect this is an issue with my setup. I need to start a new project to figure out whats going on.

@prescottprue
Copy link
Owner

Working example will be noted in docs and simple example README for clarity. Closing since this is understood behavior.

prescottprue added a commit that referenced this issue May 16, 2017
….4.0 release

* Added notes from #93 to simple example.
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

3 participants