-
Notifications
You must be signed in to change notification settings - Fork 792
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 for #183 #688
Fix for #183 #688
Conversation
Any update on approving the PR ? |
@@ -2,12 +2,13 @@ | |||
// 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 {NativeModules} from 'react-native'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider having module in polyfill/RNFetchBlob.js
:
import { NativeModules } from 'react-native';
const RNFetchBlob = NativeModules.RNFetchBlob
export default RNFetchBlob;
Seems like trivial module but allows to have single shared source.
Hi there! If everything is OK, could be great to merge it really... |
Removed all the requiring cycles by changing all to use directly the NativeModules
Anything else we need to get this merge? thanks |
Please merge this pr~ 🥲. I don`t want to see warning about it. |
Please merge this pr~ 🥲. I don`t want to see warning about it. |
This PR has already been merged to master. It hasn't yet been published to NPM; we're working on that. In the meantime you can point your package.json entry to the git commit or wait for the next package version. I'm hoping to publish a beta version in the next week or so that will contain this change and many others that have recently been merged. |
Haven't publish to NPM? |
Yeah, I apologize for the delay. It took a while to get the appropriate permissions and sort out a few things. I just published Since inheriting this project, I am not familiar with testing strategies that have been used in the past. I'll continue to look into this and other solutions to enhance testability. Unfortunately, I don't have a lot of time to dedicate to this but I'm hoping to at least begin to regain some momentum. |
Fixing #183
Removed all the requiring cycles by changing all to use directly the
NativeModules
Thanks to #183 (comment)