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

Writing code to convert Typography of TextProps.typo to typo string literal #1816

Closed
Tracked by #1800
yangwooseong opened this issue Dec 15, 2023 · 3 comments · Fixed by #1849
Closed
Tracked by #1800

Writing code to convert Typography of TextProps.typo to typo string literal #1816

yangwooseong opened this issue Dec 15, 2023 · 3 comments · Fixed by #1849
Assignees
Labels
bezier-codemod Issue or PR related to bezier-codemod

Comments

@yangwooseong
Copy link
Collaborator

yangwooseong commented Dec 15, 2023

Summary

  • Texttypo 속성에 Typography enum 을 주고 있는 것을 string literal 로 변환하고, marin 관련 속성을 shorthand로 변환하기 위한 codemod 를 작성합니다.

Description

// As-is
import { Text, Typography } from '@channel.io/bezier-react'

<Text 
  typo={Typography.Size14}
  marginHorizontal={6}
>
  some text
</Text>

// To-be
import { Text } from '@channel.io/bezier-react'

<Text 
  typo="14"
  mx={6}
>
  some text
</Text> 

styled component 안에 있는 코드도 변환해야 합니다.

// As-is
import { styled, Text, Typography } from '@chanenl.io/bezier-react'

export const Title = styled(Text).attrs({
  marginLeft: 3,
  typo: Typography.Size11,
})``

// To-be
import { styled, Text } from '@chanenl.io/bezier-react'

export const Title = styled(Text).attrs({
  ml: 3,
  typo: '11',
})``

enum 속성을 string literal 로 변경해주는 codemod는 #1639 에서 구현되어 있기 때문에, 해당 로직을 그대로 사용하면 쉽게 바꿀 수 있을 것으로 예상합니다.

@yangwooseong yangwooseong added the bezier-codemod Issue or PR related to bezier-codemod label Dec 15, 2023
@github-project-automation github-project-automation bot moved this to 📌 Backlog in Bezier React Dec 15, 2023
@yangwooseong yangwooseong mentioned this issue Dec 15, 2023
@sungik-choi
Copy link
Contributor

#1792 와 관련하여, Text 의 margin 관련 속성이 shorthand 속성으로 변경된다면 변환 코드가 추가로 필요할 수 있습니다.

@yangwooseong
Copy link
Collaborator Author

#1792 와 관련하여, Text 의 margin 관련 속성이 shorthand 속성으로 변경된다면 변환 코드가 추가로 필요할 수 있습니다.

네, Text 컴포넌트 만들어지는 것을 보고 구현을 시작하려고 합니다

@sungik-choi
Copy link
Contributor

#1820

@yangwooseong yangwooseong self-assigned this Dec 18, 2023
yangwooseong added a commit that referenced this issue Dec 22, 2023
<!--
  How to write a good PR title:
