Tracardi / tracardi

TRACARDI is a new HOME for your customer data. TRACARDI is an Composable API-first solution for any company that need inexpensive CDP to intergrate with.
https://www.tracardi.com
Other
527 stars 96 forks source link

We need to replace request with asyncRemote #284

Open atompie opened 2 years ago

atompie commented 2 years ago

We need to replace request with asyncRemote. Contrary to remote, asyncRemote can be used as awaitable.

Task

Find all occurrences of request calls and replace them with asyncRemote.

Use where possible the async way of calling the method. Example

try {
   const response = await asyncRemote({
     url: '/tracardi',
     method: "POST",
     data: data
   })

   // ...

} catch (e) {
   // ....
}

It is not possible to make call awaitable in useEffects. Then handle it with promise.

asyncRemote({
     url: '/tracardi',
     method: "POST",
     data: data
}).then((response) => {
            // ...            
}).catch((e)=> {
           // ...            
});

Test it thoroughly. Test every change separately.

Do not forget to set loading where it was used.

For example:

This code:

useEffect(() => {
        request({url: "/info/version"}, setLoading, () => {}, setReady);
    }, [])

Should be replaced like this. No await because it is in useEffects.

useEffect(() => {
        setLoading(true);
        asyncRemote(
            {url: "/info/version"}
        ).then((response) => {
            if (response?.status == 200) {
                setReady(response.data)
            }
        }).catch((e) => {
        }).finally(()=>{setLoading(false);});
    }, [])

mind the setLoading.

If it was not in useEffects always use it like this

try {
      setLoading(true);
       const response = asyncRemote(
            {url: "/info/version"}
        )
} catch(e) {
   // ...
} finally {
      setLoading(false);
}
M-RAY47 commented 2 years ago

Hello @atompie , Can you assign this issue to me?

atompie commented 2 years ago

Hello @atompie , Can you assign this issue to me?

@M-RAY47 Sure. Please make a PR after first change so we can CR it and see if you are going in the right direction. Otherwise if something is not right then you may loose a lot of time.

M-RAY47 commented 2 years ago

Hello @atompie , Can you assign this issue to me?

@M-RAY47 Sure. Please make a PR after first change so we can CR it and see if you are going in the right direction. Otherwise if something is not right then you may loose a lot of time.

Ok, I am on it.

M-RAY47 commented 2 years ago

Sorry to ask what PR and CR mean? Can you give some hint because I couldn't locate javaScript files, most of them are python files

atompie commented 2 years ago

@M-RAY47 Take a look at http://GitHub.com/tracardi/tracardi-gui

atompie commented 2 years ago

We keep gui files in this repo. I may transfer this issue to this repo.

atompie commented 2 years ago

Please fork tracardi-gui

M-RAY47 commented 2 years ago

Ok I got it, thanks

atompie commented 2 years ago

@M-RAY47 If you are working on this task please update the code frequently as the is continuous change and some of the parts of this task may be already done.

M-RAY47 commented 2 years ago

@atompie hello, sir I think you can close this issue from this project, because it should in the other project "Tracardi-gui"

caseyjkey commented 2 years ago

Is this still an issue?

atompie commented 2 years ago

@caseyjkey Yes we still have some places to refactor.