Some quarantine refactor - #441
Conversation
alexlafroscia
left a comment
alexlafroscia
left a comment
There was a problem hiding this comment.
Over-all, I think these changes look really good! I appreciate the formatting clean-up and like how you've broken things out into nice logical pieces.
Besides my suggestion about the types for the before and after hook, I think this would be a great addition!
| const combineOptions = (model: Model, payload: any, before: any, ajaxOptions: any) => { | ||
| const data = (before && before.call(model, payload)) || payload; | ||
| return assign(ajaxOptions || {}, { data }); | ||
| } | ||
|
|
||
| const handleResponse = (model: Model, response: JSONValue, after: any) => { | ||
| if (after && !model.isDestroyed) { | ||
| return after.call(model, response); | ||
| } | ||
|
|
||
| return response; | ||
| } |
There was a problem hiding this comment.
What do you think about using a type for before and after that makes it clear that they are functions that should expect Model as the value for this and response as an argument? I think that might make the type definitions a bit more accurate (and useful!)
There was a problem hiding this comment.
I will try to use that type, it will be much more readable
b8c46c8 to
efeacce
Compare
|
@alexlafroscia I've added a type for before/after functions - I'm not 100 percent sure about them, please review that and give me some advices if you see that something can be improved. Maybe I'd implement a more information about returning values from functions because so I do not have an idea what should be returned value from the function |
2 participants
Hi,
I've refactored base utils, extracted some functionalities to separated functions to make everything consistent and readable.
Thanks!