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

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

Closed
manudeli opened this issue Jun 30, 2024 · 9 comments · Fixed by #189
Closed
Assignees

Comments

@manudeli
Copy link
Member

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

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

@manudeli manudeli changed the title 잘못 노출된 불필요 인터페이스 점진적 제거가 필요합니다 [Bug] 잘못 노출된 불필요 인터페이스 점진적 제거가 필요합니다 Jun 30, 2024
@okinawaa
Copy link
Member

정말 감사합니다!! #157

@manudeli
Copy link
Member Author

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

@manudeli manudeli reopened this Jun 30, 2024
@manudeli
Copy link
Member Author

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
Copy link
Member

okinawaa commented Jul 1, 2024

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

@manudeli
Copy link
Member Author

manudeli commented Jul 1, 2024

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

맞아요

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

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

@okinawaa
Copy link
Member

okinawaa commented Jul 1, 2024

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

@manudeli
Copy link
Member Author

manudeli commented Jul 1, 2024

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

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

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

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

@okinawaa
Copy link
Member

okinawaa commented Jul 2, 2024

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

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

@manudeli
Copy link
Member Author

manudeli commented Jul 2, 2024

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

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

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 a pull request may close this issue.

2 participants