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

possible merge with feature #66

Open
farfromrefug opened this issue Apr 9, 2020 · 6 comments
Open

possible merge with feature #66

farfromrefug opened this issue Apr 9, 2020 · 6 comments

Comments

@farfromrefug
Copy link
Member

Hey @EddyVerbruggen i have a fork here with a bunch of features and bug fixes:

  • added multipart support
  • added cache support (GET requests)
  • cookie support
  • fixed ios error return format which was not consistent with {N} http modue
  • updated deps

I tried to create single feature PRs but it was too tough as there are quite a few changes code/structure wise

If you are interested i can create a "big" PR as merge from my master branch so you can test and decide to merge or not.
Would you be interested?

I am also planning on changing a few other things, like not returning a string version of the response but instead do it the same way as {N}. I am even thinking of adding a way to do those "transformations" (toJSON, toString,...) in the native code so that it is done on the background thread

@EddyVerbruggen
Copy link
Member

Hey @farfromrefug!

That sounds great! Feel free to merge to master of this repo. I trust your work and I can do without the overhead atm. Eventually we can release a 3.0.0 based on your work.

@paulgovers
Copy link

Hi @farfromrefug and @EddyVerbruggen
are there plans for implementing to possibility to transmitting a client certificate during the TLS handshake when executing a http call?

@farfromrefug
Copy link
Member Author

@paulgovers not that i know but PRs are always welcome ;)

@farfromrefug
Copy link
Member Author

@EddyVerbruggen i merged and pushed on the repo.
That would be great for you to take a quick look at it.
One thing is i cant get the code to format as you. even with the tslint conf file. Not sure why.
Would you be ok to switch to eslint? Do you have a config file you want?

@kefahB
Copy link
Contributor

kefahB commented Apr 20, 2020

@farfromrefug wonderful work 👍

I would like to suggest to set the check of Content-Type when is set to 'application/jsonbecause of the charsetapplication/json; charset=utf-8` like this :

if (opts.headers && opts.headers['Content-Type'].substring(0, 16) === 'application/json') {
        manager.requestSerializer = AFJSONRequestSerializer.serializer();
        manager.responseSerializer = AFJSONResponseSerializer.serializerWithReadingOptions(
            NSJSONReadingOptions.AllowFragments
        );
}

@farfromrefug
Copy link
Member Author

@kefahB indeed! i will go with string.startWith if you agree

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

4 participants