fix: Remove 'editorInstance' from 'data' for memory issue

junghwan-park commented 4 years ago

Please check if the PR fulfills these requirements

[x] It's submitted to right branch according to our branching model
[x] It's right issue type on title
[x] When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)
[x] The commit message follows our guidelines
[ ] Tests for the changes have been added (for bug fixes/features)
[ ] Docs have been added/updated (for bug fixes/features)
[x] It does not introduce a breaking change or has description for the breaking change

Description

Remove editorInstance from data() {} for memory issue.

Thank you for your contribution to TOAST UI product. 🎉 😘 ✨

jinwoo-kim-nhn commented 4 years ago

수고하셨습니다.

js87zz commented 4 years ago

이번 수정사항은 아니긴 한대, 이 부분도 변경하면 좋을 것 같아요. 현재 모든 이벤트들에 대해서 무조건 이벤트 리스너를 등록하고 있는데, 이벤트 리스너가 props로 넘어오는 것만 걸어줘야될 것 같아요.

addEventListener() {
   editorEvents.forEach(eventName => {
      this.editorInstance.on(eventName, (...args) => this.$emit(eventName, ...args));
  });
  // 수정 후
  Object.keys(this.$listeners).forEach(eventName => {
     this.editorInstance.on(eventName, (...args) => this.$emit(eventName, ...args));
  }
}

junghwan-park commented 4 years ago

@js87zz @dongsik-yoo 코드 리뷰 반영했습니다. 확인 부탁드립니다~!

dongsik-yoo commented 4 years ago

리뷰 완료입니다.별다른 의견 없습니다.

js87zz commented 4 years ago

이견 없습니다. 리뷰 완료입니다!

nhn / toast-ui.vue-image-editor

fix: Remove 'editorInstance' from 'data' for memory issue #19

Please check if the PR fulfills these requirements

Description