refactoring UI issue 해결과정

뚜비·2023년 9월 28일
0

2023 OSSCA - Argo Workflows

목록 보기
11/13

PR #11894

서론

아닛... 갑자기 메일이 와서 들어가보니..
멤버 분이 리팩토링을 해보라고 일거리(?)를 던져주시는게 아닌가?
지금 클래스형 컴포넌트로 되어있는 친구들을 함수형 컴포넌트로 바꾸는게 나의 임무
react hook이 있으니 바꾸버ㅗㄷ자
그럼 해야지 유후~



본론

나는 그 중에서 53번 친구를 선택했다
내가 만들 친구는

workflow template page에서 해당 template을 선택하면 detail page가 뜨고 그중에서 submit 버튼을 클릭할 시 요런 panel이 뜨는데


나는 그중에서 parameter에 들어갈 name(message)과 value(hello argo) 값 input 부분을 수정하면 된다.


실제로 옆에 workflow template를 보면 위와 같이 들어간 것을 확인할 수 있음!


해당 message parameter는 log에 출력해주는 듯?

코드를 어떻게 수정?

좀 어려운 부분은 다음과 같다.
1. 인터페이스는 어떻게 처리?
2. 생성자는 어떻게 처리할까?

해당 고민에 대한 부분을 다른 컨트리뷰션의 코드를 통해 해결할 수 있었다.
refactor(ui): workflow panel components from class to functional

  1. 인터페이스 중 Props는 그대로 둔다. Props는 외부에서 데이터를 가져와야 하기 때문. 대신 State 인터페이스는 useState를 활용한다.
  2. 생성자의 props는 함수의 파라미터로 받아오면 되고 나머지 변수들을 React Hook의 useState를 통해 관리하면 된다.

코드를 수정해보자

아래는 paramete input 부분이다.

import {Select, Tooltip} from 'argo-ui';
import * as React from 'react';
import {Parameter} from '../../../../models';
import {Utils} from '../../utils';

interface ParametersInputProps {
    parameters: Parameter[]; // Parameter 타입의 배열이 담긴 변수 parameters
    onChange?: (parameters: Parameter[]) => void; 
    // 매개변수ㅏ parameters이고 return 값이 void인 onChange 함수 
    // 인터페이스를 사용할 때 인터페이스에 정의되어 있는 속성을 모두 다 꼭 사용하지 않아도 됨 
    // 즉 ?가 있으면 객체에서 onChange 속성이 없어도 된다는 뜻
    // 이 함수는 set함수와 비슷한 역할을 하는 듯 
}

export class ParametersInput extends React.Component<ParametersInputProps, {parameters: Parameter[]}> {
    constructor(props: ParametersInputProps) {
        super(props);
        this.state = {parameters: props.parameters || []};
    }

    public render() {
        return (
            <>
                {this.props.parameters.map((parameter, index) => (
                    <div key={parameter.name + '_' + index} style={{marginBottom: 14}}>
                        <label>{parameter.name}</label>
                        {parameter.description && (
                            <Tooltip content={parameter.description}>
                                <i className='fa fa-question-circle' style={{marginLeft: 4}} />
                            </Tooltip>
                        )}
                        {(parameter.enum && this.displaySelectFieldForEnumValues(parameter)) || this.displayInputFieldForSingleValue(parameter)}
                    </div>
                ))}
            </>
        );
    }

    private displaySelectFieldForEnumValues(parameter: Parameter) { // 즉 값이 여러개고 선택가능한 필드인 경우 
        return (
            <Select
                key={parameter.name}
                value={Utils.getValueFromParameter(parameter)}
                options={parameter.enum.map(value => ({
                    value,
                    title: value
                }))}
                onChange={e => {
                    const newParameters: Parameter[] = this.state.parameters.map(p => ({
                        name: p.name,
                        value: p.name === parameter.name ? e.value : Utils.getValueFromParameter(p),
                        enum: p.enum
                    }));
                    this.setState({parameters: newParameters});
                    this.onParametersChange(newParameters);
                }}
            />
        );
    }

    private displayInputFieldForSingleValue(parameter: Parameter) { // 즉 textarea에 값을 보여줌 
        return (
            <textarea
                className='argo-field'
                value={Utils.getValueFromParameter(parameter)} // 원래는 기존의 해당 paramter 값을 보여줌 
                onChange={e => { // 만약 textarea 값에 변화가 감지되면 해당 parameterm 값을 새롭게 바꿔줘서 업데이트
                    const newParameters: Parameter[] = this.state.parameters.map(p => ({
                        name: p.name,
                        value: p.name === parameter.name ? e.target.value : Utils.getValueFromParameter(p),
                        enum: p.enum
                    }));
                    this.setState({parameters: newParameters});
                    this.onParametersChange(newParameters);
                }}
            />
        );
    }

    private onParametersChange(parameters: Parameter[]) {
        if (this.props.onChange) {
            this.props.onChange(parameters);
        }
    }
}
  • 기존 클래스형 생성자 중
    this.state = {parameters: props.parameters || []};는 어케 처리?

    기존의 this.state는 State 인터페이스 객체로 정의해서 한번에 할당해주고


this.setState{변수= 변화줄 값}로 변화를 한번에 처리한다.

해당 클래스형을 함수형으로 바꾸면

react Hook의 useState로 변수 선언을 해준 후

바로 사용하면 된다.

