idea.ts

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();
    });
}

Also we could extend even more the flexibility of each reducer by creating a custom hook that wraps the useReducer and provide before and after middleware functionalities. Here there is an article with a proposal: https://www.robinwieruch.de/react-usereducer-middleware

No, I don't think it's a good idea.

1. The point is to separate library from our code, so moving any of the library stuff to any of the reducers/hooks we have already is not the way to go. Basically think of this like if you were writing a separate library to be used by multiple React UIs.
2. Reducers in case the variables are not very dependent on each other introduce A LOT of boilerplate and I'm not sure if they add that much value if they are mixed with hooks anyway - my point is that it's hard to draw a line when to use a reducer and when to use hooks and migrating between one an the other is not easy. The main point of the architecture redesign is to set up boundaries and only then we can set up rules within these boundaries.
3. the middleware thing is basically reimplementing redux and sagas using useReducer - imho that's a quite a boilerplate for us and would complicate the code even further. I think what we need is simplicity - especially in places that are being most often and the complexity should be hidden once and for all.

There are two main issues that we try to solve with the refactor we are trying to do and I will try to explain below which they are and how they can be solved, using "mocks" of existing code (probably it wont work if we try to compile it).

First issue is the API calls that take place are all over the application (useEffects/hooks/components) with a result of possible leaks (e.g. subscriptions).
As discussed before it would be a good idea to encapsulate them in our own functions/hooks in a way that can be reusable and our components can reuse them in a safe non-complicated manner.

Second issue comes to the state management of the application and the way that it should exists (global, local, ways to update it etc.)

For the first issue I set up a file mocking existing code (based on https://medium.com/weekly-webtips/patterns-for-doing-api-calls-in-reactjs-8fd9a42ac7d4):

// ./src/hooks/api/useStateCalls.ts
import { useState, useCallback } from 'react';


export const getStateCall = async (chain, data, at): Codec => {
  const { getValuesByChain } = useChainGetters();
  const getChainValues = useCallback((chain) => getValuesByChain(chain), [getValuesByChain]);
  
  const {
    api,
    substrateValues: { estimatedFeeMethodName }
  } = getChainValues(chain);

  const params = [estimatedFeeMethodName, data];
  if (at) { params.push(at); }
  try {
    const response: Codec = await api.rpc.state.call<Codec>(...params); 
    // Add any transformation from api needed here
    return response;
  } catch (err) {
    throw new Error(err);
  }   
};

export const useStateCall = () => {
  const [isLoading, setIsLoading] = useState<boolean>(false);
  const [error, setError] = useState<unknown>(null);
  const [data, setData] = useState<unknown>(null);
  
  const execute = async (chain, data, at) => {
    try {
      setIsLoading(true);
      const stateCall = await getStateCall(chain, data, at);
      setData(stateCall);
      return stateCall;
    } catch (e) {
      setError(e);
      setIsLoading(false);
      throw e;
    }
  };
  
  return {
    isLoading,
    error,
    data,
    execute,
    executeCb: useCallback(execute, [])
  };
};

The reasons why it is set like this follows:
a) By having two functions exposed (getStateCall and useGetStateCall) the api can be used in a hook and in a normal way. This way we keep the same API logic across the application and we can even add extra functions or extra logic inside the same file that are relevant to the same API call.
b) There is 1 place for common transformation for each API call. Each api when called will return some data that we may need all of them or just specific variables. This transformation can be added in the getStateCall (non-hook) and will be reused from hook version as well. This makes testability and logic of the transformation.
c) useStateCall() is returning the data, execute AND an executeCb function. The latter can be used inside useEffect in order to reduce number of calls or (God forbid) infinite calls. From the other hand, data can be used inside a handler function (e.g. onSubmit) and have the result without any dependency on any hook. For example:

function App () => {
  const { isLoading, data, execute } = useStateCall();
  
  useEffect(() => {
    try {
      // setup somewhere here the <sourceChain>, <messageFeeType.toHex()>
      executeCb(sourceChain, messageFeeType.toHex());
    } catch(err) {
      log(error);
    }
  }, [execute]);

  return isLoading ? (<div>Loading</div>) : (<div>{data}<div>) 
}

or

function App () => {
  const { isLoading, data, execute } = useStateCall();
  
  // setup somewhere here the <sourceChain>, <messageFeeType.toHex()>

  const clickFunc = (sourceChain, messageFeeType.toHex()) => {
    try {
      execute(sourceChain, messageFeeType.toHex());
    } catch (err) {
      console.log('error');
    }
  }

  return (
  <div>
    <div>{data}</div>
    <button onClick={clickFunc}</input>
  <div>) 
}

