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

Require cycle #562

Closed
a-eid opened this issue Aug 22, 2024 · 1 comment · Fixed by #563
Closed

Require cycle #562

a-eid opened this issue Aug 22, 2024 · 1 comment · Fixed by #563
Assignees
Labels
🐛 bug Something isn't working 📚 components Anything related to the exported components of this library

Comments

@a-eid
Copy link

a-eid commented Aug 22, 2024

Describe the bug

I noticed this require cycle warning on the latest RNKC version ^1.13.2

λ  WARN  Require cycle: ../../node_modules/react-native-keyboard-controller/lib/commonjs/index.js -> ../../node_modules/react-native-keyboard-controller/lib/commonjs/components/index.js -> ../../node_modules/react-native-keyboard-controller/lib/commonjs/components/KeyboardToolbar/index.js -> ../../node_modules/react-native-keyboard-controller/lib/commonjs/index.js

Code snippet
NA

any imports would trigger this.

Repo for reproducing
NA

To Reproduce
NA

Expected behavior
No require cycle warnings.

Screenshots
Screenshot 2024-08-22 at 9 24 31 PM

@kirillzyusko
Copy link
Owner

Thank you for the report @a-eid

I will try to fix it tomorrow!

@kirillzyusko kirillzyusko added 🐛 bug Something isn't working 📚 components Anything related to the exported components of this library labels Aug 22, 2024
kirillzyusko added a commit that referenced this issue Aug 23, 2024
## 📜 Description

Fixed cycle dependencies in the code.

## 💡 Motivation and Context

Cycle dependencies may lead to unpredictable results especially if you
have many of them.

To fix them I used `eslint` rule - it's quite effective and we don't
need to change CI pipelines etc. Plus we can monitor such cycles
contioniously!

Closes
#562

## 📢 Changelog

<!-- High level overview of important changes -->
<!-- For example: fixed status bar manipulation; added new types
declarations; -->
<!-- If your changes don't affect one of platform/language below - then
remove this platform/language -->

### JS

- reorganized imports to avoid cycle deps;
- added new eslint rule usage.

## 🤔 How Has This Been Tested?

Tested on e2e tests on CI 🙂 

## 📝 Checklist

- [x] CI successfully passed
- [x] I added new mocks and corresponding unit-tests if library API was
changed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working 📚 components Anything related to the exported components of this library
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants