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

Fix circular dependencies #5887

Open
drewvolz opened this issue Apr 14, 2022 · 1 comment
Open

Fix circular dependencies #5887

drewvolz opened this issue Apr 14, 2022 · 1 comment

Comments

@drewvolz
Copy link
Member

drewvolz commented Apr 14, 2022 •
edited
Loading

The project has:

amount about assessment
8 erroring require cycles redux we must do something about these
35 warning cycles image exports we can do something about these
95 skipped cycles dependencies from node_modules not necessarily up to us to change

We can find these with the dpdm package.

> npx dpdm index.js --tree false --exclude 'node_modules' --warning true

  1) source/redux/index.ts -> source/redux/parts/settings.ts
  2) source/redux/index.ts -> source/redux/parts/buildings.ts
  3) source/redux/index.ts -> source/redux/parts/help.ts
  4) source/redux/index.ts -> source/redux/parts/courses.ts
  5) source/redux/index.ts -> source/redux/parts/stoprint.ts
  6) source/redux/index.ts -> source/redux/parts/login.ts
  7) source/navigation/types.tsx -> modules/event-list/index.ts -> modules/event-list/event-detail.tsx -> modules/event-list/event-detail-ios.tsx
  8) source/navigation/types.tsx -> modules/event-list/index.ts -> modules/event-list/event-detail.tsx -> modules/event-list/event-detail-android.tsx
📝 Full output 
✔ [226/226] Analyze done!
• Circular Dependencies
  1) source/redux/index.ts -> source/redux/parts/settings.ts
  2) source/redux/index.ts -> source/redux/parts/buildings.ts
  3) source/redux/index.ts -> source/redux/parts/help.ts
  4) source/redux/index.ts -> source/redux/parts/courses.ts
  5) source/redux/index.ts -> source/redux/parts/stoprint.ts
  6) source/redux/index.ts -> source/redux/parts/login.ts
  7) source/navigation/types.tsx -> modules/event-list/index.ts -> modules/event-list/event-detail.tsx -> modules/event-list/event-detail-ios.tsx
  8) source/navigation/types.tsx -> modules/event-list/index.ts -> modules/event-list/event-detail.tsx -> modules/event-list/event-detail-android.tsx
  
• Warnings
  001) miss "./link-table" in "source/views/building-hours/detail/building.tsx"
  002) miss "./old-main.png" in "images/icons/index.ts"
  003) miss "./optimized/alumni-hall.jpg" in "images/spaces/index.ts"
  004) miss "./optimized/boe-house.jpg" in "images/contacts/index.ts"
  005) miss "./optimized/boe.jpg" in "images/spaces/index.ts"
  006) miss "./optimized/bookstore.jpg" in "images/spaces/index.ts"
  007) miss "./optimized/buntrock.jpg" in "images/spaces/index.ts"
  008) miss "./optimized/cage.jpg" in "images/spaces/index.ts"
  009) miss "./optimized/center-for-arts-and-dance.jpg" in "images/spaces/index.ts"
  010) miss "./optimized/christiansen.jpg" in "images/spaces/index.ts"
  011) miss "./optimized/convenience.jpg" in "images/spaces/index.ts"
  012) miss "./optimized/disco.jpg" in "images/spaces/index.ts"
  013) miss "./optimized/hall-of-music.jpg" in "images/spaces/index.ts"
  014) miss "./optimized/halvorson.jpg" in "images/spaces/index.ts"
  015) miss "./optimized/old-main.jpg" in "images/spaces/index.ts"
  016) miss "./optimized/pause-kitchen.jpg" in "images/contacts/index.ts"
  017) miss "./optimized/pause-kitchen.jpg" in "images/spaces/index.ts"
  018) miss "./optimized/post-office.jpg" in "images/spaces/index.ts"
  019) miss "./optimized/print.jpg" in "images/spaces/index.ts"
  020) miss "./optimized/pubsafe.jpg" in "images/contacts/index.ts"
  021) miss "./optimized/regents-hall.jpg" in "images/spaces/index.ts"
  022) miss "./optimized/regents-math.jpg" in "images/spaces/index.ts"
  023) miss "./optimized/rolvaag-library.jpg" in "images/spaces/index.ts"
  024) miss "./optimized/safe-ride.jpg" in "images/contacts/index.ts"
  025) miss "./optimized/sarn.png" in "images/contacts/index.ts"
  026) miss "./optimized/skifter-studioa.jpg" in "images/spaces/index.ts"
  027) miss "./optimized/skoglund.jpg" in "images/spaces/index.ts"
  028) miss "./optimized/stav.jpg" in "images/spaces/index.ts"
  029) miss "./optimized/theater.jpg" in "images/spaces/index.ts"
  030) miss "./optimized/tom-porter.jpg" in "images/spaces/index.ts"
  031) miss "./optimized/tomson.jpg" in "images/spaces/index.ts"
  032) miss "./optimized/wellness.jpg" in "images/spaces/index.ts"
  033) miss "./schedule-table" in "source/views/building-hours/detail/building.tsx"
  034) miss "./student-work/detail" in "source/views/sis/index.ts"
  035) miss "./windmill.png" in "images/icons/index.ts"
  036) skip "node_modules/@callstack/react-theme-provider/lib/index.js", issuers: "modules/app-theme/index.ts", "modules/app-theme/index.ts"
  037) skip "node_modules/@expo/react-native-action-sheet/lib/module/index.js", issuers: "modules/markdown/link.tsx", "source/app.tsx"
  038) skip "node_modules/@frogpond/titlecase/to-title-case.js", issuers: "source/views/menus/lib/trim-names.ts", "source/views/menus/menu-bonapp.tsx" (3 more...)
  039) skip "node_modules/@hawkrives/react-native-alternate-icons/index.js", issuers: "source/lib/refresh.ts", "source/views/settings/components/logo.tsx" (2 more...)
  040) skip "node_modules/@react-native-community/datetimepicker/src/index.js", issuers: "modules/datepicker/basepicker.tsx", "modules/datepicker/basepicker.tsx" (1 more...)
  041) skip "node_modules/@react-native-community/netinfo/lib/module/index.js", issuers: "source/redux/init.ts"
  042) skip "node_modules/@sentry/react-native/dist/js/index.js", issuers: "modules/add-to-device-calendar/lib.ts", "source/init/sentry.ts" (2 more...)
  043) skip "node_modules/base-64/base64.js", issuers: "source/lib/stoprint/api.ts"
  044) skip "node_modules/core-js/es6/array.js", issuers: "index.js"
  045) skip "node_modules/core-js/es6/symbol.js", issuers: "index.js"
  046) skip "node_modules/css-select/lib/index.js", issuers: "modules/html-lib/index.ts"
  047) skip "node_modules/delay/index.js", issuers: "modules/add-to-device-calendar/add-to-calendar.ts", "modules/fetch/index.ts" (4 more...)
  048) skip "node_modules/domhandler/lib/index.js", issuers: "modules/html-lib/index.ts"
  049) skip "node_modules/domutils/lib/index.js", issuers: "modules/html-lib/index.ts"
  050) skip "node_modules/glamorous-native/src/index.js", issuers: "modules/markdown/code.tsx", "modules/markdown/formatting.ts" (18 more...)
  051) skip "node_modules/html-entities/lib/index.js", issuers: "modules/html-lib/index.ts"
  052) skip "node_modules/htmlparser2/lib/index.js", issuers: "modules/html-lib/index.ts"
  053) skip "node_modules/http-cache-semantics/index.js", issuers: "modules/fetch/cached.ts"
  054) skip "node_modules/js-yaml/dist/js-yaml.mjs", issuers: "source/views/building-hours/report/submit.ts", "source/views/dictionary/report/submit.ts"
  055) skip "node_modules/keyword-search/index.js", issuers: "source/views/sis/course-search/lib/execute-search.ts"
  056) skip "node_modules/lodash/cloneDeep.js", issuers: "modules/filter/filter-toolbar.tsx"
  057) skip "node_modules/lodash/concat.js", issuers: "modules/filter/section-list.tsx"
  058) skip "node_modules/lodash/deburr.js", issuers: "source/views/dictionary/list.tsx", "source/views/student-orgs/list.tsx"
  059) skip "node_modules/lodash/difference.js", issuers: "modules/filter/apply-filters.ts"
  060) skip "node_modules/lodash/filter.js", issuers: "modules/food-menu/lib/build-filters.ts", "source/views/menus/menu-github.tsx"
  061) skip "node_modules/lodash/find.js", issuers: "source/views/transportation/bus/lib/get-current-bus-iteration.ts", "source/views/transportation/bus/line.tsx"
  062) skip "node_modules/lodash/findIndex.js", issuers: "modules/food-menu/lib/find-menu.ts"
  063) skip "node_modules/lodash/findLast.js", issuers: "source/views/transportation/bus/lib/get-current-bus-iteration.ts", "source/views/transportation/bus/line.tsx"
  064) skip "node_modules/lodash/findLastIndex.js", issuers: "source/views/transportation/bus/lib/get-current-bus-iteration.ts"
  065) skip "node_modules/lodash/flatten.js", issuers: "modules/filter/filter-toolbar.tsx", "modules/food-menu/lib/build-filters.ts" (3 more...)
  066) skip "node_modules/lodash/fromPairs.js", issuers: "modules/fetch/cached.ts", "source/views/menus/menu-github.tsx" (1 more...)
  067) skip "node_modules/lodash/get.js", issuers: "source/views/settings/screens/debug/list.tsx"
  068) skip "node_modules/lodash/groupBy.js", issuers: "modules/event-list/event-list.tsx", "source/views/building-hours/stateful-list.tsx" (11 more...)
  069) skip "node_modules/lodash/intersection.js", issuers: "modules/filter/apply-filters.ts"
  070) skip "node_modules/lodash/isEqual.js", issuers: "modules/filter/section-list.tsx", "source/views/building-hours/row.tsx"
  071) skip "node_modules/lodash/keys.js", issuers: "modules/food-menu/dietary-tags-detail.tsx", "modules/food-menu/dietary-tags.tsx"
  072) skip "node_modules/lodash/lodash.js", issuers: "modules/fetch/cached.ts"
  073) skip "node_modules/lodash/map.js", issuers: "modules/food-menu/food-item-detail.tsx", "modules/food-menu/lib/build-filters.ts" (1 more...)
  074) skip "node_modules/lodash/mapValues.js", issuers: "source/lib/course-search/load-filter-options.ts", "source/views/menus/menu-bonapp.tsx" (1 more...)
  075) skip "node_modules/lodash/memoize.js", issuers: "source/views/sis/course-search/list.tsx"
  076) skip "node_modules/lodash/noop.js", issuers: "modules/button/index.tsx", "source/views/settings/screens/overview/login-credentials.tsx" (1 more...)
  077) skip "node_modules/lodash/orderBy.js", issuers: "source/views/sis/student-work/index.tsx"
  078) skip "node_modules/lodash/reduce.js", issuers: "source/views/menus/menu-bonapp.tsx"
  079) skip "node_modules/lodash/reject.js", issuers: "modules/filter/section-list.tsx"
  080) skip "node_modules/lodash/sample.js", issuers: "source/views/home/notice.tsx", "source/views/menus/menu-bonapp.tsx" (1 more...)
  081) skip "node_modules/lodash/size.js", issuers: "modules/food-menu/fancy-menu.tsx"
  082) skip "node_modules/lodash/sortBy.js", issuers: "source/views/building-hours/lib/summarize-days.ts", "source/views/sis/course-search/lib/execute-search.ts" (2 more...)
  083) skip "node_modules/lodash/toPairs.js", issuers: "modules/event-list/event-list.tsx", "source/views/building-hours/stateful-list.tsx" (12 more...)
  084) skip "node_modules/lodash/uniq.js", issuers: "modules/food-menu/lib/build-filters.ts"
  085) skip "node_modules/lodash/values.js", issuers: "modules/filter/apply-filters.ts", "modules/food-menu/fancy-menu.tsx"
  086) skip "node_modules/lodash/words.js", issuers: "source/views/dictionary/list.tsx", "source/views/student-orgs/list.tsx"
  087) skip "node_modules/lodash/xor.js", issuers: "source/views/building-hours/report/editor.tsx"
  088) skip "node_modules/lodash/zip.js", issuers: "source/views/sis/course-search/detail/index.tsx"
  089) skip "node_modules/moment-timezone/index.js", issuers: "modules/ccc-calendar/index.tsx", "modules/ccc-calendar/index.tsx" (41 more...)
  090) skip "node_modules/moment/moment.js", issuers: "modules/event-type/index.ts", "modules/food-menu/fancy-menu.tsx" (13 more...)
  091) skip "node_modules/p-props/index.js", issuers: "source/lib/course-search/load-filter-options.ts"
  092) skip "node_modules/prop-types/index.js", issuers: "modules/markdown/markdown.tsx"
  093) skip "node_modules/query-string/index.js", issuers: "modules/api/index.ts", "modules/fetch/index.ts" (3 more...)
  094) skip "node_modules/react-async/dist-web/index.js", issuers: "source/views/dictionary/list.tsx", "source/views/dictionary/list.tsx" (2 more...)
  095) skip "node_modules/react-markdown/src/react-markdown.js", issuers: "modules/markdown/markdown.tsx"
  096) skip "node_modules/react-native-button/Button.js", issuers: "modules/button/index.tsx"
  097) skip "node_modules/react-native-calendar-events/index.js", issuers: "modules/add-to-device-calendar/lib.ts"
  098) skip "node_modules/react-native-communications/AKCommunications.js", issuers: "source/components/call-phone.ts", "source/components/send-email.ts"
  099) skip "node_modules/react-native-custom-tabs/index.js", issuers: "modules/open-url/open-url.ts"
  100) skip "node_modules/react-native-device-info/lib/module/index.js", issuers: "source/views/home/button.tsx", "source/views/settings/screens/overview/support.tsx"
  101) skip "node_modules/react-native-gesture-handler/lib/module/index.js", issuers: "source/app.tsx"
  102) skip "node_modules/react-native-keychain/index.js", issuers: "source/lib/login.ts", "source/lib/login.ts"
  103) skip "node_modules/react-native-linear-gradient/index.js", issuers: "source/views/home/button.tsx", "source/views/streaming/webcams/thumbnail.tsx"
  104) skip "node_modules/react-native-paper/src/index.js", issuers: "modules/searchbar/searchbar-android.tsx", "source/app.tsx" (1 more...)
  105) skip "node_modules/react-native-popover-view/dist/index.js", issuers: "modules/filter/filter-popover.tsx", "modules/filter/filter-popover.tsx"
  106) skip "node_modules/react-native-restart/lib/module/index.js", issuers: "source/lib/refresh.ts", "source/views/settings/screens/overview/server-url.tsx"
  107) skip "node_modules/react-native-safari-view/index.js", issuers: "modules/open-url/open-url.ts"
  108) skip "node_modules/react-native-screens/lib/module/index.js", issuers: "source/init/navigation.ts"
  109) skip "node_modules/react-native-search-bar/src/index.js", issuers: "modules/searchbar/searchbar-ios.tsx"
  110) skip "node_modules/react-native-tableview-simple/lib/module/index.js", issuers: "modules/tableview/cells/button.tsx", "modules/tableview/cells/delete-button.tsx" (7 more...)
  111) skip "node_modules/react-native-tableview-simple/src/components/Section.tsx", issuers: "modules/tableview/index.tsx"
  112) skip "node_modules/react-native-typography/src/index.js", issuers: "modules/button/index.tsx", "modules/markdown/formatting.ts" (5 more...)
  113) skip "node_modules/react-native-vector-icons/Entypo.js", issuers: "source/views/home/button.tsx"
  114) skip "node_modules/react-native-vector-icons/Ionicons.js", issuers: "modules/filter/active-filter-button.tsx", "modules/filter/filter-toolbar-button.tsx" (7 more...)
  115) skip "node_modules/react-native-vector-icons/glyphmaps/Ionicons.json", issuers: "modules/icon/source.ts"
  116) skip "node_modules/react-native-webview/index.js", issuers: "source/views/streaming/radio/player.tsx"
  117) skip "node_modules/react-native/Libraries/react-native/react-native-implementation.js", issuers: "modules/add-to-device-calendar/lib.ts", "modules/badge/outline.tsx" (142 more...)
  118) skip "node_modules/react-navigation-material-bottom-tabs/dist/index.js", issuers: "modules/navigation-tabs/tabbed-view.ts"
  119) skip "node_modules/react-navigation-stack/lib/module/index.js", issuers: "source/navigation/navigator.ts"
  120) skip "node_modules/react-navigation-tabs/lib/module/index.js", issuers: "modules/navigation-tabs/tabbed-view.ts"
  121) skip "node_modules/react-navigation/src/index.js", issuers: "modules/ccc-calendar/index.tsx", "modules/event-list/types.ts" (16 more...)
  122) skip "node_modules/react-redux/es/index.js", issuers: "source/app.tsx", "source/views/building-hours/detail/toolbar-button.tsx" (10 more...)
  123) skip "node_modules/react/index.js", issuers: "modules/add-to-device-calendar/add-to-calendar.ts", "modules/badge/outline.tsx" (157 more...)
  124) skip "node_modules/redux-logger/dist/redux-logger.js", issuers: "source/redux/index.ts"
  125) skip "node_modules/redux-promise/lib/index.js", issuers: "source/redux/index.ts"
  126) skip "node_modules/redux-thunk/es/index.js", issuers: "source/redux/index.ts"
  127) skip "node_modules/redux/es/redux.js", issuers: "source/redux/index.ts", "source/redux/index.ts" (1 more...)
  128) skip "node_modules/semver/index.js", issuers: "source/views/help/index.tsx"
  129) skip "node_modules/tinycolor2/tinycolor.js", issuers: "modules/badge/outline.tsx", "modules/colors/util.ts" (3 more...)
  130) skip "node_modules/wordwrap/index.js", issuers: "source/views/dictionary/report/submit.ts"

Why we need to remove these

Metro bundler does not handle require cycles well. While webpack could fix/ignore these cycles as a crutch, Metro chooses to have you deal with them, which I like.

Require cycles can affect app performance, slowing down some components and add unnecessary depth to items (even with webpack). These cycles should be resolved and in some cases can cause larger issues of things being undefined. If we're lucky, the ones that are impacting the repo are showing up as red and yellow screens without broken components.

By resolving these we can improve the stability of the app while making it more performant.

How do I identify them?

Metro is pretty good about letting us know if these are here, although I have witnessed a case where they have been so deep in the require stack that it took commenting code out to get to the level where they'd show up.

Possible symptoms of these require cycles:

  • breakage in features directly related to the require cycle
  • longer load times of a component

How do I fix them?

A pattern that we make use of in this codebase is exporting something from file A and then importing/exporting it via a central index file. That index is then consumed elsewhere throughout the app and unfortunately causes cycles.

A lot of this will probably boil down to:

  • changing imports to not be that central index import (and updating the imports to directly import from the exported location)
  • looking for anything that imports from '.'
  • looking for where FileA imports FileB and FileB imports FileA and moving one of these exports into a separate file

Invocation

  1. npm i -g dpdm
  2. dpdm index.js
  • --warning false to disable warnings (not recommended since it may be a file you need to look into)
  • --tree false to ignore the large tree representation that is printed out

Both flags are enabled by default and are useful for debugging if files are included or if there is an issue with how things are being imported. If you see an ignored file in the warning section, you can modify the include or exclude paths.

Going further

This may not be enough to find and fix all of our cycles. It may be necessary to take our own packages we maintain out of node_modules and analyze them independently from within their own entry point (and not AAO-React-Native's entry point).

cp -r node_modules/@frogpond/some-dep ./some-dep
cd some-dep
dpdm dist/index.js
@drewvolz
Copy link
Member Author

https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/no-cycle.md

We should add to our rules:

  • import/no-cycle: error
  • import/no-self-import: error

@drewvolz drewvolz mentioned this issue Aug 8, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@drewvolz