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

Remove path alias of tsconfig and use tilde(~) specifier #1109

Merged
merged 13 commits into from
Mar 20, 2023

Conversation

Dogdriip
Copy link
Member

@Dogdriip Dogdriip commented Jan 19, 2023

Self Checklist

  • I wrote a PR title in English.
  • I added an appropriate label to the PR.
  • I wrote a commit message in English.
  • I wrote a commit message according to the Conventional Commits specification.
  • I added the appropriate changeset for the changes.
  • [Component] I wrote a unit test about the implementation.
  • [Component] I wrote a storybook document about the implementation.
  • [Component] I tested the implementation in various browsers.
    • Windows: Chrome, Edge, (Optional) Firefox
    • macOS: Chrome, Edge, Safari, (Optional) Firefox
  • [New Component] I added my username to the correct directory in the CODEOWNERS file.

Related Issue

Summary

  • tsconfig에서 정의하는 path alias(paths)를 제거하고, 절대경로 import를 tilde(~) specifier로 사용할 수 있도록 합니다.

Details

변경사항

  • As-is
    • 다음과 같이 tsconfig에서 각 경로에 대해 일일히 절대경로를 설정해 줍니다.
    • "paths": {
      "Components/*": ["src/components/*"],
      "Constants/*": ["src/constants/*"],
      "Foundation": ["src/foundation"],
      "Foundation/*": ["src/foundation/*"],
      "Hooks/*": ["src/hooks/*"],
      "Layout/*": ["src/layout/*"],
      "Providers/*": ["src/providers/*"],
      "Types/*": ["src/types/*"],
      "Utils/*": ["src/utils/*"],
      "Worklets/*": ["src/worklets/*"]
      }
  • To-be
    • 절대경로 개별 설정을 삭제하고, tilde 기호를 사용한 ~/* 경로를 ./*로 인식하도록 합니다.
    • 이에 따라 절대경로 import가 다음과 같이 변경됩니다:
      • import ... from 'Foundation'import ... from '~/src/foundation'
      • import ... from 'Utils/testUtils'import ... from '~/src/utils/testUtils'
      • 등등...

설명

  • 지금까지 느꼈던, 그리고 생각해 봤던 기존 방식의 문제점은 다음과 같습니다:
    • 디렉토리 변경에 유연하지 않습니다. 단적으로 src 디렉토리 내에 src/pages, src/images 등 새로운 폴더가 생길 때마다 tsconfig에서 이를 매핑해주어야 하며, Jest 설정 등도 동일하게 변경해주어야 합니다. 이는 Use Parcel as bundler instead of Rollup #1097 (comment) 에서도 설명한 바 있습니다.
      • moduleNameMapper: {
        '\\.(jpg|ico|jpeg|png|gif|eot|otf|webp|svg|ttf|woff|woff2|mp4|webm|wav|mp3|m4a|aac|oga)$':
        'identity-obj-proxy',
        '\\.(css|less|scss|sass)$': 'identity-obj-proxy',
        'Components/(.*)$': '<rootDir>/src/components/$1',
        'Constants/(.*)$': '<rootDir>/src/constants/$1',
        '^Foundation$': '<rootDir>/src/foundation',
        'Foundation/(.*)$': '<rootDir>/src/foundation/$1',
        'Hooks/(.*)$': '<rootDir>/src/hooks/$1',
        'Layout/(.*)$': '<rootDir>/src/layout/$1',
        'Providers/(.*)$': '<rootDir>/src/providers/$1',
        'Types/(.*)$': '<rootDir>/src/types/$1',
        'Utils/(.*)$': '<rootDir>/src/utils/$1',
        'Worklets/(.*)$': '<rootDir>/src/worklets/$1',
        },
  • 앞으로 문제가 될 수도 있는 점들은 다음과 같습니다:
    • 일부 번들러 및 이에 포함된 Transformer에서는 baseUrlpaths에 의존하는 절대경로 세팅을 인식하지 못합니다. Parcel의 경우가 대표적으로, 대안으로 tilde나 absolute(/) specifier를 기본 지원합니다. #

Breaking change or not (Yes/No)

사용처에서는 없습니다.

References

@Dogdriip Dogdriip added refactoring Issue or PR related to refactoring with no functional changes suggestion labels Jan 19, 2023
@Dogdriip Dogdriip self-assigned this Jan 19, 2023
@changeset-bot
Copy link

changeset-bot bot commented Jan 19, 2023

⚠️ No Changeset found

Latest commit: 3c09b5c

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 Jan 20, 2023

Chromatic Report

🚀 Congratulations! Your build was successful!

@Dogdriip Dogdriip marked this pull request as draft January 20, 2023 09:13
Copy link
Contributor

@guswnsxodlf guswnsxodlf left a comment

Choose a reason for hiding this comment

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

eslint 룰이 변경되어야 할 것 같네요

Copy link
Contributor

@sungik-choi sungik-choi left a comment

Choose a reason for hiding this comment

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

👍

packages/bezier-react/.storybook/preview.js Outdated Show resolved Hide resolved
@Dogdriip
Copy link
Member Author

eslint 룰이 변경되어야 할 것 같네요

import/extensions 룰 관련해서 뭔가 꼬인 것 같아서, #1114 머지 이후에 살펴보겠습니다 😓

@sungik-choi
Copy link
Contributor

Rebase 필요

@codecov
Copy link

codecov bot commented Mar 14, 2023

Codecov Report

Patch coverage: 96.56% and no project coverage change.

Comparison is base (f01dd49) 77.61% compared to head (3c09b5c) 77.61%.

Additional details and impacted files
@@           Coverage Diff            @@
##           next-v1    #1109   +/-   ##
========================================
  Coverage    77.61%   77.61%           
========================================
  Files          296      296           
  Lines         3815     3815           
  Branches       846      846           
========================================
  Hits          2961     2961           
  Misses         559      559           
  Partials       295      295           
Impacted Files Coverage Δ
...eact/src/components/Avatars/Avatar/Avatar.types.ts 100.00% <ø> (ø)
...omponents/Avatars/AvatarGroup/AvatarGroup.types.ts 100.00% <ø> (ø)
.../Avatars/CheckableAvatar/CheckableAvatar.styled.ts 0.00% <0.00%> (ø)
...onents/Avatars/CheckableAvatar/CheckableAvatar.tsx 0.00% <0.00%> (ø)
...bezier-react/src/components/Banner/Banner.types.ts 100.00% <ø> (ø)
...bezier-react/src/components/Button/Button.types.ts 100.00% <ø> (ø)
...s/bezier-react/src/components/Emoji/Emoji.types.ts 100.00% <ø> (ø)
...components/Forms/FormControl/FormControlContext.ts 100.00% <ø> (ø)
...components/Forms/Inputs/TextArea/TextArea.types.ts 100.00% <ø> (ø)
...eact/src/components/Forms/Inputs/TextArea/utils.ts 100.00% <ø> (ø)
... and 148 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@Dogdriip Dogdriip marked this pull request as ready for review March 14, 2023 08:27
Comment on lines +2 to +8
import type {
BezierComponentProps,
ChildrenProps,
DisableProps,
SizeProps,
AdditionalStylableProps,
} from '~/src/types/ComponentProps'
Copy link
Contributor

Choose a reason for hiding this comment

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

PR 컨텍스트와는 다른데 import 개행 스타일이 사람마다 달라서, 매번 불필요한 diff가 상당히 많이 생기네요.
eslint 룰이나 prettier를 빨리 적용해야할듯.

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 Prettier 도입 관련해서는 니즈가 계속 있었던 것 같은데, 혹시 관련 이슈가 있을까요?

Copy link
Contributor

@sungik-choi sungik-choi Mar 15, 2023

Choose a reason for hiding this comment

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

제가 오늘 내일중으로 이슈 생성해놓을게요~ 이번주나 다음주내로 빠르게 작업해보려고합니다.
이 PR 적용 이후에 작업하려고해요

@sungik-choi
Copy link
Contributor

👍

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.

ButtonGroup 확인했습니다

@sungik-choi
Copy link
Contributor

충돌 해결되면 제가 머지하겠습니다 @Dogdriip

@sungik-choi sungik-choi merged commit 46a392f into channel-io:next-v1 Mar 20, 2023
@Dogdriip Dogdriip deleted the refactor/tilde-specifier branch March 22, 2023 02:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Issue or PR related to refactoring with no functional changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants