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

Add assertion utility function #1602

Merged
merged 3 commits into from
Sep 8, 2023

Conversation

sungik-choi
Copy link
Contributor

@sungik-choi sungik-choi commented Sep 8, 2023

Self Checklist

  • I wrote a PR title in English and added an appropriate label to the PR.
  • I wrote the commit message in English and to follow the Conventional Commits specification.
  • I added the changeset about the changes that needed to be released. (or didn't have to)
  • I wrote or updated documentation related to the changes. (or didn't have to)
  • I wrote or updated tests related to the changes. (or didn't have to)
  • I tested the changes in various browsers. (or didn't have to)
    • Windows: Chrome, Edge, (Optional) Firefox
    • macOS: Chrome, Edge, Safari, (Optional) Firefox

Summary

개발자들의 디버깅 용이성, deprecated된 속성 등을 잘 파악할 수 있는 유틸 함수들을 추가합니다.

Details

  • process.env.NODE_ENV !== 'production' 인 케이스를 개발 환경으로 정의하고, 이 케이스에서만 에러를 보여주도록 합니다.
  • createContext 유틸에 assert 함수를 적용합니다.

Breaking change? (Yes/No)

No

References

@sungik-choi sungik-choi added the enhancement Issues or PR related to making existing features better label Sep 8, 2023
@sungik-choi sungik-choi self-assigned this Sep 8, 2023
@changeset-bot
Copy link

changeset-bot bot commented Sep 8, 2023

⚠️ No Changeset found

Latest commit: ae2b52b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@github-actions
Copy link
Contributor

github-actions bot commented Sep 8, 2023

Chromatic Report

🚀 Congratulations! Your build was successful!

@sungik-choi
Copy link
Contributor Author

unused module로 타입 체크가 실패하는데, 이후 PR에서 warn 함수를 사용할 예정이라 무시해주셔도 좋을 거 같습니다.

Copy link
Collaborator

@yangwooseong yangwooseong left a comment

Choose a reason for hiding this comment

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

어플리케이션에서 NODE_ENV='development' 으로 환경변수 주고 베지어를 사용하면 베지어 안에서도 NODE_ENV='development'로 인식하는게 맞을지 궁금하네요. 그 때 베지어는 이미 빌드된 패키지라서 베지어를 빌드할 때 주는 환경변수 (지금 없는 것 같은데 맞나요..?)를 사용하지 않나 라는 의문이 들었습니다.

@sungik-choi
Copy link
Contributor Author

sungik-choi commented Sep 8, 2023

어플리케이션에서 NODE_ENV='development' 으로 환경변수 주고 베지어를 사용하면 베지어 안에서도 NODE_ENV='development'로 인식하는게 맞을지 궁금하네요. 그 때 베지어는 이미 빌드된 패키지라서 베지어를 빌드할 때 주는 환경변수 (지금 없는 것 같은데 맞나요..?)를 사용하지 않나 라는 의문이 들었습니다.

라이브러리를 빌드하더라도 process.env가 라이브러리 빌드 시점의 NODE_ENV로 바인딩이 되지 않을거라 생각했어요.
이건 테스트해보는게 확실할 거 같습니다.


여기는 테스트 결과

esm build : process.env.NODE_ENV 가 그대로 있음

function isDev() {
  return process.env.NODE_ENV !== 'production';
}
class AssertionException extends Error {
  constructor(message) {
    super();
    this.name = 'AssertionException';
    this.message = message ?? 'assertion failed';
  }
}
function assert(predicate, message) {
  if (predicate) {
    return;
  }
  if (isDev()) {
    throw new AssertionException(message);
  }
}

export { assert };

test

export function assert(
  predicate: boolean,
  message?: string,
): asserts predicate {
  console.log('process.env.NODE_ENV: ', process.env.NODE_ENV)
  if (predicate) {
    return
  }

  if (isDev()) {
    throw new AssertionException(message)
  }
}
  • 위와 같이 코드를 변경하고
  • 피그마 플러그인(애플리케이션)에 createContext (내부에서 assert 호출)를 사용하는 Checkbox 컴포넌트를 임시로 추가
  • index.html 오픈 시 process.env.NODE_ENV: production 로그가 찍히는 걸 확인했어요

@sungik-choi sungik-choi merged commit 0d54036 into channel-io:main Sep 8, 2023
3 of 4 checks passed
@sungik-choi sungik-choi deleted the feat/assertion branch September 8, 2023 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issues or PR related to making existing features better
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants