joltup / rn-fetch-blob

A project committed to making file access and data transfer easier, efficient for React Native developers.
MIT License
2.81k stars 770 forks source link

Require cycle #183

Open mengerzhuanyong opened 5 years ago

mengerzhuanyong commented 5 years ago

Require cycle: node_modules/rn-fetch-blob/index.js -> node_modules/rn-fetch-blob/polyfill/index.js -> node_modules/rn-fetch-blob/polyfill/FileReader.js -> node_modules/rn-fetch-blob/index.js

It is recommended to solve this problem!

reilem commented 5 years ago

I'm also getting these warnings on the new version of react-native.

Require cycle: node_modules/rn-fetch-blob/index.js -> node_modules/rn-fetch-blob/polyfill/index.js -> node_modules/rn-fetch-blob/polyfill/Blob.js -> node_modules/rn-fetch-blob/index.js
Require cycle: node_modules/rn-fetch-blob/index.js -> node_modules/rn-fetch-blob/polyfill/index.js -> node_modules/rn-fetch-blob/polyfill/XMLHttpRequest.js -> node_modules/rn-fetch-blob/index.js
Require cycle: node_modules/rn-fetch-blob/index.js -> node_modules/rn-fetch-blob/polyfill/index.js -> node_modules/rn-fetch-blob/polyfill/FileReader.js -> node_modules/rn-fetch-blob/index.js
Require cycle: node_modules/rn-fetch-blob/index.js -> node_modules/rn-fetch-blob/polyfill/index.js -> node_modules/rn-fetch-blob/polyfill/Fetch.js -> node_modules/rn-fetch-blob/index.js
joegoodall2 commented 5 years ago

Same issue

zhaiyjgithub commented 5 years ago

same issue

mitevdev commented 5 years ago

same issue

tenwiz commented 5 years ago

Yes, I've faced the same issue here.

tenwiz commented 5 years ago

Require cycle: node_modules/rn-fetch-blob/index.js -> node_modules/rn-fetch-blob/polyfill/index.js -> node_modules/rn-fetch-blob/polyfill/Blob.js -> node_modules/rn-fetch-blob/index.js

tskorupka commented 5 years ago

Same issue :(

burtek commented 5 years ago

same here

rodrigowpl commented 5 years ago

same here, any idea?

sithartha commented 5 years ago

I've got the same.

tylerbarnett commented 5 years ago

Same Here

AsadSaleh commented 5 years ago

Any one already working on this issue? Or any one has an idea to solve this?

wd commented 5 years ago

@AsadSaleh This is not a critical problem, just a warning for 'maybe there is something wrong in your code', so you can simply ignore this warning by add codes like bellow, and wait someone will fix this warning ...

YellowBox.ignoreWarnings([
    'Remote debugger is in a background tab which may cause apps to perform slowly',
    'Require cycle: node_modules/rn-fetch-blob/index.js',
    'Require cycle: node_modules/react-native/Libraries/Network/fetch.js'
]);
qiaolin-pan commented 5 years ago

any update on this ?

soumyamishra89 commented 5 years ago

@wd This is a warning but needs to be dealt with. This has come up with new react-native version (0.57) and requires certain code restructuring in rn-fetch-blob library.

Is anyone working on this?

AGOGD commented 5 years ago

I also encountered this problem today.

zhuxianqin commented 5 years ago

Modify these files can avoid require cycle: IN DIR rn-fetch-blob/polyfill all thease 4 files: Blob.js, Fetch.js, FileReader.js, XMLHttpRequest.js

// import RNFetchBlob from '../index.js' import {NativeModules} from 'react-native'; const RNFetchBlob = NativeModules.RNFetchBlob

nhuthuynh195 commented 5 years ago

Modify these files can avoid require cycle: IN DIR rn-fetch-blob/polyfill all thease 4 files: Blob.js, Fetch.js, FileReader.js, XMLHttpRequest.js

// import RNFetchBlob from '../index.js' import {NativeModules} from 'react-native'; const RNFetchBlob = NativeModules.RNFetchBlob

this problem has resolved, thanks brother

Zbish commented 5 years ago

there is going to be a PR for @zhuxianqin fix ???

Faolain commented 5 years ago

@zhuxianqin can you please submit a PR with this fix?

zhuxianqin commented 5 years ago

@zhuxianqin can you please submit a PR with this fix?

sorry, I don't know how to release github project, I will try to do it in several days

zhuxianqin commented 5 years ago

@zhuxianqin can you please submit a PR with this fix?

to develop this project, I need to follow instructions below: https://github.com/joltup/rn-fetch-blob-dev I have to test both on android & IOS Sorry, I think it's better for the original owner to fix it

msqar commented 5 years ago

@zhuxianqin seems that they are not adding it, it sucks man :(

Seems that you know this library a lot, though. Can you help me with something? I created this post https://github.com/joltup/rn-fetch-blob/issues/305

ThaJay commented 5 years ago

This should be fixed. I'm glad react-native now warns about this but there should have never been cyclic dependencies in the first place.

here it is solved with constructor injection: https://stackoverflow.com/questions/38841469/how-to-fix-this-es6-module-circular-dependency/44226565#44226565

https://github.com/react-native-community/react-native-svg/issues/773

Another way to solve this is to make an interface with a setter and a getter and require the interface instead of the original class. The original file uses the setter and all others use the getter.

interface.js:

let thing = {}

export function getThing () {
  return thing
}

export function setThing (thingToSet) {
  thing = thingToSet
}

thing.js:

import {setThing} from './interface'

class Thing {}

setThing(Thing)

other files:

import {getThing} from './interface'

const Thing = getThing()
msqar commented 5 years ago

@ThaJay yea man, i receive like 6-7 warning notifications every time i refresh the app, is getting super annoying. Do you have any idea how to apply this interface for this specific case?

vitorverasm commented 5 years ago

Modify these files can avoid require cycle: IN DIR rn-fetch-blob/polyfill all thease 4 files: Blob.js, Fetch.js, FileReader.js, XMLHttpRequest.js

// import RNFetchBlob from '../index.js' import {NativeModules} from 'react-native'; const RNFetchBlob = NativeModules.RNFetchBlob

This did the trick on RN 0.57 This bug should be fixed...

msqar commented 5 years ago

@vitor-veras i have to put the fetch-blob file in my app and edit it then? no other way until they fix this? :/ i don't get it, i assume i don't have to update these files directly from node_modules otherwise it will be over-written whenever i do another npm install... so how should i do it?

vitorverasm commented 5 years ago

That's only a warning from React Native, but it can slowdown the app performance. And yes if you update the package with npm/yarn the fix will go away. There's no proper aproach to this i think, hopefully the dev team fix this soon. There is actually a PR for this #296 .

Faolain commented 5 years ago

As mentioned by @alpha0010 in #296 one would encounter problems where RNFetchBlob is called within the files the import is removed from.

@ThaJay would you be able to submit a PR?

FibreFoX commented 5 years ago

Just stupid me, but why are these polyfills even required here? As RN 0.59 comes with a new JavaScriptCore, are these polyfills still necessary? Or is there even a way to DISABLE these polyfills, like a non-polyfilled version of the lib?

ThaJay commented 5 years ago

I see. The next version of this could have these imports removed and only be compatible with 59 and up. https://github.com/facebook/react-native/releases

Bhavdip commented 5 years ago

@zhuxianqin solution works perfectly.

cristianoccazinsp commented 5 years ago

Any updates? Seeing the same warnings

mayconmesquita commented 5 years ago

Any updates here?

ithustle commented 5 years ago

+1

alpha0010 commented 5 years ago

Please test #357; I have insufficient time currently.

npm install alpha0010/rn-fetch-blob#require-cycle

NoumanSakhawat commented 5 years ago

Modify these files can avoid require cycle: IN DIR rn-fetch-blob/polyfill all thease 4 files: Blob.js, Fetch.js, FileReader.js, XMLHttpRequest.js

// import RNFetchBlob from '../index.js' import {NativeModules} from 'react-native'; const RNFetchBlob = NativeModules.RNFetchBlob

Thanks Buddy this is working

tuanduc2904 commented 4 years ago

Modify these files can avoid require cycle: IN DIR rn-fetch-blob/polyfill all thease 4 files: Blob.js, Fetch.js, FileReader.js, XMLHttpRequest.js

// import RNFetchBlob from '../index.js' import {NativeModules} from 'react-native'; const RNFetchBlob = NativeModules.RNFetchBlob

I did the same thing, but I had problems calling the RNFetchBlob.config function. This is my code

import {NativeModules} from 'react-native';
const { RNFetchBlob } = NativeModules;
RNFetchBlob.config(.....);

My code snippet shows an error: TypeError: undefined is not a function (near '...RNFetchBlob.config...')

Deg5112 commented 4 years ago

you can not just modify the node_module files i don't know why people are offering that as a solution

This needs to be fixed directly on this repo

To help people understand the actual import problem

illum_mobile____Sites_illum_mobile__-_____node_modules_rn-fetch-blob_polyfill_index_js

This is essentially a circular dependency, bad design: https://en.wikipedia.org/wiki/Circular_dependency Needs to be reworked to avoid this, this should of been an obvious mistake during development.

a simple solution is to pass RNFetchBlob as an argument to Fetch, essentially injecting as a dependency. I'm sure a better solution is available but this is just an idea.

let RNFetchBlob = {
  Blob, File, XMLHttpRequest, ProgressEvent, Event, FileReader
};

RNFetchBlob = {...RNFetchBlob, Fetch(RNFetchBlob)} //or maybe just a static setter Fetch.setRnFetchBlock(Fetch)

export default RNFetchBlob
ThaJay commented 4 years ago

@Deg5112 This is indeed one of the possible solutions to inject a dependency. I proposed another possibility but alas, no response thus far. I am not actively working with this at the moment but in a few weeks I might if nobody else has stepped in by then.

CyrusZei commented 4 years ago

any solution on this? or pr ?

NoumanSakhawat commented 4 years ago

any solution on this? or pr ?

Here you go:

https://github.com/joltup/rn-fetch-blob/issues/183#issuecomment-450826541

ThaJay commented 4 years ago

@NoumanSakhawat That is not a solution, it's a workaround that has to be done after every node modules install. I guess the owner of this does not care because it has been 1,5 years by now.

jessicarobins commented 4 years ago

@ThaJay still a hack but you can use patch-package so at least you only have to modify it once and not after every npm install

ThaJay commented 4 years ago

@jessicarobins That looks like the first decently clean workaround I have come across.

asasouza commented 4 years ago

The problem continues in version 0.12.0, RN v 0.62.2

osamaaamer95 commented 4 years ago

@asasouza install patch-package, copy and paste this in a patch file named rn-fetch-blob+0.12.0.patch under /patches, reinstall node modules and run patch-package.

diff --git a/node_modules/rn-fetch-blob/polyfill/Blob.js b/node_modules/rn-fetch-blob/polyfill/Blob.js
index 53662a7..379a7ad 100644
--- a/node_modules/rn-fetch-blob/polyfill/Blob.js
+++ b/node_modules/rn-fetch-blob/polyfill/Blob.js
@@ -2,7 +2,9 @@
 // Use of this source code is governed by a MIT-style license that can be
 // found in the LICENSE file.

-import RNFetchBlob from '../index.js'
+// import RNFetchBlob from '../index.js'
+import {NativeModules} from 'react-native';
+const RNFetchBlob = NativeModules.RNFetchBlob
 import fs from '../fs.js'
 import getUUID from '../utils/uuid'
 import Log from '../utils/log.js'
diff --git a/node_modules/rn-fetch-blob/polyfill/Fetch.js b/node_modules/rn-fetch-blob/polyfill/Fetch.js
index 3ecb591..fbb0702 100644
--- a/node_modules/rn-fetch-blob/polyfill/Fetch.js
+++ b/node_modules/rn-fetch-blob/polyfill/Fetch.js
@@ -1,4 +1,6 @@
-import RNFetchBlob from '../index.js'
+// import RNFetchBlob from '../index.js'
+import {NativeModules} from 'react-native';
+const RNFetchBlob = NativeModules.RNFetchBlob
 import Log from '../utils/log.js'
 import fs from '../fs'
 import unicode from '../utils/unicode'
diff --git a/node_modules/rn-fetch-blob/polyfill/XMLHttpRequest.js b/node_modules/rn-fetch-blob/polyfill/XMLHttpRequest.js
index 9036b2b..f1a99e4 100644
--- a/node_modules/rn-fetch-blob/polyfill/XMLHttpRequest.js
+++ b/node_modules/rn-fetch-blob/polyfill/XMLHttpRequest.js
@@ -2,7 +2,9 @@
 // Use of this source code is governed by a MIT-style license that can be
 // found in the LICENSE file.

-import RNFetchBlob from '../index.js'
+// import RNFetchBlob from '../index.js'
+import {NativeModules} from 'react-native';
+const RNFetchBlob = NativeModules.RNFetchBlob
 import XMLHttpRequestEventTarget from './XMLHttpRequestEventTarget.js'
 import Log from '../utils/log.js'
 import Blob from './Blob.js'
asasouza commented 4 years ago

Thanks for the answer @osamaaamer95, after read the @jessicarobins comment this was my workaround too and worked.

cmcleese commented 4 years ago

Thank you @osamaaamer95 for this updated patch file. From https://github.com/joltup/rn-fetch-blob/issues/183#issuecomment-621968820 . This works for me.

deltamish commented 4 years ago

@osamaaamer95 did the exact same thing but still facing the issue D:/Work/Projects/VS/react-native-test/test-pdf-app/node_modules/rn-fetch-blob/index.js 12:12 Module parse failed: Unexpected token (12:12) You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders | AppState, | } from 'react-native'

import type { | RNFetchBlobNative, | RNFetchBlobConfig,

any idea why