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

js - oop - mvvm 3강 코드 중 질문사항 #177

Open
p-iknow opened this issue Feb 1, 2020 · 0 comments
Open

js - oop - mvvm 3강 코드 중 질문사항 #177

p-iknow opened this issue Feb 1, 2020 · 0 comments

Comments

@p-iknow
Copy link
Owner

p-iknow commented Feb 1, 2020

소스코드 링크
3강 코드 기준입니다만, 4강 코드에서도 동일한 문제가 있습니다.
이슈가 있어 나름으로 해결했으나 이 해결방법에도 문제가 있습니다. 좋은 방법이 있으시면 피드백 부탁드립니다.

이슈 및 배경

image

  viewmodelUpdated(updated, viewmodel) {
    const items = {};
    this.#items.forEach(item => {
      items[item.viewmodel] = [
        type(viewmodel[item.viewmodel], ViewModel),
        item.el
      ];
    });
    updated.forEach(v => {
      if (!items[v.subKey]) return;
      const [vm, el] = items[v.subKey];
      const processor = this.#processors[v.cat];
      if (!el || !processor) return;
      processor.process(vm, el, v.k, v.v);
    });
  }

Binder Class 의 viewmodelUpdated 메소드 내부에서 ViewModel Class의 인스턴스인 viewModel 변수를 사용하고 있습니다.

viewmodel 변수는 해당 메소드에서 인자로 전달받거나,Binder 인스턴스 생성시에 constructor 에도 전달되지 않았습니다. 결국에는 전역 변수(ViewModel의 인스턴스)인 viewmodel 값을 사용하고 있다는 뜻이죠.

Hika 님의 소스는 4.js 파일 하나로 되어있기에 전역 viewmodel 값을 참조해도 오류가 생기지 않지만, 각 클래스를 별도 모듈로 나눠 import해서 쓰면 viewmodel 값은 undefined가 됩니다.

단순히 오류가 난다는 사실 이외에도 메소드 선언시에 인스턴스인 viewmodel 변수를 사용하는 것 자체가 문제가 있어 보입니다.

나름의 해결 방안

image
image

// Binder.js
export const Binder = class extends ViewModelListener {
 ...
  viewmodelUpdated(updated, viewmodel) {
    const items = {};
    this.#items.forEach(item => {
      items[item.viewmodel] = [
        type(viewmodel[item.viewmodel], ViewModel),
        item.el
      ];
    });
    updated.forEach(v => {
      if (!items[v.subKey]) return;
      const [vm, el] = items[v.subKey];
      const processor = this.#processors[v.cat];
      if (!el || !processor) return;
      processor.process(vm, el, v.k, v.v);
    });
  }
 ...
};
const ViewModel = class extends ViewModelListener {
 ...
  notify() {
    this.#listeners.forEach(v => v.viewmodelUpdated(this.#isUpdated, this));
  }
};

Binder class 에서 viewmodelUpdated(updated, viemodel) 메소드가 인자 2개를 받도록 하고

ViewModel Class의 notify 메소드에서 this.#listeners.forEach(v => v.viewmodelUpdated(this.#isUpdated, this)); 두번째 인자로 viewmodel을 전달 하면 일단 문제는 해결됩니다.

위 수정의 문제점

기존 코드의 viewmodel 변수는 최상위 root viewmodel(내부에 wrapper, content 속성을 가진)였으나, 코드 변경이후 각 this 에 따라 viewmodel 이 달라집니다.

여기까지가 제가 생각해 본 부분인데 어떻게 하면 이 문제를 해결할 수 있을까요?

Hika 님 답변

원인은 제가 render의 코드를 그대로 복사해서 그래요. render는 인자로 viewmodel이 들어와서 전역바인딩이 아니죠. 이론적으로 binder에게는 두가지 선택지가 있습니다. 한번의 하나의 viewmodel과만 바인딩하겠다고하면 필드로 잡아주면 될테고, 아니면 역시 viewmodelValue에 직접 들어오는 방식입니다.
후자가 나을듯하네요. 3강기준으로
 

const ViewModelListener = class{
 viewmodelUpdated(viewmodel, updated){throw "override";}
};
 
const ViewModel = class extends ViewModelListener{
 ...
 notify(){
  this.#listeners.forEach(v=>v.viewmodelUpdated(this, this.#isUpdated));
 }
};
 
const Binder = class extends ViewModelListener{
 ..
 viewmodelUpdated(viewmodel, updated){
  ...
 }
 ..
};

이렇게 직접 viewmodel 을 인자로 보내주는게 젤 나을거에요. 4강에서는 notifyViewModel이 아니라 ViewModelSubject가 하기 때문에 3강처럼 해결할 수는 없습니다 ^^. 그렇게하면 엄격하게는 Binder의 리스너가 ViewModelSubjectViewModel로 다운캐스팅한 꼴이 되어 LSP위반이 됩니다. 그래서 ViewModelSubjectnotifynotify(){throw "override"}abstract하고 ViewModel로 가져와서 구현하면 됩니다. 그러면 watch시점에 명시적으로 ViewModel을 구독하고 있으니까 리스너의 첫번째 인자도 ViewModel 온다는 것을 확정할 수 있으니까요 ^^

수정 4.js gist link

18번째줄에 리스너의 첫번째인자가 제네릭처럼 풀린 target인자를 갖습니다. 이 인자는 나중에 확정됩니다.
57번째줄subjectnotify할때 notifyTarget을 보내주는데 이게 바로 추상속성입니다.
72번째줄ViewModelnotifyTarget을 확정지어줍니다.
183째줄에 바인더가 ViewModel을 구독했으므로
164번째줄의 리스너에 targetViewModel로 확정됩니다.
168번째줄에 드디어 target으로부터 서브뷰모델을 얻게 됩니다.

@p-iknow p-iknow changed the title js - oop - mvvm 강 코드 중 질문사항 js - oop - mvvm 3강 코드 중 질문사항 Feb 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant