Skip to content

Conversation

@ljdongz
Copy link
Collaborator

@ljdongz ljdongz commented May 5, 2025

#️⃣ 연관된 이슈

📝 작업 내용

RootView를 정의했고 로그인 상태에 따라 LoginView 또는 MainView를 보여주도록 구현했습니다.
DIContainer를 구현했습니다.

🎨 스크린샷

2025-05-05.4.10.12.mov

💬 추가 설명

@ljdongz ljdongz requested a review from f-lab-barry May 5, 2025 07:18
@ljdongz ljdongz self-assigned this May 5, 2025
Comment on lines 33 to 42

// TODO: Core 모듈 or Feature(공통) 모듈 중 어느 곳이 적합한지 고민
@Observable
final class AppState {
var isLoginned: Bool = false
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

앱 내에서 전역적으로 사용될 상태를 AppState라는 이름으로 정의했습니다.
RootView 하위에 서로 독립적인 View인 LoginView, MainView 간의 상태를 공유하기 위해 사용됩니다.

싱글톤으로 구현된 DIContainer에 AppState 객체를 등록해서 각 View(ViewModel)에서 가져와 사용하도록 했습니다.

고민했던 부분은, RootView에서 AppState 객체를 생성 후 LoginView, MainView에 파라미터로 넘겨주는 방법을 생각해 보았지만, 화면 뎁스가 깊어질 경우 파라미터로 전달되는 과정이 많아질 것을 염두하여 싱글톤 객체인 DIContainer를 통해 AppState 객체에 접근하는 방식으로 구현했습니다.

@ljdongz ljdongz linked an issue May 5, 2025 that may be closed by this pull request
}

func execute() -> String? {
authRepository.fetchCurrentUserID()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

struct AuthUser {
let id: String
}

class AuthUserManager: AuthUserManagerLogic {
var authUser: AuthUser?
fetch
save() { self.authUser = dd }
}

Base automatically changed from feature/#26 to develop May 6, 2025 17:06
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

앱 전역적으로 (로그인) 유저 정보 상태를 공유하기 위한 AuthManager를 구현했습니다.
이벤트 스트림을 활용했고, 이는 로그인 유저 상태 변경에 따른 처리를 즉각적으로 수행하기 위함입니다.

고민 사항

  1. 이후 Core 모듈로 이전할 예정인데, 모듈 이름을 어떻게 지을지 고민입니다.
    (Auth는 너무 제너럴한 거 같아서 외부 라이브러리와 네이밍 충돌이 발생하지 않을까라는 걱정입니다..)
  2. 분리된 모듈에 FirebaseAuth 모듈을 포함시켜 온전히 Auth 관련 모듈로 구현해보려고 하는데, FirebaseAuth를 위한 AuthService 객체를 만들어야 할 지, 아니면 Manager 내부에서 FirebaseAuth 메서드를 호출해도 괜찮을 지 고민입니다.

Comment on lines 15 to 17
struct UserModel {
let id: String
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Entity로 분리하는게 좋아보여요.

UserModel 보다는 User로 명확히 네이밍 부탁드려요~


/// Core
let linkMetadataProvider: LinkMetadataProvider = LinkMetadataProviderImpl()
let authSessionManager: AuthManager = AuthManagerImpl()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let authSessionManager: AuthManager = AuthManagerImpl()
let authManager: AuthManager = AuthManagerImpl()

이름 동일하게 authManager로 하는게 좋을 것 같아요 세션이 붙으면 혼란이 있을 것 같아서요~

}
}

func setCurrentUser(id: String) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set은 objective-C 스타일로 swift에서는 지양되고 있어요.
또 current라는 자세한 상태까지 외부에 노출할 필요는 없을 것 같아서 아래와 같은 형태가 좋아보여요~

  • updateUser(to user: User)
  • saveUser(to user: USer)

currentUser = user
}

func getCurrentUser() -> UserModel? {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

get 또한 objective-C 스타일로 swift에서는 지양되고 있어요.
단순히 user() -> User? 로 가거나 authManager 내부에 속해있으니 load() -> User? 정도로 두어도 괜찮을 것 같아요.

return currentUser
}

func clearCurrentUser() {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

요것도 clear()로만 두어도 될 것 같아요.

Comment on lines 33 to 39
func checkLoginStatus() {
if let user = Auth.auth().currentUser {
setCurrentUser(id: user.uid)
} else {
clearCurrentUser()
}
}
Copy link

@f-lab-barry f-lab-barry May 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

로그인 된 상태인지만 체크하고 싶다면 아래와 같은 프로퍼티를 둬도 괜찮을 것 같아요

var isAuthenticated: Bool {
  guard let currentUser else { return false }
  return true
}

final class AuthManagerImpl: AuthManager {
var userEvent: PassthroughSubject<UserModel?, Never> = .init()

private var currentUser: UserModel?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

맥락상 현재 상태인게 맞아서 currentUser 보다 user로만 두어도 괜찮을 것 같아요


protocol AuthManager {
var userEvent: PassthroughSubject<UserModel?, Never> { get }
var user: UserModel? { get }
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UserModel의 경우 FirebaseAuth에 User 타입이 정의되어있어 UserModel 이름으로 사용하고 있습니다

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아하 그렇군요 ~

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

그렇다면 AuthUser 등의 네이밍으로 가져가도 좋을 것 같아요

Comment on lines 25 to 28
@Observable
final class AuthManagerImpl: AuthManager {
var userEvent: PassthroughSubject<UserModel?, Never> = .init()
var user: UserModel?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

기존에 Manager 내부에서 이벤트 스트림을 사용하여 로그인 상태를 추적했던 방식에서,
SwiftUI에서 View가 상태 변화를 감지하기 위해 사용하는 방식인 Observable을 사용해서 RootView가 로그인 상태를 추적하도록 했습니다.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

좋습니다~

Copy link

@f-lab-barry f-lab-barry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AuthManager 개선 사항만 챙겨주세요~

@sonarqubecloud
Copy link

@ljdongz ljdongz merged commit fb4c559 into develop May 13, 2025
2 checks passed
@ljdongz ljdongz deleted the feature/#28 branch May 13, 2025 04:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] 루트 화면 설정 기능 구현

3 participants