export class ParametersInput extends React.Component<ParametersInputProps, {parameters: Parameter[]}> {
    constructor(props: ParametersInputProps) {
        super(props);
        this.state = {parameters: props.parameters || []};
    }

이를

export function ParametersInput(props: ParametersInputProps) {
    const [parameters, setParameters] = useState<Parameter[]>(props.parameters || []);
  • 메소드들은 어케 처리?
    우선 unMount Mount 업데이트에 관한 내용이 없기 때문에 useEffect를 사용할 필요는 없다.
    또한 Select 컴포넌트에 자체적으로 해당 이벤트 처리함수가 포함되어 있기 때문에 우리는 값을 전달해주기만 하면 된다.
    async await이 필요한가?

기존 코드는 다음과 같다.

private displaySelectFieldForEnumValues(parameter: Parameter) {
        return (
            <Select
                key={parameter.name}
                value={Utils.getValueFromParameter(parameter)}
                options={parameter.enum.map(value => ({
                    value,
                    title: value
                }))}
                onChange={e => {
                    const newParameters: Parameter[] = this.state.parameters.map(p => ({
                        name: p.name,
                        value: p.name === parameter.name ? e.value : Utils.getValueFromParameter(p),
                        enum: p.enum
                    }));
                    this.setState({parameters: newParameters});
                    this.onParametersChange(newParameters);
                }}
            />
        );
    }

    private displayInputFieldForSingleValue(parameter: Parameter) {
        return (
            <textarea
                className='argo-field'
                value={Utils.getValueFromParameter(parameter)}
                onChange={e => {
                    const newParameters: Parameter[] = this.state.parameters.map(p => ({
                        name: p.name,
                        value: p.name === parameter.name ? e.target.value : Utils.getValueFromParameter(p),
                        enum: p.enum
                    }));
                    this.setState({parameters: newParameters});
                    this.onParametersChange(newParameters);
                }}
            />
        );
    }

    private onParametersChange(parameters: Parameter[]) {
        if (this.props.onChange) {
            this.props.onChange(parameters);
        }
    }

수정한 코드는 다음과 같다.

function displaySelectFieldForEnumValues(parameter: Parameter) {
        return (
            <Select
                key={parameter.name}
                value={Utils.getValueFromParameter(parameter)}
                options={parameter.enum.map(value => ({
                    value,
                    title: value
                }))}
                onChange={e => {
                    const newParameters: Parameter[] = parameters.map(p => ({
                        name: p.name,
                        value: p.name === parameter.name ? e.value : Utils.getValueFromParameter(p),
                        enum: p.enum
                    }));
                    setParameters(newParameters);
                    this.onParametersChange(newParameters);
                }}
            />
        );
    }

    function displayInputFieldForSingleValue(parameter: Parameter) {
        return (
            <textarea
                className='argo-field'
                value={Utils.getValueFromParameter(parameter)}
                onChange={e => {
                    const newParameters: Parameter[] = parameters.map(p => ({
                        name: p.name,
                        value: p.name === parameter.name ? e.target.value : Utils.getValueFromParameter(p),
                        enum: p.enum
                    }));
                    setParameters(newParameters);
                    onParametersChange(newParameters);
                }}
            />
        );
    }

    function onParametersChange(parameters: Parameter[]) {
        if (props.onChange) {
            props.onChange(parameters);
        }
    }
  • 궁금증

왜 props에서도 onChange가 있고 클래스에서도 parameter 값이 잇는 걸까? 내부에서 parameter가 변경이 되면 ParameterInput의 컴포넌트를 가지는 부모 컴포넌트의 workflowParameter도 변경되어야 하기 때문에

typesrcipt 설정
이제 elint랑 prettier를 활용해 코드개선이 잘 되었는지 확인해보자 yarn lint 해보자

async await

나의 질문
다름이 아니라 궁금한 점이 있어 질문드립니다. 컴포넌트 마다 asyn await 키워드를 사용한 함수가 있고 사용하지 않은 함수들이 있는데 그 차이는 무엇인가요!? 데이터를 스토리지에서 가져와야 하는 경우에만 async await 키워드를 추가해주는 것일까요?!

윤우님의 답변

  • 자바스크립트는 인터프리터 언어로 차례로 한줄씩 로직을 실행하게 되는데 이때 동기 비동기 개념이 적용되어서 비동기 로직은 순서에 맞더라도 나중에 실행되는 로직들이 있습니다! 그 중 하나가 fetch를 통해 http 통신을 하는 로직이 그러한데 async 함수를 작성하면 promise 를 반환하는 함수로 변환됩니다! 그럼 함수 내부에서 await 라는 문법을 사용할 수 있고 await 키워드를 붙이게 되면 실행되는 비동기 로직에 대한 응답을 기다렸다가 완료되고 난 후에 다음 코드 실행으로 넘어가게 됩니다
  • await 를 안썼을 경우 http 통신으로 데이터를 get -> http 통신이 아직 끝나지 않았는데 다음 로직 실행 -> 데이터가 없으므로 error 반환
    await 를 썼을 경우 -> http 통신으로 데이터를 get -> http 통신이 끝날때까지 기다렸다 응답을 받음 -> 다음로직 순차적 실행
  • 일반 자바스크립트의 동작은 기본이 동기적이기에 http 통신, 웹 api 같은 비동기로 동작하는 애들에게만 async 함수를 사용합니다!

즉 한마디로 순차적으로 코드가 실행된다고 해도 로직 자체가 비동기 로직인 경우가 있다.

  • 나의 추가질문
    그렇다면 통신이 필요한 함수의 내부 로직 중 응답에 대해 "기다림"이 필요한 경우 aysnc/awiat 키워드가 사용된다는 의미인것이죠?!

보통 비동기의 개념을 검색해보면 "작업을 요청했을 경우 작업이 종료될 때까지 기다리지 않고 다른 작업 하고 있다가~"라는 설명 혹은 "병렬적 처리"하는 설명이 있던데
해당 설명은 비동기 함수 내부(await 키워드는 async 함수 내부에서 사용가능하므로)가 아닌 비동기 함수와 다른 로직이 함께 사용되는 경우를 의미하는 것일까요?!

  • 윤우님의 답변
    비동기는 병렬이 맞습니다!
    소스코드실행이 끝나기전에
    다음코드가 실행이되죠
    그렇게 하면 안되기에
    await로 소스코드 실행이 끝날때까지 기다리게 만들어서
    동기처럼 동작하게해주는겁니다
    마지막 질문은 맞습니다!
    데이터를 받아와서
    State에 보관해야하는데
    데이터를 받아오기전에 SetState로 보관로직이 실행되면 에러가납니더
    그렇기에 순차적 실행을 보장해주려고 하는거죵

  • 즉 비동기는 병렬적 수행 -> 그중에서 응답이 꼭 필요한 경우 async/await 키워드를 사용

문제 발생


github action ui 테스트에서 에러 발생하였다.
Shadowed name : 'parameters'라는 것인데...
23. 타입 파라미터의 섀도잉을 피하라
즉, 이미 ParametersInput()에는 parameters라는 변수가 있는데 onParamtersChange()의 파라미터에 parameters

피드백

  1. 타입스크립트는 타입을 추론할 수 있어 생략 가능하다!
const [parameters, setParameters] = useState<Parameter[]>(props.parameters || []);

useState<type>으로 선언하지 않아도 된다는 뜻!!

const [parameters, setParameters] = useState(props.parameters || []);
  1. Anti pattern 크흡...
    기존에 굳이 왜 parameter를 따로 state로 추가하는지 이해가 안 갔는데.. 그럼에도 state로 추가해줬다!!
    근데 역시나.. 삭제해줘도 되는 것이었음..
    따라서 useState로 선언해준 부분은 아예 삭제!

  2. onChange는 꼭 필요한 함수로 설정해주자!
    기존에 props에 onChange?로 되어있는데 onChange는 있어도 되고 없어도 되는 함수가 아니기에.. 제거해주자

  3. 코드를 더 간결하게
    사실 기존에 있는 코드는 props에서 Parameter 값을 가져오는데 굳~~이 state에 저장하고 input이나 선택된 값이 다르면 굳이 state 값도 변경하고 props도 변경해준다.. 그냥 이럴거면 props 자체를 변경해줘도 되는 거 아닌가? 했는데 역시나...

onParameterChange함수를 새롭게 refactoring 해보자!!

  • 굳~이 각각 변경시키지 말고 onParameterChange 함수에 parameter 업데이트에 대한 기능을 작성
function onParameterChange(parameter: Parameter, value: string) {
        const newParameters: Parameter[] = props.parameters.map(p => ({
            name: p.name,
            value: p.name === parameter.name ? value : Utils.getValueFromParameter(p),
            enum: p.enum
        }));
        props.onChange(newParameters);
    }
  • 각 diplay의 onChange 함수를 onParameterChange에 통합 가능
    displaySelected는 e.value로 값을 가져오고 다른 diplayInputFields는 e.target.value로 가져오는게 문제인데..
    각각 다른 사용(접근 방식이 다름)이지만 외부에서 값을 각각 가져온 후 onParametrChane의 value 파라미터로서 사용하면 되니까!
    e.value vs. e.target.value in the other usage
function displaySelectFieldForEnumValues(parameter: Parameter) {
        return (
            <Select
                key={parameter.name}
                value={Utils.getValueFromParameter(parameter)}
                options={parameter.enum.map(value => ({
                    value,
                    title: value
                }))}
                onChange={e => onParameterChange(parameter, e.value)}
            />
        );
    }

    function displayInputFieldForSingleValue(parameter: Parameter) {
        return <textarea className='argo-field' value={Utils.getValueFromParameter(parameter)} onChange={e => onParameterChange(parameter, e.target.value)} />;
    }
  • 컴포넌트 내에 순서바꾸기
  1. state
  2. callbacks
  3. render
    함수 순으로 되어있는데
    onParamterChange는 callback 함수와 비슷하므로~


    그렇다고 합니다!!!
  • Refactoring Tip!!
    내가 주저주저 했던 썰을 풀었더니.. 아주 자세한 답변을 해주셨다...

    onChange가 실제로 optional 하다면 some state가 필요했을 거라고 한다!! optional 하지 않기 때문에 parameter가 state로 추가된 것이라고!! (불필요한 코드를 줄이자!!)

결과


submit-workflows-panel



parameter가 여러개인 경우

profile
SW Engineer 꿈나무 / 자의식이 있는 컴퓨터

0개의 댓글