-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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/core js with esmodule #361
Conversation
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.
와!! ESM으로 바꾸니 속이 다 후련하네요 ㅎㅎ 완료입니다.
src/js/command/addShape.js
Outdated
@@ -3,10 +3,9 @@ | |||
* @fileoverview Add a shape | |||
*/ | |||
import commandFactory from '../factory/command'; | |||
import Promise from 'core-js/library/es6/promise'; | |||
import consts from '../consts'; | |||
import Promise from 'core-js-pure/features/promise'; |
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.
Promise를 관리하는 모듈을 만들어서 리턴하는 건 어떠세요? 추후에 라이브러리 변경이나 폴더 경로 변경이 있을 때도 수정이 간편할 것 같아요.
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.
네 수정이 잦으면 귀찮을것 같으니.. 좋은 방법 같네요. Promise를 리턴해주는 유틸함수를 만들어서 처리해보겠습니다.
src/js/component/shape.js
Outdated
import Component from '../interface/component'; | ||
import consts from '../consts'; | ||
import {rejectMessages, eventNames, keyCodes as KEY_CODES, componentNames, fObjectOptions, SHAPE_DEFAULT_OPTIONS} from '../consts'; |
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.
prettier 쓰나요?
아니면 임포트 한개당 개행 하나가 어떨까요? 가로로 너무 길지 않은가 해서요.
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.
넵 반영하겠습니다. 감사합니다.
@@ -321,8 +321,8 @@ class Ui { | |||
_makeMenuPartitionElement() { | |||
const partitionElement = document.createElement('li'); | |||
const partitionInnerElement = document.createElement('div'); | |||
partitionElement.className = util.cls('item'); | |||
partitionInnerElement.className = util.cls('icpartition'); | |||
partitionElement.className = cls('item'); |
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.
cls
함수 있었네요?ㅋ
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.
저번 리뷰에서, 일부 적용했었어요 ㅎㅎ
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.
[03/13] 리뷰 완료합니다. 수고하셨습니다.
나중에 시간되면 alias
도 한번 알아보세요. ../../util
과 같은 패턴을 @src/util
과 같은 패턴으로 바꿀 수 있어요. 좀 깔끔한 방법 같습니다.
* feat: babel version up with change module system esmodule * feat: enable useBuiltIns option of bable7 * fix: fixed config middle commit * feat: update corejs with esmodule syntax * apply codereview * apply codereview - 2
Please check if the PR fulfills these requirements
fix #xxx[,#xxx]
, where "xxx" is the issue number)Description
Thank you for your contribution to TOAST UI product. 🎉 😘 ✨