search

toss / es-hangul

A modern JavaScript library for handling Hangul characters.
https://es-hangul.slash.page/
MIT License
1.24k stars 83 forks source link

[Bug] 잘못 노출된 불필요 인터페이스 점진적 제거가 필요합니다 #158

Closed manudeli closed 1 month ago

manudeli commented 2 months ago

모든 인터페이스가 *로 노출되면 안될 것 같아보여서(테스트용으로 export했을 수 있기 때문에) 현재 이미 노출되고 있는 것은 노출되도록 변경했습니다.

utils와 같은 폴더의 경우 노출되면 안되는 인터페이스 같아서 이슈로도 함께 남겨요. 잘못 노출된 인터페이스는 점진적으로 제거되면 좋을 거 같아요

okinawaa commented 2 months ago

정말 감사합니다!! #157

manudeli commented 2 months ago

이 이슈는 아직 해결된 이슈가 아닌걸로 보여서 reopen할게요

manudeli commented 2 months ago

Suspensive에서는 tsup설정에 아래와 같이 다루고 있는데요. 1 depth의 인터페이스만 노출시키기 위함이에요

import type { Options } from 'tsup'

export const options: Options = {
  clean: true,
  banner: { js: '"use client"' },
  format: ['cjs', 'esm'],
  target: ['chrome51', 'firefox53', 'edge18', 'safari11', 'ios11', 'opera38', 'es6', 'node14'],
  entry: ['src/*.{ts,tsx}', '!**/*.{spec,test,test-d,bench}.*'], // src의 depth에서 있는 파일은 모두 노출되기 위한 것으로 간주. 폴더 내에 잇으면 노출되지 않는 것으로 생각하고 있습니다. _internal과 같은 방식도 직관적이고 좋은 것 같습니다
  outDir: 'dist',
  sourcemap: true,
  dts: true,
}
okinawaa commented 2 months ago

잘못노출된 인터페이스가 아직 제거가 안되었기 때문에 리오픈 하신건가요? 그렇다면 major업데이트시 제거되면 이슈가 닫히는것인가요?

manudeli commented 2 months ago

잘못노출된 인터페이스가 아직 제거가 안되었기 때문에 리오픈 하신건가요?

맞아요

그렇다면 major업데이트시 제거되면 이슈가 닫히는것인가요?

이 경우는 의도된 인터페이스 노출이 아니기 때문에 메이저를 올리지 않고 버그픽스로 보고 패치버전 업데이트에서 챙겨가야 한다고 생각해요. 어떨까요?

okinawaa commented 2 months ago

사용자의 서비스 코드가 변경되어야한다면, 웬만하면 메이저 업데이트를 하면 좋지않나요? 저는 주로 라이브러리 패치버전이 업데이트되었다고, BC가 존재한다고는 생각을 안해봤어서요. 기존 es-hangul에 존재하던 함수가 사라지는것이므로, 사용자 서비스 코드가 변경되어야하고 이는 BC이고 메이저 업데이트가 적절할것 같은데 혹시 제가 잘못 알고 있다면 알려주세요!!

manudeli commented 2 months ago

사용자의 서비스 코드가 변경되어야한다면, 웬만하면 메이저 업데이트를 하면 좋지않나요? 저는 주로 라이브러리 패치버전이 업데이트되었다고, BC가 존재한다고는 생각을 안해봤어서요. 기존 es-hangul에 존재하던 함수가 사라지는것이므로, 사용자 서비스 코드가 변경되어야하고 이는 BC이고 메이저 업데이트가 적절할것 같은데 혹시 제가 잘못 알고 있다면 알려주세요!!

이것을 버그로 볼 것이냐 아니냐에 따라 다를 거 같아요

라이브러리에서 의도하지 않게 함수에 타이핑을 잘못하거나 동작이 잘못 동작한 것을 사용하는 경우에 올바르게 패치한 경우에도 사용자 입장에서는 달라질 수 있을 거 같아요. 그래서 버그픽스인건지 의도된 변경인 것인지에 따라 다르게 봐야할 거 같아요. 저의 생각에는 메이저 업데이트도 좋은 방법이라고 생각하기는 해요. 하지만 마케팅적인 측면에서 메이저버전에서 사용자가 기대하는 것은 BREAKING CHANGE가 아니라 많은 기능 추가라고 생각해요.

메이저이든 패치든 이 이슈는 해결전에 모두에게 공유되어야하고 아직 close하지 않는 것이 더 좋다고 생각해요. 어떤 방식이든 해결되면 좋겠습니다

okinawaa commented 2 months ago

패치버전에서 BC가 있다는것을 예측하지 못할 수 있으므로, 제공하던 함수를 제공하지 않는것은 메이저버전 변경으로 처리하도록 하면 좋겠어요. 패치버전을 업데이트했는데, 서비스 코드를 고쳐야하면, 라이브러리에 대한 신뢰가 떨어질 것 같습니다. (또 언제든 깨질 수 있다는 점)

실제, 서비스에서 사용하는 라이브러리 메이저 업데이트를 주저하는 이유중 가장 큰 것이 BC가 있을 수도 있다는 점이기 때문입니다. 이 부분은 manudeli님도 크게 반대하지는 않는 입장이신 것 같아서 메이저 업데이트때 이 이슈 반영할게요.

manudeli commented 2 months ago

다음 버전에서는 제거된다는 표시가 필요해보여요

Deprecated jsdoc을 추가하면 좋을 거 같아요