https icon indicating copy to clipboard operation
https copied to clipboard

possible merge with feature

Open farfromrefug opened this issue 5 years ago • 6 comments

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

farfromrefug avatar Apr 09 '20 12:04 farfromrefug

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.

EddyVerbruggen avatar Apr 09 '20 15:04 EddyVerbruggen

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?

paulgovers avatar Apr 10 '20 14:04 paulgovers

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

farfromrefug avatar Apr 10 '20 14:04 farfromrefug

@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?

farfromrefug avatar Apr 11 '20 07:04 farfromrefug

@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
        );
}

kefahB avatar Apr 20 '20 10:04 kefahB

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

farfromrefug avatar Apr 20 '20 10:04 farfromrefug