search

invertase / react-native-firebase

🔥 A well-tested feature-rich modular Firebase implementation for React Native. Supports both iOS & Android platforms for all Firebase services.
https://rnfirebase.io
Other
11.68k stars 2.21k forks source link

Require Cycle: DocumentReference <> DocumentSnapshot #1446

Closed TrekSoft closed 5 years ago

TrekSoft commented 6 years ago

Issue

The newest version of react native (0.57.0-rc.3) has started giving yellow bar warnings about require cycles.

I'm using v5.6.0 of react-native-firebase and am getting a warning about this require cycle:

Require cycle: node_modules\react-native-firebase\dist\modules\firestore\DocumentReference.js -> node_modules\react-native-firebase\dist\modules\firestore\DocumentSnapshot.js -> node_modules\react-native-firebase\dist\modules\firestore\DocumentReference.js

Environment

  1. Application Target Platform:

Android

  1. Development Operating System:

Windows 10

  1. Build Tools:

N/A

  1. React Native version:

0.57.0-rc.3

  1. React Native Firebase Version:

5.6.0

  1. Firebase Module:

Firestore

  1. Are you using typescript?

No

TrekSoft commented 6 years ago

Actually it turns out there's a bunch more of them which I could list here as well if interested

Salakar commented 6 years ago

@TrekSoft Yes please, could you list them and I'll restructure the code. Thanks

Loving react-native-firebase and the support we provide? Please consider supporting us with any of the below:

TrekSoft commented 6 years ago

Thanks so much @Salakar ! Here's what I have:

Thanks again!

taschik commented 6 years ago

We are running in to the similar error as well:

Require cycle: node_modules/react-native-firebase/dist/modules/firestore/DocumentReference.js -> node_modules/react-native-firebase/dist/modules/firestore/DocumentSnapshot.js -> node_modules/react-native-firebase/dist/modules/firestore/utils/serialize.js -> node_modules/react-native-firebase/dist/modules/firestore/DocumentReference.js

Require cycles are allowed, but can result in uninitialized values. Consider refactoring to remove the need for a cycle.

That's our setup here: 

    System:
      OS: macOS High Sierra 10.13.6
      CPU: x64 Intel(R) Core(TM) i9-8950HK CPU @ 2.90GHz
      Memory: 985.18 MB / 32.00 GB
      Shell: 5.3 - /bin/zsh
    Binaries:
      Node: 10.10.0 - /usr/local/bin/node
      Yarn: 1.9.4 - /usr/local/bin/yarn
      npm: 6.4.1 - /usr/local/bin/npm
      Watchman: 4.9.0 - /usr/local/bin/watchman
    SDKs:
      iOS SDK:
        Platforms: iOS 12.0, macOS 10.14, tvOS 12.0, watchOS 5.0
      Android SDK:
        Build Tools: 23.0.1, 25.0.0, 26.0.3, 27.0.3, 28.0.1
        API Levels: 23, 26, 27, 28
    IDEs:
      Android Studio: 3.1 AI-173.4907809
      Xcode: 10.0/10A255 - /usr/bin/xcodebuild
    npmPackages:
      react: ^16.5.2 => 16.5.2
      react-native: ^0.57.0 => 0.57.0
    npmGlobalPackages:
      react-native-cli: 2.0.1
      react-native-git-upgrade: 0.2.7

Firebase: 

react-native-firebase@5.0.0-rc2

- Firebase/Core (5.8.1):
    - Firebase/CoreOnly
    - FirebaseAnalytics (= 5.1.4)
  - Firebase/CoreOnly (5.8.1):
    - FirebaseCore (= 5.1.3)
  - Firebase/Messaging (5.8.1):
    - Firebase/CoreOnly
    - FirebaseMessaging (= 3.1.2)
  - FirebaseAnalytics (5.1.4):
    - FirebaseCore (~> 5.1)
    - FirebaseInstanceID (~> 3.2)
    - GoogleAppMeasurement (~> 5.1)
    - GoogleUtilities/AppDelegateSwizzler (~> 5.2)
    - GoogleUtilities/MethodSwizzler (~> 5.2)
    - GoogleUtilities/Network (~> 5.2)
    - "GoogleUtilities/NSData+zlib (~> 5.2)"
    - nanopb (~> 0.3)
  - FirebaseCore (5.1.3):
    - GoogleUtilities/Logger (~> 5.2)
  - FirebaseInstanceID (3.2.1):
    - FirebaseCore (~> 5.1)
    - GoogleUtilities/Environment (~> 5.2)
  - FirebaseMessaging (3.1.2):
    - FirebaseCore (~> 5.0)
    - FirebaseInstanceID (~> 3.0)
    - GoogleUtilities/Reachability (~> 5.2)
    - Protobuf (~> 3.1)
Salakar commented 6 years ago

These can be ignored for now as it doesn't break anything, the code will be refactored in an upcoming major release to address these warnings.

You can hide them for now if you wish to:

import { YellowBox } from 'react-native';

YellowBox.ignoreWarnings(['Require cycle:']);
aidan-doherty commented 6 years ago

Has this been fixed yet? i am also getting the same yellow warnings.

aidan-doherty commented 6 years ago

I have set yellow box to ignore these warnings for now until it is updated. I assume it should be ok if nothing is breaking.

ziyafenn commented 5 years ago

Same problem here. RN 0.57.2

burtek commented 5 years ago

Can confirm, also getting those

jstansbe commented 5 years ago

I'm using core, messaging, and notifications for Android

    "react-native": "0.57.4",
    "react-native-firebase": "^5.0.0"

console output (the below occurs 10 times):

Require cycle: node_modules\react-native-firebase\dist\utils\apps.js -> node_modules\react-native-firebase\dist\modules\core\app.js -> node_modules\react-native-firebase\dist\utils\apps.js
FadiAboMsalam commented 5 years ago

same problem here

    "react-native": "0.57.3",
   "react-native-firebase": "^5.1.0",
Salakar commented 5 years ago

See: https://github.com/facebook/metro/issues/287

MrLoh commented 5 years ago

I cannot get rid of them in my console. Would be great if this could be fixed, as it really isn't a very good coding practice

llaine commented 5 years ago

Same problem here: 

    "react-native": "0.57.8",
    "react": "16.6.3",
    "react-native-firebase": "^5.0.0",
radinreth commented 5 years ago

All you need to do to get rid of the warning from console is to patch it in metro ./node_modules/metro/src/lib/polyfills/require.js because the warning come from there,

Luckily I found Exilz, does this job well. Feel free to check it out ->

ospfranco commented 5 years ago

Require cycles on their own are not bad, while the warning is justified I think trying to get rid of require cycles completely is a bad idea for most projects, this included, adding it to the ignored list would be my way to go.

vdlindenmark commented 5 years ago

I'm also getting this on RN0.59... Would be great if this could be fixed.. :-)

vgm8 commented 5 years ago

Same here, having the same error in RN0.59

RWOverdijk commented 5 years ago

Pretty annoying 😄 Anything we can do to help?

Salakar commented 5 years ago

This has been completely solved in v6 and a drastic internals rework was the only way to solve something like this, unfortunately.

v6 is still WIP but if you're one of the lucky few to only need the Firebase modules listed on the v6 changelog then you can switch to v6 already (instructions on changelog). The modules listed there are ready for use despite their 'alpha' tag on npm as these are more like a 'RC' version - alpha tagged as some modules (the ones not listed there) still need to be migrated to v6 still before it can be released - and all the modules require the same version.

RWOverdijk commented 5 years ago

I can't wait to use that. I've been waiting for the stack traces... So sexy.

Unfortunately cloud messaging isn't in that list yet.

Thanks for being awesome. 😎

TrekSoft commented 5 years ago

Thanks so much Salakar! I'm now teaching a college night class on the side with my job at Amazon and have all of them using Firebase with your library since it is by far the easiest and best database integration I know of with mobile.

asherccohen commented 5 years ago

Hi, is this issue now resolved or should we wait for v6?

mikehardy commented 5 years ago

@i1990jain that is abusive.

Salakar commented 5 years ago

For the record; this was the comment:

image

The maintainers and contributors work tirelessly on this project and in most cases during their own personal time. When users such as yourself make these kinds of comments but they themselves don't make an effort to contribute it's hurtful to those who do make the effort.

We don't tolerate any kind of derogatory comment, so @i1990jain consider this your final warning, you've been blocked from our projects for 72 hours, I suggest you read up on the code of conduct once you come back and before you have any further interactions with this project.

stale[bot] commented 5 years ago

