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.

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

Switch to isomorphic-fetch #61

Closed
wants to merge 1 commit into from
Closed

Conversation

zpao
Copy link
Member

@zpao zpao commented Sep 25, 2015

Fixes #60

This should be pretty safe. With browserify and webpack packaging, this will use whatwg-fetch (exactly the same code we had).

@zpao
Copy link
Member Author

zpao commented Sep 25, 2015

Hmm, this actually needs a little bit more… going to try to actually build relay first (are some issues atm)

@zpao
Copy link
Member Author

zpao commented Sep 26, 2015

Ok, confirmed we're all good with facebook/relay#393.

I'm not wild about the versioning here. I didn't think about changes like this to fbjs needing corresponding bumps in fbjs-scripts. Oh well, we'll bump minors for both here.

@zpao
Copy link
Member Author

zpao commented Sep 30, 2015

cc @boourns - can you test this branch again with RN? I'm hoping by preempting the require we can still export something useful if it exists.

@boourns
Copy link

boourns commented Sep 30, 2015

I will try it tonight or early tomorrow.

On Wednesday, 30 September 2015, Paul O’Shannessy notifications@github.com
wrote:

cc @boourns https://github.com/boourns - can you test this branch again
with RN? I'm hoping by preempting the require we can still export something
useful if it exists.


Reply to this email directly or view it on GitHub
#61 (comment).

@boourns
Copy link

boourns commented Oct 1, 2015

OK. This PR does not work on react native currently.

After some debugging I've found that the problem is the @providesModule fetch line here:
https://github.com/zpao/fbjs/blob/isomorphic-fetch/src/__forks__/fetch.js#L9

I don't quite understand why, but with @providesModule in place, the rest of ./fetch.js is never executed when fetchWithRetries.js requires ./fetch (you can confirm this with a console.log). And what's returned by require('./fetch') in fetchWithRetries.js is a module object that looks like it has the fetch method available as fetch.fetch:
image

With the @providesModule line removed, ./fetch.js runs as expected, exports the fetch function, and this PR functions as desired on react native.

@boourns
Copy link

boourns commented Oct 2, 2015

@zpao did my feedback make sense?

@denvned
Copy link

denvned commented Nov 20, 2015

Is the problem described by @boourns still actual? Are there any other problems that prevent this from being merged?

@jeswin
Copy link

jeswin commented Dec 23, 2015

+1 to getting this merged. Server-side Relay takes a lot of manual patching right now.

@sophiebits
Copy link
Contributor

Happened in #95.

@sophiebits sophiebits closed this Jan 5, 2016
@zpao zpao deleted the isomorphic-fetch branch February 10, 2016 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants