-
-
Save tomusdrw/6be7adb067ba44920931d8146f7467f7 to your computer and use it in GitHub Desktop.
type PayloadType = { | |
}; | |
type EstimateFeeType = {}; | |
class ApiCalls { | |
async estimateFee(chain: Chain, laneId: LaneId, payload: PayloadType): EstimateFeeType { | |
const payloadType = createType(chain, "OutboundPayload", payload); | |
return await stateCall(estimateFeeMethod, ...); | |
} | |
} | |
function Component() { | |
const apiCalls: ApiCalls = useApiCalls(); | |
const { isFeeCalculating, setFeeCalculating} = useState(false); | |
const { fee, setFee } = useState(null); | |
const { feeError, setFeeError } = useState(null); | |
useEffect(() => { | |
setFeeCalculating(true); | |
setFee(null); | |
try { | |
const fee = await apiCalls.estimateFee(chain, laneId, payload) | |
setFee(fee); | |
setFeeCalculating(false); | |
} catch (e) { | |
setFeeCalculating(false); | |
setFeeError("error"); | |
} | |
}); | |
} | |
import {useEstimateFee} from './hooks/api/useEstimateFee'; | |
function Component() { | |
const { fee: number | null, feeError: string|null, isFeeCalculating: bool, recalculateFee: () => void } = useEstimateFee(laneId, ...); | |
fee==null | |
isFeeCalculating == true | |
// | |
useEffect(() => { | |
recalculateFee(); | |
}); | |
} |
@todr @hbulgarini Thank you for your kind words.
Concerning 1)
: useReducer is a (lets say) more complicated useState. Its not frowned upon or dismissed for use by me, and it has benefits that can alter complicated states while useState is more of a shallow state holder (P.S. I promise never to argue about reducers again)
Concerning 2)
: I do not mention the "presentational" components as its not part of our discussions. I have said before that there must be "clever"/"parent"/"container" components that take care of data, states, api calls (if needed), dispatchers etc etc, and simple "stupid"/"childs"/"presentational" components that the ONLY thing that should do is re-render based on prop and (worst case scenario) maybe fire back something.
Sample code is there just for pointing out one solution for our refactor scenarios, and not of course that would not be the ultimate solution. This is meant just as some food for thought. It has a lot of fixes/makeovers to make the code reusable and better.
I was diving a little more on this and trying to bring all what we discuss to our existing processes. And here my conclusions:
-
store.js
: is exactly what we have now, but split in different reducers per concern. I would not join them all in one global reducer since one update on each specific concern would force all the reducers to update. -
useApi.ts
: I think there should be two separate hooks, theuseApi.ts
as it is in your example code nikos, but then the generic call should be extracted to a different process. -
useGenericCall.ts
: i think this is a pretty useful and reusable hook but it was pretty hard to add the logic to handle global calls which are provided to it as input props of the hook. The main problem i faced with this is that when you have to provide the global functions as input props for this hook , it is really easy to forget a useCallback that will make the whole app to enter in a rerender infinite loop.
So my recommendation is to keep this generic call hook, but to only use it when this hook is going to be used locally ( without having to use this values as global) -
Global calls: My preferred options is to use reducers. Not only for the benefits of state management flow , etc, but also because i have confirmed that the use of the
dispatcher
is much much more secure in terms of rerender control than sharing trough props chains or context a wrapped function that will end calling asetState
. In case those calls are not properly wrapped in auseCallback
everything gets rerender again because the function reference is different in each render.
So my suggested rule is in case a value has to be global ( as estimatedFee and payload ) , they should have their own dedicated hook that will expose a function that will handle their corresponding update status with dispatchers ( similar to my first comment )
The following can really make all the above TLDR and show what I mean in idea.ts and project I built.
I am approaching it more like this:
- Layer 1: API Layer ----------------------> PolkadotJS API
- Layer 2: Bridges API specific Layer -----> Bridges API that uses specific needed things from polkadotJS API (not all API calls from polkadot JS are needed
- Layer 3: Bridges Simple/Specific Business logic ----------> Functions/hooks that are using the Bridges API for 1 VERY specific reusable action (get the chain, estimate fee etc etc)
- Layer 4: Bridges Advanced/Complicated Logic ---------> Functions/hooks that can use any of Layers 2, 3 to make complicated logic (send a transaction and estimate fee and do something else at the same time)
- Layer 5: Bridges or other Bridges-related UI ------> UI components that can use any of Layers 2 - 4, to re-create or reuse Bridges functionalities (simple or complicated)
Layers 2 -4 is the "library" we need to build. Layer 5 should be the actual Bridges UI. Layer 2-4 should ALWAYS work with specific version of Layer 1
Cool stuff @wirednkod, thanks for the write up.
I really like the idea with
getStateCall/useStateCall
functions. Take a look below, left a few comments and also made theuseStateCall
generic so that we don't have to duplicate that code for every API call. Obviously this will require a bit of TS type magic to get work.Regarding state management - I like your approach, there are few things that I'd like to point out though:
useState
functions passed via context as well.useState/reducer
discussion is a bit out of scope though, so I'm just noting it down - don't want to trigger anyone :)useState
for presentation state (likeisToggled
) sometimes). IMHO the more complex the display/component is - the more the component should avoid using domain-specific hooks.@hbulgarini
My point is that we have both
useState
anduseReducer
for global state management and I think it's without any particular reason - like if I was to add a new global state I wouldn't know if it should be a reducer oruseState
.You either pass them via
props
or you pass them viacontext
. I don't really know if there is any 3rd way of doing that. Examples:or
The same design can be applied not only in the top-level component, but anywhere down the hierarchy.
Obviously passing props is super cumbersome, but it simplifies thinking about the code a lot - cause you see all of the things that are being used right from reading the prop definition.
Context are more magical, hence they should be use with great care - i.e. you don't want a random tiny component (for instance a button) suddenly making a global state call if that's something that could be encapsulated in a parent component and the function to invoke could be passed as a prop.
Hiding context usage and a lot of hook-nesting (i.e. hook calling another hook, etc) make it difficult to follow the logic of the application, of course it's needed to avoid duplication, but the hardest part in programming is navigating between too much and too little of it (the hardest except for naming things, obviously:))