- Follow [the Conventional Commits
specification](https://www.conventionalcommits.org/en/v1.0.0/).
  - Give as much context as necessary and as little as possible
  - Prefix it with [WIP] while it’s a work in progress
-->

## Self Checklist

- [x] I wrote a PR title in **English** and added an appropriate
**label** to the PR.
- [x] I wrote the commit message in **English** and to follow [**the
Conventional Commits
specification**](https://www.conventionalcommits.org/en/v1.0.0/).
- [x] I [added the
**changeset**](https://github.com/changesets/changesets/blob/main/docs/adding-a-changeset.md)
about the changes that needed to be released. (or didn't have to)
- [x] I wrote or updated **documentation** related to the changes. (or
didn't have to)
- [x] I wrote or updated **tests** related to the changes. (or didn't
have to)
- [x] I tested the changes in various browsers. (or didn't have to)
  - Windows: Chrome, Edge, (Optional) Firefox
  - macOS: Chrome, Edge, Safari, (Optional) Firefox

## Related Issue
<!-- Please link to issue if one exists -->

<!-- Fixes #0000 -->

- Fixes #1816

## Summary
<!-- Please brief explanation of the changes made -->

- `Text` 컴포넌트의 인터페이스를 변경하기 위한 codemod 를 추가합니다. 아래와 같이 변환됩니다.
- #1844 base, from
a1f267f

```tsx
// As-is
import { Text, Typography } from '@channel.io/bezier-react'

<Text 
  typo={Typography.Size14}
  marginHorizontal={6}
>
  some text
</Text>

// To-be
import { Text } from '@channel.io/bezier-react'

<Text 
  typo="14"
  mx={6}
>
  some text
</Text>

// As-is
import { Text, styled, Typography } from '@channel.io/bezier-react'

const Title = styled(Text).attrs({
  typo: Typography.Size11,
  marginHorizontal: 6,
})``

// To-be
import { Text, styled } from '@channel.io/bezier-react'

const Title = styled(Text).attrs({
  typo: '11',
  mxl: 6,
})``
```

## Details
<!-- Please elaborate description of the changes -->

- 컴포넌트의 prop 을 변경하는 유틸과 styled component 안에서 .attrs 뒤에 나오는 객체를 변경하는 유틸을
추가했습니다.

### Breaking change? (Yes/No)
<!-- If Yes, please describe the impact and migration path for users -->

- No

## References
<!-- Please list any other resources or points the reviewer should be
aware of -->

- #1816
@github-project-automation github-project-automation bot moved this from 📌 Backlog to ✅ Done in Bezier React Dec 22, 2023
yangwooseong added a commit that referenced this issue Jan 19, 2024
<!--
  How to write a good PR title:
- Follow [the Conventional Commits
specification](https://www.conventionalcommits.org/en/v1.0.0/).
  - Give as much context as necessary and as little as possible
  - Prefix it with [WIP] while it’s a work in progress
-->

## Self Checklist

- [x] I wrote a PR title in **English** and added an appropriate
**label** to the PR.
- [x] I wrote the commit message in **English** and to follow [**the
Conventional Commits
specification**](https://www.conventionalcommits.org/en/v1.0.0/).
- [x] I [added the
**changeset**](https://github.com/changesets/changesets/blob/main/docs/adding-a-changeset.md)
about the changes that needed to be released. (or didn't have to)
- [x] I wrote or updated **documentation** related to the changes. (or
didn't have to)
- [x] I wrote or updated **tests** related to the changes. (or didn't
have to)
- [x] I tested the changes in various browsers. (or didn't have to)
  - Windows: Chrome, Edge, (Optional) Firefox
  - macOS: Chrome, Edge, Safari, (Optional) Firefox

## Related Issue
<!-- Please link to issue if one exists -->

<!-- Fixes #0000 -->

- #1816

## Summary
<!-- Please brief explanation of the changes made -->

- `v2-text-component-interface` codemod 가 속성 값의 이름을 변경할 때(e.g.
typo={Typography.Size14} -> typo='14'), 컴포넌트 이름을 확인하지 않고 속성의 이름만 확인하도록
변경합니다.
- `sourceFile.fixUnusedImports()` 를 제거합니다.

## Details
<!-- Please elaborate description of the changes -->

- 기존의 변환 로직에서는 컴포넌트 이름이 정확히 `Text`이어야 하기 때문에 아래와 같은 경우를 변환할 수 없었습니다.
Text 컴포넌트는 사용처가 워낙 많아서 이름을 바꿔서 사용하는 곳도 많았습니다. 데스크 코드 기준으로 이런 경우가 100 여개
정도는 되었기 때문에 변환에 포함시키도록 합니다. `Text` 말고는 typo={Typography} 를 props 로 가지는
컴포넌트가 없기 때문에 이렇게 해도 문제가 없을 것으로 생각됩니다.

```tsx
import * as Styled from './some.styled.ts'
import { Typography } from '@channel.io/bezier-react'

<Styled.Title typo={Typography.Size14}>
  channel
</Styled.Title>
```

- `Typography` enum 을 제거하면서 사용처가 없어질 때, import 구문에서 제거하기 위해 ts-morph에서
제공하는 `sourceFile.fixUnusedImports()`를 사용했습니다. 하지만
`sourceFile.fixUnusedImports()`는 소스파일 전체를 순회해야하기 때문에 비용이 매우 크고, import
'styles.css' 와 같은 경우까지 제거해버려서 사이드 이펙트가 있습니다. 유틸함수로 만든
removeUnusedNamedImport 로 대체하였고, 시간이 2분 -> 20~30초 정도로 단축되는 것을 확인했습니다.
`sourceFile.fixUnusedImports()` 사용은 이제 지양하는 게 좋아보입니다.

### Breaking change? (Yes/No)
<!-- If Yes, please describe the impact and migration path for users -->

- No

## References
<!-- Please list any other resources or points the reviewer should be
aware of -->

- None
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bezier-codemod Issue or PR related to bezier-codemod
Projects
No open projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants