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.

Sign up for GitHub

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

Jump to bottom

build(rollup): add browser exports condition to module resolve #1637

Merged
merged 2 commits into from
Sep 18, 2023
Merged

build(rollup): add browser exports condition to module resolve #1637

merged 2 commits into from
Sep 18, 2023

Conversation

sungik-choi
Copy link
Contributor

@sungik-choi sungik-choi commented Sep 18, 2023 •
edited
Loading

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

  • rollup plugin-node-resolve 의 옵션에 browser: true 를 추가하여, package.json의 exports 필드 중 browser 를 함께 확인하도록 수정합니다.
  • 이 변경으로 TextArea 컴포넌트의 높이가 올바르게 지정되지 않는 문제를 수정합니다.

Details

react-textarea-autosize@8.5.0 버전에서 Andarist/react-textarea-autosize#373 변경사항이 적용되었습니다. 라이브러리의 빌드 최적화 전략이 변경되며 package.json의 exports 필드가 수정되었는데, 기존 브라우저 API를 사용하는 빌드 아티팩트는 browsers 필드 하위에 포함되도록 변경되었습니다. 마찬가지로 여기서 브레이킹 체인지가 발생했는데, 기존 main, module 필드가 가르키는 아티팩트들에서는 브라우저 API를 사용하지 않는 버전으로 변경되었습니다.

rollup의 plugin-node-resolve는 기본값으로 exports 필드를 확인할 때, default, module, import 3가지의 조건 필드만을 확인합니다. 여기에 browser 필드가 없으므로, 번들 시 브라우저 API를 사용하지 않는 버전의 react-textarea-autosize가 포함되게 됩니다.

플러그인의 browser 속성을 true 로 변경하여 모듈 해석 시 browser 필드(exports.brower, browser)도 참고하도록 변경합니다.

If true, instructs the plugin to use the browser module resolutions in package.json and adds 'browser' to exportConditions if it is not present so browser conditionals in exports are applied.

Breaking change? (Yes/No)

No

References

@sungik-choi sungik-choi added fix PR related to bug fix build Issue or PR related to build system labels Sep 18, 2023
@sungik-choi sungik-choi self-assigned this Sep 18, 2023
@changeset-bot
Copy link

changeset-bot bot commented Sep 18, 2023

🦋 Changeset detected

Latest commit: 2ff689e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@channel.io/bezier-react Patch
bezier-figma-plugin Patch

Not sure what this means? Click here to learn what changesets are.

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

@codecov
Copy link

codecov bot commented Sep 18, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (ab3062e) 86.96% compared to head (2ff689e) 86.96%.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #1637   +/-   ##
=======================================
  Coverage   86.96%   86.96%           
=======================================
  Files         281      281           
  Lines        3882     3882           
  Branches      820      820           
=======================================
  Hits         3376     3376           
  Misses        432      432           
  Partials       74       74

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@github-actions
Copy link
Contributor

Chromatic Report

🚀 Congratulations! Your build was successful!

@sungik-choi sungik-choi merged commit 3cb4c5b into channel-io:main Sep 18, 2023
6 checks passed
@sungik-choi sungik-choi deleted the fix/textarea-autosize-sizing branch September 18, 2023 05:33
@sungik-choi
Copy link
Contributor Author

sungik-choi commented Sep 18, 2023 •
edited
Loading

image

CJS, ESM 모두 올바르게 빌드되는 모습을 확인했습니다.

@github-actions github-actions bot mentioned this pull request Sep 18, 2023
Merged
sungik-choi pushed a commit that referenced this pull request Sep 18, 2023
@github-actions
ci(changesets): version packages (#1622)
d505cdc 
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## @channel.io/bezier-react@1.13.0

### Minor Changes

- Add `preventHideOnOutsideClick` property to `Modal` component
([#1617](#1617)) by
@yangwooseong

### Patch Changes

- Fixes an issue where the height of `TextArea` component is not
specified correctly. Modify the build settings to match the package.json
exports fields change in 8.5.0 of `react-textarea-autosize`.
([#1637](#1637)) by
@sungik-choi

- Add `ProgressBarSize`, `ProgressBarVariant` string literal type and
deprecate enum
([#1595](#1595)) by
@Dogdriip

## bezier-figma-plugin@0.4.1

### Patch Changes

-   Updated dependencies
    -   @channel.io/bezier-react@1.13.0

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@sungik-choi sungik-choi mentioned this pull request Oct 19, 2023
Merged
6 tasks
sungik-choi added a commit to sungik-choi/bezier-react that referenced this pull request Oct 19, 2023
@sungik-choi
build(bezier-react): revert channel-io#1637
5d77004
sungik-choi added a commit that referenced this pull request Oct 20, 2023
@sungik-choi
Downgrade react-textarea-autosize version to 8.3.4 to fix SSR documen…
82a1314 
…t error (#1688)

<!--
  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 #1687 

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

- react-textarea-autosize의 버전을 `8.3.4` 로 다운그레이드하고, 버전을 고정합니다.
- revert #1637

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

#1637 에서 react-textarea-autosize의 변경으로 인해 rollup의 node module resolve
방식을 변경했습니다. SSR 환경에서도 browser용 번들을 바라보게되면서, `document` 관련 에러가 발생하여 빌드가
실패하는 문제가 발생했습니다.

이를 해결하고자 여러 방법을 고려해봤는데,

1. main module은 SSR용 모듈로 두고, browser용 exports field를 추가: 빌드 시 browser
exports field를 고려하지 않고 있는 다른 사용처에서의 사이드 이펙트가 우려되어서 선택하지 않았습니다.
2. external dependency로 명시하여 peer dependency로 변경: Breaking change를 발생시키고
싶지 않았습니다. TextArea 컴포넌트를 사용하지 않는 사용처에서도 라이브러리 사용이 강제됩니다.
peerDependenciesMeta.optional를 사용하는 방법으로 우회하더라도 매력적인 선택지는 아니라고 생각했습니다.
3. 런타임에 document 체크를 하는 로직 추가: 로직을 추가하고 테스트 해보았으나 여전히 에러가 발생했습니다. 자세히 빌드
과정(Next.js)을 뜯어보진 않았지만, 런타임을 고려하기 이전에 모든 모듈을 정적 분석 + 에러를 던지는 것으로 보입니다.
4. **다운그레이드 및 리버트**

기존에도 큰 문제없이 잘 동작하던 모듈이고, react-textarea-autosize의 변경사항을 봐도 서버 사이드에서 파일
사이즈의 감소 외에는 로직 변경이 없었습니다. 사용처에 별다른 영향이 없을 거로 판단하고 문제 해결을 위해 다운그레이드하는
방향으로 결정했습니다.

### 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 -->

- [관련 채널톡
스레드](https://desk.channel.io/root/groups/WebBezier-124831/6530b7edbda0b8528c47)
- Andarist/react-textarea-autosize#335
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yangwooseong yangwooseong Awaiting requested review from yangwooseong yangwooseong is a code owner

Assignees

@sungik-choi sungik-choi

Labels
build Issue or PR related to build system fix PR related to bug fix
Projects
No open projects
Bezier React
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@sungik-choi