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

Refactor: Removes routerHistory from Redux store #573

Merged
merged 6 commits into from
Apr 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 17 additions & 1 deletion src/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ import hathorLib from '@hathor/wallet-lib';
import { IPC_RENDERER } from './constants';
import AddressList from './screens/AddressList';
import NFTList from './screens/NFTList';
import { updateLedgerClosed } from './actions/index';
import { resetNavigateTo, updateLedgerClosed } from './actions/index';
import { WALLET_STATUS } from './sagas/wallet';
import ProposalList from './screens/atomic-swap/ProposalList';
import EditSwap from './screens/atomic-swap/EditSwap';
Expand All @@ -58,12 +58,14 @@ function Root() {
walletStartState,
isVersionAllowed,
wallet,
navigateTo,
} = useSelector((state) => {
return {
ledgerClosed: state.ledgerWasClosed,
walletStartState: state.walletStartState,
isVersionAllowed: state.isVersionAllowed,
wallet: state.wallet,
navigateTo: state.navigateTo,
};
});
const dispatch = useDispatch();
Expand Down Expand Up @@ -160,6 +162,20 @@ function Root() {
}
}, [isVersionAllowed, wallet]);

/**
* This effect allows navigation to be triggered from background tasks such as sagas.
*/
useEffect(() => {
// Ignore this effect if there is no route to navigateTo
if (!navigateTo.route) {
return;
}

// Navigate to the informed route and reset the navigateTo state property
navigate(navigateTo.route, { replace: navigateTo.replace });
dispatch(resetNavigateTo());
}, [navigateTo])

// Handles failed wallet states
if (walletStartState === WALLET_STATUS.FAILED) {
return <LoadWalletFailed />;
Expand Down
22 changes: 17 additions & 5 deletions src/actions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export const types = {
WALLET_RESET_SUCCESS: 'WALLET_RESET_SUCCESS',
WALLET_REFRESH_SHARED_ADDRESS: 'WALLET_REFRESH_SHARED_ADDRESS',
SET_SERVER_INFO: 'SET_SERVER_INFO',
STORE_ROUTER_HISTORY: 'STORE_ROUTER_HISTORY',
SET_NAVIGATE_TO: 'SET_NAVIGATE_TO',
WALLET_RELOADING: 'WALLET_RELOADING',
FEATURE_TOGGLE_INITIALIZED: 'FEATURE_TOGGLE_INITIALIZED',
SET_FEATURE_TOGGLES: 'SET_FEATURE_TOGGLES',
Expand Down Expand Up @@ -441,11 +441,23 @@ export const reloadingWallet = () => ({
});

/**
* @param {RouterHistory} routerHistory History object from react-dom-navigation
* @param {string} route Route that should be navigated to in consequence of an event
* @param {boolean} replace Should we navigate with the replace parameter set
*/
export const storeRouterHistory = (routerHistory) => ({
type: types.STORE_ROUTER_HISTORY,
routerHistory,
export const setNavigateTo = (route, replace = false) => ({
type: types.SET_NAVIGATE_TO,
route,
replace,
});

/**
* Resets the `navigateTo` property.
* Should be called after every successful navigation executed through this property.
*/
export const resetNavigateTo = () => ({
type: types.SET_NAVIGATE_TO,
route: '',
replace: false,
});

/**
Expand Down
17 changes: 9 additions & 8 deletions src/reducers/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,8 @@ const initialState = {
// This should store the last action dispatched to the START_WALLET_REQUESTED so we can retry
// in case the START_WALLET saga fails
startWalletAction: null,
// RouterHistory object
routerHistory: null,
// A helper for listeners to navigate to another screen on saga events
navigateTo: { route: '', replace: false },

/**
* Indicates if the Atomic Swap feature is available for use
Expand Down Expand Up @@ -258,8 +258,8 @@ const rootReducer = (state = initialState, action) => {
return onStartWalletFailed(state);
case types.WALLET_BEST_BLOCK_UPDATE:
return onWalletBestBlockUpdate(state, action);
case types.STORE_ROUTER_HISTORY:
return onStoreRouterHistory(state, action);
case types.SET_NAVIGATE_TO:
return onSetNavigateTo(state, action);
case types.SET_UNLEASH_CLIENT:
return onSetUnleashClient(state, action);
case types.SET_FEATURE_TOGGLES:
Expand Down Expand Up @@ -969,14 +969,15 @@ export const onWalletBestBlockUpdate = (state, action) => {
};

/**
* @param {RouterHistory} action.routerHistory History object from react-dom-navigation
* @param {string} action.route Route that should be navigated to in consequence of an event
* @param {boolean} action.replace Whether we should navigate with the replace parameter set
*/
export const onStoreRouterHistory = (state, action) => {
const { routerHistory } = action;
export const onSetNavigateTo = (state, action) => {
const { route, replace } = action;

return {
...state,
routerHistory,
navigateTo: { route, replace },
};
};

Expand Down
4 changes: 2 additions & 2 deletions src/sagas/atomicSwap.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
proposalFetchFailed,
proposalFetchSuccess,
proposalUpdated,
setNavigateTo,
types
} from "../actions";
import { specificTypeAndPayload } from "./helpers";
Expand Down Expand Up @@ -164,8 +165,7 @@ function* createProposalOnBackend(action) {
updatePersistentStorage(allProposals);

// Navigating to the Edit Swap screen with this proposal
const routerHistory = yield select((state) => state.routerHistory);
routerHistory(`/wallet/atomic_swap/proposal/${proposalId}`, { replace: true });
yield put(setNavigateTo(`/wallet/atomic_swap/proposal/${proposalId}`, true));
} catch (e) {
yield put(lastFailedRequest({ message: e.message }));
}
Expand Down
27 changes: 8 additions & 19 deletions src/sagas/wallet.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ import {
startWalletFailed,
walletStateError,
walletStateReady,
storeRouterHistory,
setNavigateTo,
reloadingWallet,
tokenInvalidateHistory,
sharedAddressUpdate,
Expand Down Expand Up @@ -105,14 +105,12 @@ export function* startWallet(action) {
passphrase,
pin,
password,
routerHistory,
xpub,
hardware,
} = action.payload;
let xpriv = null;

yield put(loadingAddresses(true));
yield put(storeRouterHistory(routerHistory));

if (hardware) {
// We need to ensure that the hardware wallet storage is always generated here since we may be
Expand Down Expand Up @@ -193,7 +191,7 @@ export function* startWallet(action) {
/**
* Lock screen will call `resolve` with the pin screen after validation
*/
routerHistory('/locked/');
dispatch(setNavigateTo('/locked/'));
dispatch(lockWalletForResult(resolve));
}),
passphrase,
Expand Down Expand Up @@ -280,7 +278,7 @@ export function* startWallet(action) {
console.error(e);
// Return to locked screen when the wallet fails to start
LOCAL_STORE.lock();
routerHistory('/');
yield put(dispatch(setNavigateTo('/')));
return
}
}
Expand All @@ -299,7 +297,7 @@ export function* startWallet(action) {
}

// Wallet start called, we need to show the loading addresses screen
routerHistory('/loading_addresses', { replace: true });
yield put(setNavigateTo('/loading_addresses', true));

// Wallet might be already ready at this point
if (!wallet.isReady()) {
Expand Down Expand Up @@ -333,7 +331,7 @@ export function* startWallet(action) {

LOCAL_STORE.unlock();

routerHistory('/wallet/', { replace: true });
yield put(setNavigateTo('/wallet/', true));

yield put(loadingAddresses(false));

Expand Down Expand Up @@ -501,7 +499,6 @@ export function* handleTx(wallet, tx) {
export function* handleNewTx(action) {
const tx = action.payload;
const wallet = yield select((state) => state.wallet);
const routerHistory = yield select((state) => state.routerHistory);

if (!wallet.isReady()) {
return;
Expand Down Expand Up @@ -537,11 +534,7 @@ export function* handleNewTx(action) {
// Set the notification click, in case we have sent one
if (notification !== undefined) {
notification.onclick = () => {
if (!routerHistory) {
return;
}

routerHistory(`/transaction/${tx.tx_id}/`);
put(setNavigateTo(`/transaction/${ tx.tx_id }/`))
tuliomir marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
Expand Down Expand Up @@ -661,7 +654,6 @@ export function* walletReloading() {

const wallet = yield select((state) => state.wallet);
const useWalletService = yield select((state) => state.useWalletService);
const routerHistory = yield select((state) => state.routerHistory);

// If we are using the wallet-service, we don't need to wait until the addresses
// are reloaded since they are stored on the wallet-service itself.
Expand Down Expand Up @@ -703,7 +695,7 @@ export function* walletReloading() {

// Load success, we can send the user back to the wallet screen
yield put(loadWalletSuccess(allTokens, registeredTokens, currentAddress));
routerHistory('/wallet/', { replace: true });
yield put(setNavigateTo('/wallet/', true));
yield put(loadingAddresses(false));
} catch (e) {
yield put(startWalletFailed());
Expand All @@ -723,7 +715,6 @@ export function* refreshSharedAddress() {
}

export function* onWalletReset() {
const routerHistory = yield select((state) => state.routerHistory);
const wallet = yield select((state) => state.wallet);

localStorage.removeItem(IGNORE_WS_TOGGLE_FLAG);
Expand All @@ -734,9 +725,7 @@ export function* onWalletReset() {

yield put(walletResetSuccess());

if (routerHistory) {
routerHistory('/welcome');
}
yield put(setNavigateTo('/welcome'));
}

export function* onWalletServiceDisabled() {
Expand Down
2 changes: 1 addition & 1 deletion src/screens/ChoosePassphrase.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ function ChoosePassphrase() {
*/
const handlePassphrase = async () => {
modalContext.hideModal();
await walletUtils.addPassphrase(wallet, passphraseRef.current.value, pinRef.current.value, passwordRef.current.value, navigate)
await walletUtils.addPassphrase(wallet, passphraseRef.current.value, pinRef.current.value, passwordRef.current.value)
navigate('/wallet/');
}

Expand Down
2 changes: 1 addition & 1 deletion src/screens/LoadWallet.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ function LoadWallet() {
const pinSuccess = () => {
LOCAL_STORE.unlock();
// First we clean what can still be there of a last wallet
wallet.generateWallet(words, '', pin, password, navigate);
wallet.generateWallet(words, '', pin, password);
LOCAL_STORE.markBackupDone();
LOCAL_STORE.open(); // Mark this wallet as open, so that it does not appear locked after loading
// Clean pin and password from redux
Expand Down
2 changes: 1 addition & 1 deletion src/screens/LockedWallet.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ function LockedWallet() {

setLoading(true);

dispatch(startWalletRequested({ pin, routerHistory: navigate }));
dispatch(startWalletRequested({ pin }));
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/screens/NewWallet.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ function NewWallet() {
const pinSuccess = () => {
// Generate addresses and load data
LOCAL_STORE.unlock();
wallet.generateWallet(words, '', pin, password, navigate);
wallet.generateWallet(words, '', pin, password);
// Mark this wallet as open, so that it does not appear locked after loading
LOCAL_STORE.open();
// Clean pin, password and words from redux
Expand Down
2 changes: 1 addition & 1 deletion src/screens/Server.js
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ function Server() {
// We don't have PIN on hardware wallet
const pin = isHardwareWallet ? null : pinRef.current.value;
try {
await walletUtils.changeServer(wallet, pin, navigate, networkChanged);
await walletUtils.changeServer(wallet, pin, networkChanged);
navigate('/wallet/');
} catch (err) {
setLoading(false);
Expand Down
1 change: 0 additions & 1 deletion src/screens/StartHardwareWallet.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,6 @@ function StartHardwareWallet() {
passphrase: '',
pin: null,
password: '',
routerHistory: navigate,
xpub,
hardware: true,
}));
Expand Down
14 changes: 5 additions & 9 deletions src/utils/wallet.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,12 +115,11 @@ const wallet = {
* @param {string} passphrase
* @param {string} pin
* @param {string} password
* @param {Object} routerHistory History to push new path in case of notification click
*
* @memberof Wallet
* @inner
*/
generateWallet(words, passphrase, pin, password, routerHistory) {
generateWallet(words, passphrase, pin, password) {
try {
walletUtils.wordsValid(words);
} catch(e) {
Expand All @@ -138,7 +137,6 @@ const wallet = {
passphrase,
pin,
password,
routerHistory,
}))
},

Expand Down Expand Up @@ -316,9 +314,9 @@ const wallet = {
*
* @param {HathorWallet} wallet The wallet instance
* @param {string} pin The pin entered by the user
* @param {any} routerHistory
* @param networkChanged
*/
async changeServer(wallet, pin, routerHistory, networkChanged) {
async changeServer(wallet, pin, networkChanged) {
// We only clean the storage if the network has changed
await wallet.stop({ cleanStorage: true, cleanAddresses: true });

Expand All @@ -335,14 +333,12 @@ const wallet = {
passphrase: '',
pin,
password: '',
routerHistory,
hardware: false,
}));
} else {
store.dispatch(startWalletRequested({
passphrase: '',
password: '',
routerHistory,
xpub: wallet.xpub,
hardware: true,
}));
Expand All @@ -361,13 +357,13 @@ const wallet = {
* @memberof Wallet
* @inner
*/
async addPassphrase(wallet, passphrase, pin, password, routerHistory) {
async addPassphrase(wallet, passphrase, pin, password) {
const words = await LOCAL_STORE.getWalletWords(password);

// Clean wallet data, persisted data and redux
await this.cleanWallet(wallet);
helpers.loadStorageState();
this.generateWallet(words, passphrase, pin, password, routerHistory);
this.generateWallet(words, passphrase, pin, password);
},

/*
Expand Down
Loading