Skip to content

[WIP] Fix app state clash on browser refresh #1496

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

[WIP] Fix app state clash on browser refresh #1496

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions apps/address-book/app/app-state-reducer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
const appStateReducer = state => {
import { getContentHolder } from '../../../shared/lib/utils'

let prevState = {}

const appStateReducer = currentState => {
const state = getContentHolder('entries', currentState, prevState)
prevState = { ...state }
Copy link
Collaborator

@chadoh chadoh Oct 31, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just prevState = state? Does it need to be a brand new object?

Copy link
Contributor Author

@e18r e18r Oct 31, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as the state doesn't get changed later on, it doesn't need to.

In principle, the practice is to create a newState object instead of modifying the state. But just to be safe, I did that. Cause if it gets changed, things can get nasty.

Copy link
Collaborator

@chadoh chadoh Nov 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect that in general we return new state objects too frequently, and that this is part of what causes unwanted refreshes of our apps.

return {
...state
}
}

export default appStateReducer
export default appStateReducer
9 changes: 8 additions & 1 deletion apps/allocations/app/app-state-reducer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { BigNumber } from 'bignumber.js'
import { ETHER_TOKEN_FAKE_ADDRESS } from '../../../shared/lib/token-utils'
import { getContentHolder } from '../../../shared/lib/utils'

// Use this function to sort by ETH and then token symbol
const compareBalancesByEthAndSymbol = (tokenA, tokenB) => {
Expand All @@ -17,7 +18,13 @@ const getTokenFromAddress = (tokenAddress, tokenList) => {
return tokenList.find(token => token.address === tokenAddress)
}

function appStateReducer(state) {
let prevState = {}

function appStateReducer(currentState) {

const state = getContentHolder('accounts', currentState, prevState)
prevState = { ...state }

const { accounts: budgets, balances } = state || {}

const balancesBn = balances
Expand Down
8 changes: 7 additions & 1 deletion apps/dot-voting/app/app-state-reducer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
function appStateReducer(state) {
import { getContentHolder } from '../../../shared/lib/utils'

let prevState = {}

function appStateReducer(currentState) {
const state = getContentHolder('votes', currentState, prevState)
prevState = { ...state }
return {
...state,
}
Expand Down
8 changes: 7 additions & 1 deletion apps/projects/app/app-state-reducer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
function appStateReducer(state) {
import { getContentHolder } from '../../../shared/lib/utils'

let prevState = {}

function appStateReducer(currentState) {
const state = getContentHolder('repos', currentState, prevState)
prevState = { ...state }
return state || {}
}

Expand Down
8 changes: 7 additions & 1 deletion apps/rewards/app/app-state-reducer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,14 @@ import {
calculateMyRewardsSummary
} from './utils/metric-utils'
import { MILLISECONDS_IN_A_MONTH, MILLISECONDS_IN_A_WEEK, MILLISECONDS_IN_A_YEAR, MILLISECONDS_IN_A_DAY } from '../../../shared/ui/utils/math-utils'
import { getContentHolder } from '../../../shared/lib/utils'

function appStateReducer(state) {
let prevState = {}

function appStateReducer(currentState) {

const state = getContentHolder('rewards', currentState, prevState)
prevState = { ...state }

if(state){
state.amountTokens = state.balances.map(token => {
Expand Down
15 changes: 15 additions & 0 deletions shared/lib/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,18 @@ export function formatTokenAmount(
formatDecimals(round(amount / Math.pow(10, decimals), rounding), 18)
)
}

/**
* Chooses between two objects depending on whether a given array property in
* each of them has content. If the comparison is nonconclusive, the first
* object (the incumbent) is returned.
*/
export const getContentHolder = (property, incumbent, contender) => {
if (incumbent && incumbent.hasOwnProperty(property)
&& incumbent[property].length > 0)
return incumbent
if (contender && contender.hasOwnProperty(property)
&& contender[property].length > 0)
return contender
return incumbent
}