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/core js with esmodule #361

Merged
merged 8 commits into from
Mar 16, 2020
Merged

Feat/core js with esmodule #361

merged 8 commits into from
Mar 16, 2020

Conversation

jinwoo-kim-nhn
Copy link
Contributor

@jinwoo-kim-nhn jinwoo-kim-nhn commented Mar 10, 2020

Please check if the PR fulfills these requirements

  • It's submitted to right branch according to our branching model
  • It's right issue type on title
  • When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)
  • 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)
  • It does not introduce a breaking change or has description for the breaking change

Description

  • babel7 은 builtins 옵션없이는 IE를 잘 지원하지 않는것으로 확인되어서.. 업데이트 보류
  • module.exports 방식을 esmodule 방식으로 코드 일괄수정
  • 더이상 잘 지원하지 않는 core-js2 를 core-js3 으로 업데이트
    • 전역 네임스페이스 오염없이 개별로 import 사용하려면 core-js-pure를 사용하라고 가이드 하고 있으므로 core-js-pure를 적용

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

Copy link
Member

@junghwan-park junghwan-park left a comment

Choose a reason for hiding this comment

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

와!! ESM으로 바꾸니 속이 다 후련하네요 ㅎㅎ 완료입니다.

@@ -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';
Copy link
Contributor

Choose a reason for hiding this comment

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

Promise를 관리하는 모듈을 만들어서 리턴하는 건 어떠세요? 추후에 라이브러리 변경이나 폴더 경로 변경이 있을 때도 수정이 간편할 것 같아요.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

네 수정이 잦으면 귀찮을것 같으니.. 좋은 방법 같네요. Promise를 리턴해주는 유틸함수를 만들어서 처리해보겠습니다.

import Component from '../interface/component';
import consts from '../consts';
import {rejectMessages, eventNames, keyCodes as KEY_CODES, componentNames, fObjectOptions, SHAPE_DEFAULT_OPTIONS} from '../consts';
Copy link
Contributor

Choose a reason for hiding this comment

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

prettier 쓰나요?
아니면 임포트 한개당 개행 하나가 어떨까요? 가로로 너무 길지 않은가 해서요.

Copy link
Contributor Author

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');
Copy link
Contributor

Choose a reason for hiding this comment

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

cls 함수 있었네요?ㅋ

Copy link
Contributor Author

Choose a reason for hiding this comment

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

저번 리뷰에서, 일부 적용했었어요 ㅎㅎ

Copy link
Contributor

@dongsik-yoo dongsik-yoo left a 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과 같은 패턴으로 바꿀 수 있어요. 좀 깔끔한 방법 같습니다.

@jinwoo-kim-nhn jinwoo-kim-nhn merged commit 5369a4a into master Mar 16, 2020
@jinwoo-kim-nhn jinwoo-kim-nhn deleted the feat/coreJsWithEsmodule branch March 20, 2020 08:12
HerlinMatos pushed a commit to EveryMundo/tui.image-editor that referenced this pull request Jul 2, 2020
* 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
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

Successfully merging this pull request may close these issues.

None yet

3 participants