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

Automatically pull headers from node-form-data. #31

Merged
merged 1 commit into from
Jul 22, 2015
Merged

Automatically pull headers from node-form-data. #31

merged 1 commit into from
Jul 22, 2015

Conversation

DylanPiercey
Copy link
Contributor

This is convenient for isomorphic apps that rely on form-data.

@bitinn
Copy link
Collaborator

bitinn commented Jul 22, 2015

Hi, thx for the PR, but I don't think node-fetch should have knowledge about body.getBoundary.

So really we are making a special case for node-form-data, while my example is just to demonstrate that it works with node-form-data.

Any reason you have to do this instead of passing headers separately?

@DylanPiercey
Copy link
Contributor Author

I understand that it is a bit specific to node-form-data however I think it would be a nice add on as both libraries are attempting to achieve similar goals (isomorphic ajax requests).

Currently I have to write code something like this to make it work isomorphically.

form = new FormData();

form.append("file", file);

// Check if I am on the server :(
if (!process.browser) {
  headers = form.getHeaders();
  for (field in headers) {
    options.headers[field] = headers[field];
  }
}

// Use node-fetch with options

It would be awesome if this small exception could be added (similar to supporting streams) to allow for a nice serverside (form-data) experience that also works in the browser.

@bitinn
Copy link
Collaborator

bitinn commented Jul 22, 2015

Alright I get your use-case here, here is my understanding:

  • It looks like you would like FormData support on node-fetch, which is fair enough.
  • The main problem: unlike streams, no spec for FormData on node.js
  • The main difference between client-side FormData api and node-form-data: on client-side Fetch API generate the body payload and boundary, on node.js, node-form-data handles it.
  • To keep node-fetch lightweight and free from dependency, we were only supporting the alternative method mentioned on node-form-data.
  • So unfortunately this use case is not as isomorphic as it seems.

I will take the PR, as I see no better solution to detect FormData reliably on node.js

bitinn added a commit that referenced this pull request Jul 22, 2015
Automatically pull headers from node-form-data.
@bitinn bitinn merged commit 18b0f50 into node-fetch:master Jul 22, 2015
@bitinn
Copy link
Collaborator

bitinn commented Jul 22, 2015

PS: to clarify my personal stand on isomorphic javascript, I think it can be misleading to developers who thinks the code just works both client-side and server-side, when the truth is far from it (what we are doing is a good example of such problem).

@bitinn
Copy link
Collaborator

bitinn commented Jul 22, 2015

released as v1.3.2, thx for your work.

@DylanPiercey
Copy link
Contributor Author

Thanks a lot for your work to get this merged and released so quickly. I understand your viewpoint (that devs should realize that the code on the server is often different from the client). Personally I think that this is explained well in both node-fetch and node-form-data and that users of both understand that they work with native node modules under the hood and expose some api differences (Streams and such).

I am glad to see this merged as both libraries together act as a great (complete), abstraction for isomorphic fetching.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants