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

React components not updating on store change #22

Closed
ro-savage opened this issue Feb 12, 2018 · 6 comments
Closed

React components not updating on store change #22

ro-savage opened this issue Feb 12, 2018 · 6 comments

Comments

@ro-savage
Copy link

There is a good chance this is a just me issue. But I've been trying to figure out what's going on for a while with no success.

When I update my store, my components won't auto update to recognise the change.

Full repo: https://github.com/ro-savage/the-places-you-will-go
Code Sandbox: https://codesandbox.io/s/github/ro-savage/the-places-you-will-go
(react-easy-state 4.1.2, React 16.2, CRA 1.1.1)

Store

import { store } from 'react-easy-state'

const destinations = {
  add: (destination) => { 
    destinations.list.push(destination); 
    console.log(destination, destinations.list)
  },
  new: '',
  list: [
    'Canberra',
    'Auckland',
    'Berlin',
  ]
}

export default store(destinations)

Listing an array

import React from 'react'
import { view } from 'react-easy-state'

import destinations from '../destinationsStore.js'

const ListDestination = () => {
  return (
    <ul>
      {destinations.list.map(destination => {
        return (
          <li key={destination}>{destination}</li>
        )
      })}
    </ul>
  )
}

export default view(ListDestination)

Adding to array

import React from 'react'
import { view } from 'react-easy-state'

import destinations from '../destinationsStore.js'

const AddDestination = () => {
  const onClick = () => {
    destinations.add(destinations.new)
    destinations.new = ''
  }
  return (
    <div>
      <div>Choose a destination</div>
      <input 
          type="text" placeholder="Destination" 
          onChange={(e) => {destinations.new = e.target.value}} 
          value={destinations.new} 
       />
      <button onClick={onClick}>Add Destination</button>
    </div>
  )
}

export default view(AddDestination)

App

import React, { Component } from "react"
import "./App.css"

import { view } from 'react-easy-state'
import AddDestination from './destinations/AddDestination/AddDestination';
import ListDestinations from './destinations/ListDestinations/ListDestinations';


class App extends Component {
  render() {
    return (
      <div className="App">
        <header className="App-header">
          <AddDestination />
          <ListDestinations />
        </header>
      </div>
    )
  }
}

export default view(App)
@bleeinc
Copy link

bleeinc commented Feb 12, 2018

Each time you import the store I believe you are creating a new store. If you would like to import the same store in multiple files you could make it a singleton class and return the already existing instance.

@solkimicreb
Copy link
Member

solkimicreb commented Feb 12, 2018

There is a bug in your code, but it is partly my mistake 🙂 I miscommunicated something.

Your store is not reactive

import { store } from 'react-easy-state'

const destinations = {
  add: (destination) => { 
    destinations.list.push(destination); 
  },
  new: '',
  list: [
    'Canberra',
    'Auckland',
    'Berlin',
  ]
}

export default store(destinations)

You should move the store wrapping to the top to make it reactive

import { store } from 'react-easy-state'

const destinations = store({
  add: (destination) => { 
    destinations.list.push(destination); 
  },
  new: '',
  list: [
    'Canberra',
    'Auckland',
    'Berlin',
  ]
})

export default destinations

Some context:

Easy State has a principle of never messing with user data. It wraps things with a reactive layer, but it never instruments anything with reactivity directly (by mutating it). The layer can be removed and reapplied at any time. Check this example:

import { store } from 'react-easy-state'

const person = { name: 'Bob' }
const reactivePerson = store(person)

// this triggers renders
reactivePerson.name = 'Rick'

// this does not
person.name = 'Morty'

// both update the underlying person object
// but reactivePerson has a wrapping reactive layer

In your store, you do something like this:

const person = {
  // this updates the none reactive raw object
  updateName: name => person.name = name
}

// this exports the wrapped reactive object
export default store(person)

This can be avoided by wrapping as early as possible - preferably at definition time:

const person = store({
  // this updates the none reactive raw object
  updateName: name => person.name = name
})

// this exports the wrapped reactive object
export default person

I will update the docs and examples to reflect this. I never had to deal with this, because I prefer the none arrow method syntax with this. this refers to the reactive object, since the method with this is called on the exported reactive object.

const person = {
  // this works, because `this` is the reactive object
  // `person` is still the raw none reactive object
  updateName (name) {
    this.name = name
  }
}

export default store(person)

Also updating the examples to have a bit more variety would be nice. Maybe some arrow function methods, decorators and typescript could be added. If you have any working example, I would gladly take it as a contribution 🙂

Edit: just to make it clear, I think this is one of the coolest features of the library and it will stay like this. I just messed up the communication about it.

@solkimicreb
Copy link
Member

@bleeinc Objects are singletons in JS and Easy State does not mess with that. If you create a store object, it stays like that - a single object.

@ro-savage
Copy link
Author

ro-savage commented Feb 12, 2018

Interesting. It makes sense now but it never occurred to me.

Works

const destinations = {
  add: function(destination) { this.list.push(destination) },
  list: []
}
export default store(destinations)

Works

const destinations = store({
  add: function(destination) { this.list.push(destination) },
  list: []
})

Works

const destinations = store({
  add: (destination) => destinations.list.push(destination)
  list: []
})
export default destinations

Incorrect JS / Gives useful error

const destinations = store({
  add: (destination) => this.list.push(destination)
  list: []
})
export default destinations

Reactivity breaks with no error / Object is updated / Re-renders shows updated list

const destinations = {
  add: (destination) => destinations.list.push(destination)
  list: []
}
export default store(destinations)

The clearest thing to me would be to write in the docs that store should be used at definition and make the examples always that way. Then you take account the use of arrow functions and function declarations.

Having said that, I probably would have still done it because in my mind store(obj) == store({}). I am not sure if you could have a check when in dev mode that would warn people not to do the last example.

You could add a Gotachs / Help / FAQ section and include the last example. Under a heading such as React not re-rendering on change.

@solkimicreb
Copy link
Member

Yeah, that's a nice summary. A Gotchas section would definitely help and will be added. Examples are already refactored in the next branch.

@solkimicreb
Copy link
Member

I added a gotchas section and refactored the examples in the recently released v5. Let me know if you think something is missing 🙂

Thanks for the issue!

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