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 runtime overhead related with css variables #1819

Merged
merged 2 commits into from
Dec 18, 2023

Conversation

sungik-choi
Copy link
Contributor

@sungik-choi sungik-choi commented Dec 15, 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

Remove runtime overhead related with css variables

Details

  • 단순 문자열을 반환하는 함수인데 불필요하게 함수로 분리할 필요가 없다고 판단하여 제거합니다
    • 오버헤드를 없애자면 빌드 타임에 cv() -> 해당하는 문자열로 변경해줘도 되지만, 굳이 필요한가 싶어서 기존처럼 단순 string으로 변경했습니다
  • cssVarValue -> cssVar로 이름 단순화

Breaking change? (Yes/No)

No

References

없음

@sungik-choi sungik-choi added the refactoring Issue or PR related to refactoring with no functional changes label Dec 15, 2023
@sungik-choi sungik-choi self-assigned this Dec 15, 2023
Copy link

changeset-bot bot commented Dec 15, 2023

⚠️ No Changeset found

Latest commit: e823eda

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

Copy link
Contributor

github-actions bot commented Dec 15, 2023

Chromatic Report

🚀 Congratulations! Your build was successful!

@sungik-choi sungik-choi force-pushed the perf/css-var-overhead branch from fbd6669 to 299dd3f Compare December 15, 2023 12:43
Copy link

codecov bot commented Dec 15, 2023

Codecov Report

Attention: 8 lines in your changes are missing coverage. Please review.

Comparison is base (7d98d23) 86.64% compared to head (e823eda) 86.61%.

Files Patch % Lines
packages/bezier-react/src/utils/props.ts 0.00% 3 Missing ⚠️
...react/src/components/Modals/Modal/ModalContent.tsx 33.33% 0 Missing and 2 partials ⚠️
packages/bezier-react/src/utils/style.ts 50.00% 2 Missing ⚠️
...onents/Forms/SegmentedControl/SegmentedControl.tsx 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            alpha    #1819      +/-   ##
==========================================
- Coverage   86.64%   86.61%   -0.04%     
==========================================
  Files         278      278              
  Lines        3790     3774      -16     
  Branches      796      795       -1     
==========================================
- Hits         3284     3269      -15     
  Misses        433      433              
+ Partials       73       72       -1     

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

@sungik-choi sungik-choi force-pushed the perf/css-var-overhead branch from 299dd3f to e823eda Compare December 18, 2023 04:34
@sungik-choi sungik-choi marked this pull request as ready for review December 18, 2023 04:42
@sungik-choi sungik-choi merged commit b00dcf3 into channel-io:alpha Dec 18, 2023
4 of 6 checks passed
@sungik-choi sungik-choi deleted the perf/css-var-overhead branch December 18, 2023 08:01
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
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants