[GitHub] [incubator-devlake] likyh commented on issue #2744: [refactor][config-ui] Some development refactor

[GitBox <gi...@apache.org>]

likyh commented on issue #2744:
URL: https://github.com/apache/incubator-devlake/issues/2744#issuecomment-1216553291

   @e2corporation I agree with you on the use of typescript. We can use ts gradually and discreetly. Now we can name `configuredProject` as `configuredProjectId` to avoid this question.
   
   >> 1. only update the local state (defined in the current component by useState) in the function of useEffect, which depends on the props (state passed by the parent).
   > > 2. call the function onXXX/handleXXX/setXXX passed by the parent only when listening on the local state bound with UI.
   1 and 2 mean we cannot use useEffect to update the parent state by depending on another parent state like:
   https://github.com/apache/incubator-devlake/blob/main/config-ui/src/pages/configure/settings/jira.jsx#L99-L105
   >
   > Not sure if I understand what problem you are trying to identify here, initial state is passed down from parent. In the case of JIRA (like all other providers) all it's settings is routed through one settings handler (onSettingsChange) which stores transformation settings keyed by Board ID (JIRA) and Project Name (GitHub) and Project ID (GitLab)
   
   I notice 2 refactors here:
   1. Look at this code. This `useEffect` deps on `configuredBoard` passed from parent and call `onSettingsChange` passed from parent. It will cause `onSettingsChange` to call unexpectedly when its parent change `configuredBoard`. Son component should not add new connections between the parent's state and the parent's callback. Or it will cause the parent component to need to know all run detail in its children.
   https://github.com/apache/incubator-devlake/blob/b31ec914f586ee043af6be18bb734dcbb29d6c59/config-ui/src/pages/configure/settings/jira.jsx#L99-L105
   2. Maybe some states can be defined in son components. Such as `configuredProject`/`configuredBoard` can be defined in `DataTransformations` instead of `create-blueprint.jsx`, because it just an UI state in step 3. Of couse, it's just an example to describe what situation I mean, or maybe there are some unavoidable reasons to define these states in page components. Just an example to show how to reduce vars in the global context.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org