-
Notifications
You must be signed in to change notification settings - Fork 1
chore: upgrade dependencies; remove freezed #19
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
Conversation
samandmoore
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
domainlgtm
just a few questions about stuff that we might not need anymore as well
|
do we need to update the readme too? any examples? |
| onResponse: onResponse, | ||
| ); | ||
| } on Exception catch (e) { | ||
| if (e is CheckedFromJsonException) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: This is a behavior change. Previously, we'd only emit a DecodingError event if the Exception was a CheckedFromJsonException. That felt a bit smelly to me (it relies on the consumer's decoding implementation. I.e. if they used dart_mappable, a MapperException is thrown instead, and this check wouldn't ever emit). Now, we emit this event on every Exception thrown during deserialization. This encourages two things:
- Keeping
onResponselogic focused on just deserialization to minimize the chance of anExceptionbeing thrown that's not related to deserialization - Allowing consumers themselves to decide if they "react" to a
DecodingError(they can check if the passedExceptionwas indeed a variant they care about).
samandmoore
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
domainlgtm
the exception event emitting behavior change makes sense to me too.
nice clean up!
|
/no-platform |
📰 Summary of changes
This PR does the following (and includes⚠️ Breaking changes ⚠️ ):
freezedandfreezed_annotationdependencies which were previously used only to give us sealed-like classes. Sincesealed classbecame a thing after this library was designed, we can drop that and usedsealeddirectly. Note that the public API/constructors of these types (NetworkRequestBodyandSturdyHttpEvent) has changed and will require adjustments (which should be doable via Find All + Replace).DecodingErrors are emitted. See the comment below.🧪 Testing done
Unit test coverage feels sufficient here.