d) the isLoading can be used in order to show the status of the specific API call and display appropriate conditional rendering.
e) We can use same approach for serialize request that first depends on the second.

That means that in a case of subscriptions we could use as example an api WITH subscriptions AND needed data for 2nd subscription. Based on the example approach introduced that could be from:

let unsubImportedHeaders: Promise<VoidFn> | null;
    const unsubBestFinalized: Promise<VoidFn> | null = api.query[bridgedGrandpaChain].bestFinalized(
      (res: CodecHeaderId) => {
        const bestBridgedFinalizedBlock = res.toString();
        unsubImportedHeaders = api.query[bridgedGrandpaChain].importedHeaders(bestBridgedFinalizedBlock, (res: any) => {
          if (res.toJSON()) {
            setBestBridgedFinalizedBlock(res.toJSON().number);
          }
        });
      }
    );
return async (): Promise<void> => {
  unsubBestFinalized && (await unsubBestFinalized)();
  unsubImportedHeaders && (await unsubImportedHeaders)();
};

to:

const { unsubBestFinalized, data }: Promise<VoidFn> | null = await useBestFinalized();
const { unsubImportedHeaders }  = await useImportedHeaders(data)''

return async (): Promise<void> => {
  unsubBestFinalized && (await unsubBestFinalized)();
  unsubImportedHeaders && (await unsubImportedHeaders)();
};

For the second issue, as discussed before, I prefer to follow patterns that react itself follows and I will place it here.
We should keep 2 kind of states based on the functionality that they are used for.
By "used for" I mean, the state should be global (a context, a provider, initial state and dispatch through useReduce) if is used for many components and that state has complex updateable state logic, and "LOCAL" if state is used from small amount of components (parent can pass it to children or 1 only component has it).

As it is visible on "sketch" above the outcome of all the contexts together (if it was possible) should be 1 main "tree" that dispatchers update 1 or more "branches" of the "tree".

(To put the 2 together now) my idea of best approach would be to use functions/hooks for the APIs (as mentioned above) in order to retrieve data needed (most likely the "heavy" ones would be for the init State of the app - thus contexts)
and then each component (or parent of components) will use api functions/hooks to retrieve data needed for the logic that exists inside the component, and then based on the outcome call dispatchers for updating the logic of the context.

@wirednkod first of all : nice answer!

I like the idea of having two definitions in one file: the call with async and the hook that returns all the states referred to the call ( isLoading, data, error , etc .. ). I see the benefits on your proposal.

Anyway in your file ./src/hooks/api/useStateCalls.ts it is not easy to me to see how you would share that data into a global state. Which i think is still necessary, here two examples:

payload: i see that value being used at least in two parts of the app:

1. Construct the call to send the message.
2. Payload section of the current transaction:

