-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Feature/BAR-2] Async Boundary 구현 #1
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
import type { ReactNode } from 'react'; | ||
import { Component, type PropsWithChildren } from 'react'; | ||
|
||
import CommonErrorScreen from './ErrorScreen'; | ||
|
||
interface ErrorBoundaryProps { | ||
errorFallBack?: ReactNode; | ||
onError?: (error: Error) => void; | ||
} | ||
|
||
class ErrorBoundary extends Component< | ||
PropsWithChildren<ErrorBoundaryProps>, | ||
{ error: Error | null } | ||
> { | ||
error = null; | ||
|
||
componentDidCatch(error: Error) { | ||
if (this.props.onError) { | ||
this.setState({ | ||
error, | ||
}); | ||
} | ||
} | ||
|
||
render() { | ||
const { error } = this.state; | ||
|
||
if (error) { | ||
if (this.props.errorFallBack) { | ||
return this.props.errorFallBack; | ||
} | ||
|
||
return <CommonErrorScreen error={error} />; | ||
} | ||
|
||
return this.props.children; | ||
} | ||
} | ||
|
||
export default ErrorBoundary; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
interface CommonErrorScreenProps { | ||
error: Error; | ||
} | ||
|
||
// TODO: 의도되지 않은 에러에 대한 공통 스크린 정의 (404, 400, 500 등은 에러 페이지로 구현) | ||
const CommonErrorScreen = ({ error }: CommonErrorScreenProps) => { | ||
if (!error) { | ||
window.location.reload(); | ||
} | ||
|
||
return <>{JSON.stringify(error)}</>; | ||
}; | ||
|
||
export default CommonErrorScreen; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
import { type PropsWithChildren, type ReactNode, Suspense } from 'react'; | ||
|
||
import LoadingView from '../Loading'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Loading과 LoadingView 중 하나로 통일하는 건 어떤가요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
혹시 통일하는게 더 명시적일까요 ?? |
||
import ErrorBoundary from './components/ErrorBoundary'; | ||
|
||
interface AsyncBoundaryProps { | ||
loadingFallback?: ReactNode; | ||
} | ||
|
||
const AsyncBoundary = ({ | ||
children, | ||
loadingFallback = <LoadingView />, | ||
}: PropsWithChildren<AsyncBoundaryProps>) => { | ||
return ( | ||
<ErrorBoundary> | ||
<Suspense fallback={loadingFallback}>{children}</Suspense> | ||
</ErrorBoundary> | ||
); | ||
}; | ||
|
||
export default AsyncBoundary; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
const Spinner = () => { | ||
return <>spinner</>; | ||
}; | ||
|
||
export default Spinner; |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이후 케이스 추가가 필요합니다. AsyncBoundary뿐 아니라 별도의 컴포넌트로 도 사용될 수 있게 분리 |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,18 @@ | ||||||
import type { FC } from 'react'; | ||||||
|
||||||
import Spinner from './components/Spinner'; | ||||||
|
||||||
type LoadingViewType = 'spinner'; | ||||||
|
||||||
interface LoadingViewProps { | ||||||
type?: LoadingViewType; | ||||||
} | ||||||
|
||||||
const LoadingView: FC<LoadingViewProps> = ({ type = 'spinner' }) => { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
React.FC는 지양하는 게 좋아서 이렇게 작성하면 어떨까요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 우선 토픽 공유주셔서 감사해요 ! 알고 있던 토픽인데 현재는 해당하지 않는 이슈입니다. 컨벤션 영역이라고 생각되어서 차주 회의에 얘기해보면 좋을것 같습니다. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 요약하면 React18부터는 children을 암묵적으로 가지지 않습니다. |
||||||
switch (type) { | ||||||
default: | ||||||
return <Spinner />; | ||||||
} | ||||||
}; | ||||||
|
||||||
export default LoadingView; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
loading과 마찬가지로 팩토리가 필요.
추후 추가할 예정입니다 !