Hello 👋, to help manage issues we automatically close stale issues. This issue has been automatically marked as stale because it has not had activity for quite some time. Has this issue been fixed, or does it still require the community's attention?

This issue will be closed in 15 days if no further activity occurs. Thank you for your contributions.

Ehesp commented 5 years ago

FYI this will be fixed in v6 which is currently in progress.

SaeedZhiany commented 5 years ago

@Salakar Here is the list of cycles warning I'm getting, it may be helpful cause I didn't see more of them in the other comments.

Require cycle: node_modules\react-native-firebase\dist\utils\apps.js -> node_modules\react-native-firebase\dist\modules\core\app.js -> node_modules\react-native-firebase\dist\utils\apps.js

Require cycle: node_modules\react-native-firebase\dist\modules\admob\index.js -> node_modules\react-native-firebase\dist\modules\admob\Interstitial.js -> node_modules\react-native-firebase\dist\modules\admob\index.js

Require cycle: node_modules\react-native-firebase\dist\modules\admob\index.js -> node_modules\react-native-firebase\dist\modules\admob\RewardedVideo.js -> node_modules\react-native-firebase\dist\modules\admob\index.js

Require cycle: node_modules\react-native-firebase\dist\modules\database\Reference.js -> node_modules\react-native-firebase\dist\utils\SyncTree.js -> node_modules\react-native-firebase\dist\modules\database\Reference.js

Require cycle: node_modules\react-native-firebase\dist\modules\core\firebase.js -> node_modules\react-native-firebase\dist\utils\apps.js -> node_modules\react-native-firebase\dist\modules\core\app.js -> node_modules\react-native-firebase\dist\modules\database\index.js -> node_modules\react-native-firebase\dist\modules\core\firebase.js

Require cycle: node_modules\react-native-firebase\dist\modules\firestore\DocumentSnapshot.js -> node_modules\react-native-firebase\dist\modules\firestore\DocumentReference.js -> node_modules\react-native-firebase\dist\modules\firestore\DocumentSnapshot.js

Require cycle: node_modules\react-native-firebase\dist\modules\firestore\CollectionReference.js -> node_modules\react-native-firebase\dist\modules\firestore\Query.js -> node_modules\react-native-firebase\dist\modules\firestore\QuerySnapshot.js -> node_modules\react-native-firebase\dist\modules\firestore\DocumentChange.js -> node_modules\react-native-firebase\dist\modules\firestore\DocumentSnapshot.js -> node_modules\react-native-firebase\dist\modules\firestore\DocumentReference.js -> node_modules\react-native-firebase\dist\modules\firestore\CollectionReference.js

Require cycle: node_modules\react-native-firebase\dist\modules\firestore\DocumentReference.js -> node_modules\react-native-firebase\dist\modules\firestore\utils\serialize.js -> node_modules\react-native-firebase\dist\modules\firestore\DocumentReference.js

Require cycle: node_modules\react-native-firebase\dist\modules\firestore\utils\serialize.js -> node_modules\react-native-firebase\dist\modules\firestore\FieldValue.js -> node_modules\react-native-firebase\dist\modules\firestore\utils\serialize.js

Require cycle: node_modules\react-native-firebase\dist\modules\core\firebase.js -> node_modules\react-native-firebase\dist\utils\apps.js -> node_modules\react-native-firebase\dist\modules\core\app.js -> node_modules\react-native-firebase\dist\modules\functions\index.js -> node_modules\react-native-firebase\dist\modules\core\firebase.js

Require cycle: node_modules\react-native-firebase\dist\modules\storage\index.js -> node_modules\react-native-firebase\dist\modules\storage\reference.js -> node_modules\react-native-firebase\dist\modules\storage\task.js -> node_modules\react-native-firebase\dist\modules\storage\index.js
Salakar commented 5 years ago

Thanks @SaeedZhiany - these have all been sorted in the upcoming v6 release

Ehesp commented 5 years ago

Pushed up the v6 Firestore changes for this earlier today.

stale[bot] commented 5 years ago

Hello 👋, to help manage issues we automatically close stale issues. This issue has been automatically marked as stale because it has not had activity for quite some time. Has this issue been fixed, or does it still require the community's attention?

This issue will be closed in 15 days if no further activity occurs. Thank you for your contributions.

ketan1062 commented 4 years ago

having the same error in RN0.61.2

Ehesp commented 4 years ago

v6 has been released for a few months now with this fix.