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

No way to specify the same query param twice #2

Open
DanielHeath opened this issue Sep 16, 2015 · 12 comments
Open

No way to specify the same query param twice #2

DanielHeath opened this issue Sep 16, 2015 · 12 comments

Comments

@DanielHeath
Copy link

I'm using an API which specifies filters by passing the same URL parameter twice.

From reading the code, I'd need to provide an alternative implementation of serializeObject that could do this.

Would a PR to make hooking that possible be accepted?

@jednano
Copy link
Owner

jednano commented Sep 17, 2015

Yes it would!

@jednano
Copy link
Owner

jednano commented Sep 17, 2015

@DanielHeath the dependencies might be a bit stale. Feel free to update them. Also, this project uses TypeScript, so you might want to download Visual Studio Code for free on Windows/Mac/Linux.

@jednano
Copy link
Owner

jednano commented Sep 17, 2015

done and done

@DanielHeath
Copy link
Author

That's just the dep update, right?

@DanielHeath
Copy link
Author

Should I be editing lib or d.ts?

@DanielHeath
Copy link
Author

Also, it looks like data is totally ignored on the node side; is that right?

@jednano
Copy link
Owner

jednano commented Sep 17, 2015

Yes. I only updated the dependencies.

You want to edit files in the lib folder. The d.ts folder is generated. I actually need to clean up how these generated files are being pushed to GitHub, but I don't have a great strategy for that yet.

What do you mean data is totally ignored on the Node side? You can look at runIsomorphicTests for the tests that run on both Node and in-browser. Otherwise, node/Http.spec.ts has the tests that are specific only to Node and browser/Http.spec.ts has the tests specific only to the browser.

@jednano
Copy link
Owner

jednano commented Sep 17, 2015

BTW – what OS are you developing on? If Mac, you might have to disable the IE test, locally. If on Windows, you might have to disable Safari. Just FYI.

@DanielHeath
Copy link
Author

lib/node/Http.ts doesn't mention Helpers or this.data anywhere.

Therefore, if you pass data it'll be ignored when making a request on the server, right?

@jednano
Copy link
Owner

jednano commented Sep 17, 2015

If you're talking about sending data, you have to send it in the headers. Whether the server ignores it or not depends on your server. This library is intended to be very minimal and basic. Anything above and beyond simple HTTP requests would have to be another library that extends this one (e.g., passing data as an option, like jQuery).

If you want to receive data, that's handled here and given to you as response.text.

Where did Helpers come from?

@DanielHeath
Copy link
Author

I'm referring to https://github.com/jedmao/iso-http/blob/master/lib/browser/Http.ts#L49 when I talk about data, and https://github.com/jedmao/iso-http/blob/master/lib/Helpers.ts when I talk about Helpers

@jednano
Copy link
Owner

jednano commented Sep 28, 2015

The Node HTTP client doesn't need Helpers to function, so that's why it doesn't reference them. As for the data, I must not have documented that, so I can't remember myself. I vaguely remember having to send the data in the headers, but that's not very isomorphic (i.e., doesn't work the same across clients).

Also, it doesn't look like there are any tests against data, which might be valuable.

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

No branches or pull requests

2 participants