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

fix(core-browser): move react to peerDependencies #2562

Merged
merged 2 commits into from
Apr 18, 2023
Merged

Conversation

gemwuu
Copy link
Contributor

@gemwuu gemwuu commented Apr 7, 2023

Types

  • 🎉 New Features
  • 🐛 Bug Fixes
  • 📚 Documentation Changes
  • 💄 Code Style Changes
  • 💄 Style Changes
  • 🪚 Refactors
  • 🚀 Performance Improvements
  • 🏗️ Build System
  • ⏱ Tests
  • 🧹 Chores
  • Other Changes

Background or solution

@opensumi/ide-core-browser depends on react@^16.8.6, which should be put in peerDependencies to avoid creating extra unnecessary react@16.x environment.

Changelog

fix(core-browser): move react to peerDependencies.

@CLAassistant
Copy link

CLAassistant commented Apr 7, 2023

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@opensumi opensumi bot added the 🐞 bug Something isn't working label Apr 7, 2023
@gemwuu gemwuu changed the title fix(core-browser): move react to peerDDependencies fix(core-browser): move react to peerDependencies Apr 7, 2023
@hacke2
Copy link
Member

hacke2 commented Apr 7, 2023

/publish

@github-actions
Copy link
Contributor

github-actions bot commented Apr 7, 2023

🎉 PR Next version 2.23.3-next-1680847428.0 publish successful! You can install prerelease version via npm install package@2.23.3-next-1680847428.0 @hacke2

2.23.3-next-1680847428.0

/home/runner/work/_temp/_runner_file_commands/step_summary_4cd034eb-fc9b-4e56-91ce-7b6b6d1da056

@bytemain
Copy link
Member

bytemain commented Apr 7, 2023

请再重新提交一下 lock 文件

@gemwuu
Copy link
Contributor Author

gemwuu commented Apr 7, 2023

请再重新提交一下 lock 文件

提交了,麻烦再触发下 CI。

@codecov
Copy link

codecov bot commented Apr 7, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (c1c06c5) 57.72% compared to head (6bd56b3) 57.72%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2562   +/-   ##
=======================================
  Coverage   57.72%   57.72%           
=======================================
  Files        1324     1324           
  Lines       83544    83544           
  Branches    17392    17392           
=======================================
+ Hits        48222    48228    +6     
+ Misses      32092    32087    -5     
+ Partials     3230     3229    -1     
Flag Coverage Δ
jsdom 52.74% <ø> (+<0.01%) ⬆️
node 16.76% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 2 files with indirect coverage changes

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.

@hacke2
Copy link
Member

hacke2 commented Apr 7, 2023

/publish

@github-actions
Copy link
Contributor

github-actions bot commented Apr 7, 2023

🎉 PR Next version 2.23.3-next-1680851880.0 publish successful! You can install prerelease version via npm install package@2.23.3-next-1680851880.0 @hacke2

2.23.3-next-1680851880.0

/home/runner/work/_temp/_runner_file_commands/step_summary_1f2c1647-d290-40d9-951d-173ac8d6e331

@erha19
Copy link
Member

erha19 commented Apr 17, 2023

@gemwuu @hacke2 这个有相关结论吗?移动 react 依赖到 DevDeps 的原因是啥?

@gemwuu
Copy link
Contributor Author

gemwuu commented Apr 17, 2023

@gemwuu @hacke2 这个有相关结论吗?移动 react 依赖到 DevDeps 的原因是啥?

是移到 peerDependencies 中。组件强依赖 react 框架,更加合理的做法是声明 peerDependencies,否则会污染依赖树上的 react 版本。

Copy link
Member

@hacke2 hacke2 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@erha19 erha19 left a comment

Choose a reason for hiding this comment

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

LGTM, @Ricbet 也看一下吧

@erha19 erha19 merged commit 9b66cc8 into opensumi:main Apr 18, 2023
11 of 12 checks passed
@erha19 erha19 added this to the 2.24 milestone Apr 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants