chore: update AliceChopperInterceptor to be compatible with Chopper v8.0.0
#180
Conversation
|
|
||
| String? contentType = 'unknown'; | ||
| if (request.headers.containsKey('Content-Type')) { | ||
| contentType = request.headers['Content-Type']; | ||
| if (chain.request.headers.containsKey('Content-Type')) { |
There was a problem hiding this comment.
Content-Type string could be const I guess and would case insensitive be needed?
There was a problem hiding this comment.
Yeah, not sure. For that I'd need to import dart:io, I think. Is this package supposed to work on web?
There was a problem hiding this comment.
Well it feels like something that could be shared between the different clients (dio, http, chopper). I suppose they all have this logic. Of course with web support in mind.
There was a problem hiding this comment.
Doesn't look like this package has web in mind, so I'll just use the constants from dart:io.
|
|
||
| /// Adds data to existing alice http call | ||
| aliceCore.addResponse( |
There was a problem hiding this comment.
Not sure but I don't think its needed to add the Response afterwards. My guess is the hashcode and this function where used because the previous request and response callback we're separate. But with the new Interceptors this in not the case any more. So you can proceed the request and attach the response directly to the same AliceCall.
I also think changing this will remove the need for adding a alice_token header.
There was a problem hiding this comment.
I moved the AliceHttpResponse to the AliceHttpCall, however, I kept the alice_token cause I'm not 100% sure if it's needed for any other purpose. Maybe @jhomlala can give some more info here?
|
Hi everyone! I've just released new version of Alice which splits clients into separate packages. There's new alice + chopper package here: https://github.com/jhomlala/alice/tree/master/packages/alice_chopper . I've updated chopper to 8.0.0 in this package. Can you verify if we are aligned with our changes? |
Very nice improvement of splitting into different packages ❤️. Looking at the interceptor code I think the implementation of @techouse is a bit cleaner. So I would suggest aligning these changes 😄 Also we still have an open question about the Thanks for all your work! |
Ha, looks like I jumped the gun on this one 😂 I'll check out the changes in the evening. |
Haha, nah it just moved somewhere else 😉 |
# Conflicts: # packages/alice/lib/alice.dart # packages/alice_chopper/lib/alice_chopper_response_interceptor.dart # pubspec.lock # pubspec.yaml
|
This PR is a mess because of the recent changes. I have created #185 to supersede it. |
|
Thanks! |
Hey 👋
Recently I stumbled upon this awesome package and I noticed it (also) uses Chopper Interceptors.
In Chopper v8.0.0, there was a significant restructuring of interceptors. The
RequestInterceptorandResponseInterceptorclasses, as well as their function counterparts, were removed.Since I'm one of the maintainers of Chopper I thought I'd contribute to this library by updating the
AliceChopperInterceptorto be compatible with Chopper v8.0.0.CC / @Guldem