trufflesuite / drizzle

Reactive Ethereum dapp UI suite
905 stars 235 forks source link

AccountData - accounts and accountBalances are supposed to be arrays but are reduced into objects #4

Closed RobinReborn closed 5 years ago

RobinReborn commented 5 years ago

I get this warning in react - I have fixed it by modifying AccountData.js and using it locally.

adrianmcli commented 5 years ago

This is a bug and it requires 2 realizations:

  1. accounts should be an array, but it currently isn't.
  2. accountBalances should be an object, since it is keyed by addresses and that makes perfect sense.

Problem

  1. This line in the accountsReducer uses Object.assign instead of an array concatenation (i.e. spread operator) and it forces the array into an object:

    return Object.assign({}, state, action.accounts)
  2. This line is expecting an Array of Strings rather than an Object of Strings:

    accountBalances: PropTypes.arrayOf(PropTypes.string),

Solution

  1. This should be:

    return [...state, ...action.accounts]
  2. This should be:

    accountBalances: PropTypes.objectOf(PropTypes.string),
adrianmcli commented 5 years ago

After some discussion with @cds-amal, we decided to loosen the restraints on the React-Component PropTypes rather than changing the reducer. This is because we don't want to cause any breaking changes without a large version bump.

Therefore, new (temporary) solution is to change from this:

accounts: PropTypes.arrayOf(PropTypes.string),
accountBalances: PropTypes.arrayOf(PropTypes.string),

to:

accounts: PropTypes.any,
accountBalances: PropTypes.objectOf(PropTypes.string),

This will clear up the errors, but we must remember to (for 2.x) implement the solution I listed above.