-
Notifications
You must be signed in to change notification settings - Fork 788
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
rn-fetch-blob Implementation for React Native for Windows #701
base: master
Are you sure you want to change the base?
Conversation
We'll need to figure out what to do about this PR since it looks like this repo is no longer maintained (#666). Even so, I think it's worthwhile to iterate here while we figure out that plan. In other words, lets see if we can get this PR as finished as possible so that wherever it ends up, it'll be clean and ready to go! CC: @vmoroz @jonthysell @NickGerleman @asklar @bzoz - FYI |
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.
File path now working fine
Hi @avmoroz, These are the App.js contents: import React from 'react';
import {
Button
} from 'react-native';
import RNFetchBlob from 'rn-fetch-blob';
const App: () => React$Node = () => {
return (
<Button title="Download a pdf file"
onPress = { () => {
const cacheFile = RNFetchBlob.fs.dirs.CacheDir + '/sample.pdf';
RNFetchBlob.config({
path: cacheFile,
trusty: true
}).fetch(
'GET',
'http://samples.leanpub.com/thereactnativebook-sample.pdf'
).then(
async (res) => {
let a = 0;
if (res && res.respInfo && res.respInfo.headers && !res.respInfo.headers["Content-Encoding"] && !res.respInfo.headers["Transfer-Encoding"] && res.respInfo.headers["Content-Length"]) {
const expectedContentLength = res.respInfo.headers["Content-Length"];
let actualContentLength;
try {
const fileStats = await RNFetchBlob.fs.stat(res.path());
if (!fileStats || !fileStats.size) {
throw new Error("FileNotFound:" + source.uri);
}
actualContentLength = fileStats.size;
alert('Expected content length:' + expectedContentLength + ' Actual content length: ' + actualContentLength);
} catch (error) {
alert("DownloadFailed: " + error);
}
} else {
alert('Response headers were not the expected ones.');
}
}
).catch(
(error) => alert("Fetch call failed: " + error)
)
}}
/>
);
};
export default App; This is the way I've set up the project on Windows:
On Windows, I get an alert with "Response headers were not the expected ones." and I find that no file seems to have been saved. When running the same application for Android ( I hope this is helpful in getting similar behavior for the Windows implementation. |
windows/RNFetchBlob/RNFetchBlob.cpp
Outdated
} | ||
else | ||
{ | ||
promise.Reject("EUNSPECIFIED: Failed to write to file."); |
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.
probably want to pass the hresult at least. You can use the hresult_error
's message()
member function to get a string representation of the error
windows/RNFetchBlob/RNFetchBlob.cpp
Outdated
} | ||
catch (...) | ||
{ | ||
promise.Reject("EEXIST: File already exists."); // TODO: Include filepath |
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.
it's not always going to be a file already exists here
windows/RNFetchBlob/RNFetchBlob.cpp
Outdated
} | ||
else | ||
{ | ||
promise.Reject("Invalid encoding"); |
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.
log out the encoding as part of the rejection message
{ | ||
httpMethod = winrt::Windows::Web::Http::HttpMethod::Get(); | ||
} | ||
else if (method.compare("POST") != 0 && method.compare("post") != 0) |
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.
I think this can just be an else
@jaimecbernardo good catch. I think the current PR is returning the content stream as the res, so the response info, headers, etc. is only used for progress notifications, and once it's completed, it is thrown away and not available to the JS code. Seems like a good thing to add if it is part of the module API/contract. |
Hey, |
that would be fine by me seeing as this repo is marked as unmaintained :) thanks for volunteering! Once things are up and running we should also update the reactnative.directory site. |
@asklar yes, absolutely right. I will see what I can get done the next days |
@avmoroz @asklar I integrated it in my repo here: https://github.com/RonRadtke/react-native-blob-util |
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.
I've tested the changes and it looks like the files are now being saved on disk. There's some flakiness on this when used with react-native-pdf, though, to which I'm still trying to get to the bottom of.
On Windows, I still get an alert with "Response headers were not the expected ones." for the sample I gave earlier.
{ | ||
callback("Source file not found."); | ||
} | ||
callback(winrt::to_string(ex.message()).c_str()); |
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.
When the error is 0x8007002, callback
is called twice, which is causing a runtime error crash.
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.
I'm trying to leave a comment in the file but there's a github outage, let's see if this works.
This needs a #include <windows.h>
in the module's pch.h
^ I've fixed the current build issues I was facing on this module with a |
@avmoroz hello, for some reason I can not build the solution of the windows implementation in Visual Studio. First, I had to change this in the <PropertyGroup Label="ReactNativeWindowsProps">
<ReactNativeWindowsDir Condition="'$(ReactNativeWindowsDir)' == ''">$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), 'node_modules\react-native-windows\package.json'))\node_modules\react-native-windows\</ReactNativeWindowsDir>
</PropertyGroup> to
I'm running in a monorepo, so without this change, the solution is trying to load RNW from a location that doesn't contain RNW. Then, when I tried to build the project, I've got the following errors:
It's weird because it's errors in RNW.. And it's only happening for this project only, which means something in the solution makes errors to appear. Other community native modules are built fine. I'm using react-native-windows 0.64.23. |
@br4in3x please use the version in https://github.com/RonRadtke/react-native-blob-util/ - the joltup/rn-fetch-blob repo is unmaintained. |
Created an implementation of rn-fetch-blob for React Native for Windows.