Skip to content
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

[Feat] #121 - Tuist로 모듈 분리하기 #125

Merged
merged 9 commits into from
Mar 19, 2023
Merged

Conversation

0inn
Copy link
Contributor

@0inn 0inn commented Mar 17, 2023

🌴 PR 요약

🌱 작업한 브랜치

🌱 PR Point

기존에 회의했던 내용들 기반으로 Feature들 분리했습니다.
일단 작업 단위가 너무 커지기 전에 한 번 PR을 올려봅니다.
(지금도 크긴 한 것 같은데 죄송합니다. 생각보다 굉장히 오래 걸렸네여 . .)

📌 참고 사항

Feature 기반 모듈 분리

이와 같은 구조로 분리했고, RootFeature는 비워둔 상태입니다.

두 번 이상 의존되는 모듈은 Static의 경우 복사되기 때문에 중복된 코드가 존재하게 됩니다.
그래서 Interface 및 BaseFeatureDependency 등은 Dynamic으로 구현했습니다.

Interface는 화면 전환이 필요한 Feature들에 대해 의존성을 두었습니다.


Tuist

준호선배림이 예시로 준 코드 및 여러가지를 참고하여 구현했습니다.

Project+Templates 코드

혹쉬 예상되는 문제 상황 있으시면 바로 코멘트 부탁드립니다 . . !


Interface (화면 전환)

일단 Interface은 준호가 제안했던 방식으로 화면전환용으로만 구현해두었습니다.
추후에 승호님이 말씀하셨던 방식도 함께 고민하고 리팩하면 좋을 것 같아요.

SpalshFeatureInterface.swift

import Core
import BaseFeatureDependency

public protocol SplashFeatureInterface: BaseFeatureInterface { }

public protocol SplashFeature {
    func makeSplashVC() -> SplashFeatureInterface
    func makeNoticePopUpVC(noticeType: NoticePopUpType, content: String) -> SplashFeatureInterface
}

SplashVC.swift

import SplashFeatureInterface
import OnboardingFeatureInterface
import AuthFeatureInterface
import StampFeatureInterface

public class SplashVC: UIViewController, SplashFeatureInterface {
    
    // MARK: - Properties
    
    public var factory: (SplashFeature & OnboardingFeature & AuthFeature & StampFeature)!
}

// 화면전환

private func presentNoticePopUp(model: AppNoticeModel) {
    guard let isForcedUpdate = model.isForced else { return }
    let popUpType: NoticePopUpType = isForcedUpdate ? .forceUpdate : .recommendUpdate
    
    guard let noticePopUpVC = factory.makeNoticePopUpVC(noticeType: popUpType, content: model.notice).viewController as? NoticePopUpVC else {
        return
    }
    
    noticePopUpVC.closeButtonTappedWithCheck.sink { [weak self] didCheck in
        self?.recommendUpdateVersionChecked.send(didCheck ? model.recommendVersion : nil)
        noticePopUpVC.dismiss(animated: false)
        self?.checkDidSignIn()
    }.store(in: cancelBag)
    
    self.present(noticePopUpVC, animated: false)
}

타입 캐스팅

NoticePopUpVC의 인스턴스에 접근해야 해서 일단 다운캐스팅했습니다.
타 모듈간 인스턴스 접근 하는 애들은 의존성 주입 방식으로 바꿔뒀습니다.


기타 사항

  • Interface 네이밍 더 좋은 거 추천 받습니다.
  • 피드백 많이 많이 주세요 ! ! 🔥
  • 남은 피처 플로우 연결하고 있겠습니다 . . ^^

📸 스크린샷

기능 스크린샷
기능이름 스크린샷 첨부

📮 관련 이슈

@0inn 0inn added Feat 새로운 기능 구현 영인☁️ labels Mar 17, 2023
@0inn 0inn self-assigned this Mar 17, 2023
Copy link
Contributor

@L-j-h-c L-j-h-c left a comment

Choose a reason for hiding this comment

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

정말 대작업이네요... 새로운 코드베이스 보기 힘드셨을 텐데 고생 많으셨습니다!
노가다성 작업이 많은데 감사합니다 ㅜㅜ

지금 ModuleFactory를 DIContainer로 옮기는 과정에 있는거죠?

Comment on lines 11 to 19
public protocol BaseFeatureInterface {
var viewController: UIViewController { get }
}

public extension BaseFeatureInterface where Self: UIViewController {
var viewController: UIViewController {
return self
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

프로토콜 네이밍을 기능에 맞게 조금 더 구체적으로 바꾸는 것은 어떨까요? BaseFeature에 다른 프로토콜이 들어갈 수도 있어서 든 생각입니다 ㅎㅎ

Copy link
Contributor

Choose a reason for hiding this comment

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

RIBs에서는 ViewControllable이라고 쓰고 있네요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

음 . . RIBs를 따라가볼게요 ㅎ ㅎ

Comment on lines 12 to 14
public protocol StampFeatureInterface: BaseFeatureInterface { }

public protocol StampFeature {
Copy link
Contributor

Choose a reason for hiding this comment

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

요기 부분도 StampFeatureInterface -> 뷰의 생성에 관련된 네이밍?으로 고려해봐도 좋을 것 같아요!

@0inn
Copy link
Contributor Author

0inn commented Mar 18, 2023

지금 ModuleFactory를 DIContainer로 옮기는 과정에 있는거죠?

넵 !

Copy link
Member

@lsj8706 lsj8706 left a comment

Choose a reason for hiding this comment

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

와우..엄청난 대공사가 진행되고 있군요...
정말 대단합니다..! 혹시 뷰 연결 작업 하다가 기존 코드에서 잘 이해가 안되는 부분 있으면 언제든 연락주세요!

Copy link
Contributor

@L-j-h-c L-j-h-c left a comment

Choose a reason for hiding this comment

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

👍 👍

Comment on lines 12 to 17
public protocol SplashFeatureViewControllable: ViewControllable { }

public protocol SplashFeatureViewBuildable {
func makeSplashVC() -> SplashFeatureViewControllable
func makeNoticePopUpVC(noticeType: NoticePopUpType, content: String) -> SplashFeatureViewControllable
}
Copy link
Contributor

Choose a reason for hiding this comment

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

제가 놓친 부분이 있었네요..

특정 뷰 컨트롤러의 인스턴스에 접근해야 할 때, NoticeFeatureViewControllable 내부에 프로퍼티를 선언해 놓으면 다운캐스팅 없이 이용할 수 있습니다..!

그래서 지금처럼 SplashFeatureViewControllable로 관리하는 것을, 각 뷰 컨트롤러 마다 하나씩 프로토콜을 생성해주는 방식을 생각했었는데 어떨까요??

만약 괜찮고, 로드가 많은 것 같다면 작업 단위를 나누어도 될 것 같아요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

인정 . . 입니다
다운 캐스팅도 피하고 프로토콜에 의존하게 하는 게 더 좋은 방식같네요 . . (최고)
특정 뷰컨 인스턴스에 의존성을 가지는 부분이 여기밖에 없어서 작업량은 많지 않을 것 같습니다 ~ !
단순히 데이터 전달 목적으로 인스턴스에 접근하는 부분들은 생성자 주입 방식으로 수정했습니다.

//
//  SplashFeatureViewControllable.swift
//  SplashFeatureInterface
//
//  Created by 김영인 on 2023/03/16.
//  Copyright © 2023 SOPT-iOS. All rights reserved.
//

import Combine

import Core
import BaseFeatureDependency

public protocol SplashViewControllable: ViewControllable { }

public protocol NoticePopUpViewControllable: ViewControllable {
    var closeButtonTappedWithCheck: PassthroughSubject<Bool, Never> { get }
}

public protocol SplashFeatureViewBuildable {
    func makeSplashVC() -> SplashViewControllable
    func makeNoticePopUpVC(noticeType: NoticePopUpType, content: String) -> NoticePopUpViewControllable
}

Interface의 swift 파일명 : 00FeatureViewControllable
각 뷰컨 프로토콜명 : 00ViewControllable
각 피쳐 프로토콜명 : 00FeatureViewBuildable

로 하려고 하는데 더 나은 네이밍 추천받아요 ^^

Copy link
Contributor

Choose a reason for hiding this comment

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

고생하셨어요~~! 저도 딱 요대로 좋은 것 같아요 ㅎㅎ

Copy link
Member

@devxsby devxsby left a comment

Choose a reason for hiding this comment

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

오우 작업양이 제법 많네요..!!
정말 고생 많으셨습니다!! 최공 👍🏻

public static let workspaceName = "SOPT-iOS"
public static let deploymentTarget = DeploymentTarget.iOS(targetVersion: "16.0", devices: [.iphone])
public static let platform = Platform.iOS
public static let bundlePrefix = "com.sopt-stamp-iOS"
Copy link
Member

Choose a reason for hiding this comment

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

현재 sopt-stamp로 되어있는 부분 sopt-ios로 다같이 바꾸는 작업도 여기서 하면 좋을거같은데 어떻게 생각하시나요~?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lsj8706
com.sopt-iOS 로 번들 아이디 바꿔둬도 될까요 ? ?

Copy link
Contributor

@elesahich elesahich left a comment

Choose a reason for hiding this comment

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

image

멋져요 👏👏👏 고생 많으셨습니다!

@0inn 0inn merged commit ee2bc93 into sopt-makers:develop Mar 19, 2023
@0inn 0inn deleted the feat/#121 branch March 19, 2023 03:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feat 새로운 기능 구현 size/XXL 영인☁️
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feat] Tuist로 모듈 분리하기
5 participants