account (with derivedAccount, receiver &amount:

Transfer component s should be aware of these three values because:

1. Send Message button cannot be enabled until: a sender account is selected, dependent async calls are not finished, amount is provided and receiver account is valid.
2. To send the message we need all the values for the call.

Then is a matter to decide how are we going to handle the state management for the global values.
We all agree that it is going to be using React Context, but then we have two options: manage this global values by defining local states in each context file ( or hooks used in the context file with their respective updaters ) or use reducers which provides an out of a box dispatch mechanism.

Reducers in case the variables are not very dependent on each other introduce A LOT of boilerplate and I'm not sure if they add that much value if they are mixed with hooks anyway - my point is that it's hard to draw a line when to use a reducer and when to use hooks and migrating between one an the other is not easy. The main point of the architecture redesign is to set up boundaries and only then we can set up rules within these boundaries.

@tomusdrw I'm not sure if i understand completely what you are trying to say here. Are you saying that introducing action creators in the way i proposed add complexity because of we would be adding dependent reducers between each other? i'm not following you when you specifically said: I'm not sure if they add that much value if they are mixed with hooks anyway ... hooks in my proposal are mainly used for exposing the global variables and build abstraction layers for the component that will use those global values.

For your code i have the exact question as the i did for nikos in my reply: how are you sharing values into a global state?

Anyway in your file ./src/hooks/api/useStateCalls.ts it is not easy to me to see how you would share that data into a global state. Which i think is still necessary:

I have created a sample example in order to have the proposal I am mentioning in code.
Make sure you have a local node template running in order to be able to run this code: https://github.com/wirednkod/ideaContextSample

store.js => The store context that gathers the global states (can be 1 or more files)
useApi.js => this is a sample hook based on the proposal mentioned above that is used to either init the data at the launch of the app.
Sample1.js => Simple component that uses +, - and Reset buttons to control the Global state
Sample2.js => Simple component that user can fetch the data from API and either alter global state or not based on the result (different buttons)

On App.js takes place the use of executeCb in order to initialize the global state data from the API call (happens only once).

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 the useStateCall 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.

// ./src/hooks/api/useStateCalls.ts
import { useState, useCallback } from 'react';

export const getStateCall = async (chain, data, at): Codec => {
  const { getValuesByChain } = useChainGetters();
  const getChainValues = useCallback((chain) => getValuesByChain(chain), [getValuesByChain]);
  
  const {
    api,
    substrateValues: { estimatedFeeMethodName }
  } = getChainValues(chain);

  const params = [estimatedFeeMethodName, data];
  if (at) { params.push(at); }
  const response: Codec = await api.rpc.state.call<Codec>(...params); 
  // Add any transformation from api needed here
  return response;

  // [ToDr]  the extra `try-catch` is not necessary, errors are propagated by default.
};

// [ToDr] This function can be easily made generic to avoid duplication
export const useGenericCall = (getStateCall) => {
  const [isLoading, setIsLoading] = useMountedState<boolean>(false);
  const [error, setError] = useMountedState<unknown>(null);
  const [data, setData] = useMountedState<unknown>(null);
  
  // [ToDr] you need to always use `useCallback`, also worth to pass all the deps (note that indeed they will never change in this particular example, but better to be safe than sorry).
  const execute = useCallback(async (params...) => {
    try {
      setIsLoading(true);
      const stateCall = await getStateCall(...params);
      setData(stateCall);
      return stateCall;
    } catch (e) {
      setError(e);
      setIsLoading(false);
      // throw e; /// this is unnecessary and will trigger extra errors on the console - we did handle the exception already.
    }
  }, [setIsLoading, setData, setError, setIsLoading, getStateCall]);
  
  return {
    isLoading,
    error,
    data,
    execute,
  };
};

Regarding state management - I like your approach, there are few things that I'd like to point out though:

1. The global state does not need to be a reducer - it's fine to have top-level 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 :)
2. There is one more thing worth noting - components that don't use neither local nor global state at all - stateless components which simply get props externally. IMHO in current app and in your design they are not appreciated enough, but imho they are awesome and make it extremely easily to reason about the application. In my ideal view, some of the components should simply be responsible only for presentation logic and don't use any of domain-specific hooks at all (maybe some local useState for presentation state (like isToggled) sometimes). IMHO the more complex the display/component is - the more the component should avoid using domain-specific hooks.

@hbulgarini

@tomusdrw I'm not sure if i understand completely what you are trying to say here. Are you saying that introducing action creators in the way i proposed add complexity because of we would be adding dependent reducers between each other? i'm not following you when you specifically said: I'm not sure if they add that much value if they are mixed with hooks anyway ... hooks in my proposal are mainly used for exposing the global variables and build abstraction layers for the component that will use those global values.

My point is that we have both useState and useReducer 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 or useState.

For your code i have the exact question as the i did for nikos in my reply: how are you sharing values into a global state?

You either pass them via props or you pass them via context. I don't really know if there is any 3rd way of doing that. Examples:

function App() {
  const { apiConnection, setApiConnection } = useState(null);
  return <>
      <ReadOnlySubComponent api={apiConnection} />
      <ConnectingComponent setApiConnection={setApiConnection} />
  </>;
}

or

function App() {
  const { apiConnection, setApiConnection } = useState(null);
  return <>
      <ApiProvider connection={apiConnection}>
        <ReadOnlySubComponent />
      </ApiProvider>
     <SetApiProvider setConnection={setApiConnection}> 
        <ConnectingComponent />
     </SetApiProvider>
  </>;
}

function ReadOnlySubComponent() {
   const { connection } = useContext(ApiContext);
   ...
}

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:))

@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:

1. 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.

2. useApi.ts: I think there should be two separate hooks, the useApi.ts as it is in your example code nikos, but then the generic call should be extracted to a different process.

3. 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)

4. 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 a setState. In case those calls are not properly wrapped in a useCallback 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