feat: Strictly manage constants by adding hasValueInStringList, hasPr…

ssi02014 commented 6 months ago

Overview

타입을 좁혀주는 유틸 함수들을 활용한다면 constants 내의 상수들을 더욱 엄격하게 관리할 수 있을 것 같아 관련 작업을 진행했습니다 🙏

더 명확한 네이밍, 더 나은 방법이 있으면 의견주시면 감사드립니다!

또한, 제가 놓친 히스토리가 있다면 말씀주시면 감사드립니다 ! 🤗

PR Checklist

[x] I read and included theses actions below

I have read the Contributing Guide
I have written documents and tests, if needed.

changeset-bot[bot] commented 6 months ago

🦋 Changeset detected

Latest commit: 556e52c14dfda8fe442f32d66c3ef4e099f4da40

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

💥 An error occurred when fetching the changed packages and changesets in this PR

``` Some errors occurred when validating the changesets config: The package or glob expression "es-hangul" specified in the `fixed` option does not match any package in the project. You may have misspelled the package name or provided an invalid glob expression. Note that glob expressions must be defined according to https://www.npmjs.com/package/micromatch. ```

netlify[bot] commented 6 months ago

Deploy Preview for es-hangul ready!

Name	Link
Latest commit	556e52c14dfda8fe442f32d66c3ef4e099f4da40
Latest deploy log	https://app.netlify.com/sites/es-hangul/deploys/661cbb02d613c40008eff013
Deploy Preview	https://deploy-preview-38--es-hangul.netlify.app
Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

ssi02014 commented 6 months ago

@evan-moon 안녕하세요!! 빠른 피드백 감사드립니다 👍

이미지로 올려주신 부분은 disassembleCompleteHangulCharacter 이 undefined를 반환하는 점이 문제라고 생각합니다!

타입 시스템 관점에서 서브 타입은 상위 타입(슈퍼 타입)에 호환이 가능한 것으로 알고있습니다. (업 캐스팅)

// ex 업캐스팅, 다운캐스팅 예제
let str1: string = "";
let str2: "값" = "값";

str1 = str2; // 정상 (업캐스팅)

str2 = str1 // 타입 에러!! 'string' 형식은 '"값"' 형식에 할당할 수 없습니다. (다운 캐스팅)

따라서, 현재 @evan-moon 님께서 올려주신 이미지는 사실 해당 PR작업 이전 코드에서도 발생하는 문제입니다. 😭

결과적으로 말씀주신 부분은 undefined가 아님을 방지하면 해결할 수 있습니다.

const data = disassembleCompleteHangulCharacter('값');
const f = data?.first;
// as-is) f: string | undefined
// to-be) f: "ㄱ" | "ㄲ" | "ㄴ" | "ㄷ" | "ㄸ" | "ㄹ" | "ㅁ" | "ㅂ" | "ㅃ" | "ㅅ" | "ㅆ" | "ㅇ" | "ㅈ" | "ㅉ" | "ㅊ" | "ㅋ" | "ㅌ" | "ㅍ" | "ㅎ" | undefined

const foo = (v: string) => {
  return v;
};

foo(f); // 타입 에러, 'undefined' 형식은 'string' 형식에 할당할 수 없습니다.ts(2345)

if (f) { 
  foo(f); // as-is, to-be 모두 정상, 타입 에러 X
}

그래서 제 생각을 정리해서 말씀드리자면 타입스크립트는 기본적으로 업캐스팅을 허용하기 때문에 라이브러리 내부적으로 명확한 타입으로 관리하더라도 크게 문제가 없지 않을까 생각합니다 🙏 또한, 외부 사용자중에서도 명확한 타입으로 관리하고싶은 니즈도 충분히 있을 수 있을 것 같다는 생각을 했습니다 !

추가적으로 이런 명확하게 관리하는 상수 타입들을 export 하는 것은 어떨까요? 원하는 사람이라면 타입을 활용할 수 있게 열어두면 좋을 것 같습니다 ☺️ (-> 이번 작업에 중요한 내용은 아닙니다 😂)

evan-moon commented 6 months ago

@evan-moon 안녕하세요!! 빠른 피드백 감사드립니다 👍

이미지로 올려주신 부분은 disassembleCompleteHangulCharacter 이 undefined를 반환하는 점이 문제라고 생각합니다!

타입 시스템 관점에서 서브 타입은 상위 타입(슈퍼 타입)에 호환이 가능한 것으로 알고있습니다. (업 캐스팅)
// ex 업캐스팅, 다운캐스팅 예제
let str1: string = "";
let str2: "값" = "값";

str1 = str2; // 정상 (업캐스팅)

str2 = str1 // 타입 에러!! 'string' 형식은 '"값"' 형식에 할당할 수 없습니다. (다운 캐스팅)
따라서, 현재 @evan-moon 님께서 올려주신 이미지는 사실 해당 PR작업 이전 코드에서도 발생하는 문제입니다. 😭

결과적으로 말씀주신 부분은 undefined가 아님을 방지하면 해결할 수 있습니다.
const data = disassembleCompleteHangulCharacter('값');
const f = data?.first;
// as-is) f: string | undefined
// to-be) f: "ㄱ" | "ㄲ" | "ㄴ" | "ㄷ" | "ㄸ" | "ㄹ" | "ㅁ" | "ㅂ" | "ㅃ" | "ㅅ" | "ㅆ" | "ㅇ" | "ㅈ" | "ㅉ" | "ㅊ" | "ㅋ" | "ㅌ" | "ㅍ" | "ㅎ" | undefined

const foo = (v: string) => {
  return v;
};

foo(f); // 타입 에러, 'undefined' 형식은 'string' 형식에 할당할 수 없습니다.ts(2345)

if (f) { 
  foo(f); // as-is, to-be 모두 정상, 타입 에러 X
}
그래서 제 생각을 정리해서 말씀드리자면 타입스크립트는 기본적으로 업캐스팅을 허용하기 때문에 라이브러리 내부적으로 명확한 타입으로 관리하더라도 크게 문제가 없지 않을까 생각합니다 🙏 또한, 외부 사용자중에서도 명확한 타입으로 관리하고싶은 니즈도 충분히 있을 수 있을 것 같다는 생각을 했습니다 !

추가적으로 이런 명확하게 관리하는 상수 타입들을 export 하는 것은 어떨까요? 원하는 사람이라면 타입을 활용할 수 있게 열어두면 좋을 것 같습니다 ☺️ (-> 이번 작업에 중요한 내용은 아닙니다 😂)

앗 이제보니 저 에러는 undefined 때문이었네요. 빠르게 검토하다보니 실수가 있었습니다. 🙏 제가 두 타입 간의 관계에 대해 혼동이 있었네요. 좋은 피드백 감사합니다!

그렇다면 저는 이견없습니다! 좋은 기여 감사합니다 @ssi02014 님!

ssi02014 commented 6 months ago

@evan-moon 좋은 인사이트를 제공하는 라이브러리에 기여할 수 있게되어 기쁘네요! 빠른 검토 및 피드백 감사드립니다 👍

toss / es-hangul