Draft for a new ajax API built on top of fetch()#4585
Merged
Conversation
TimWolla
reviewed
Nov 15, 2021
TimWolla
reviewed
Nov 16, 2021
TimWolla
approved these changes
Nov 16, 2021
Member
TimWolla
left a comment
There was a problem hiding this comment.
The public API of this module LGTM now. The remaining comments are just about “code style” within this module (and missing doc comments).
Member
Author
|
@TimWolla Please verify the latest changes and compare it against the documentation in case there are any discrepancies. |
Member
LGTM. I tested the first example in the documentation (against UserProfileAction::getUserProfile()) with the following HTML: |
TimWolla
approved these changes
Nov 17, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The design of the existing AJAX API is quite old and dates back to the behavior of
WCF.Action.Proxy. It lacks any support for asynchronous actions (Promises) and has a subpar control flow caused by the different methods to prepare (_ajaxSetup()), invoke and evaluate the result (_ajaxSuccess()and_ajaxFailure()). Especially modules serving multiple endpoints suffer from_ajaxSuccess()being a glorifiedswitchto keep the code readable. The lack of proper types for the request and response make it hard to work with it.At some point I have evaluated Axios as an alternative, but at the end of the day decided against it. It adds a lot of bells and whistles, but its major selling point are the compatibility with ancient browsers as well as some nice helper features. We already maintain a working implementation on top of
XMLHttpRequest, so we are familiar with the requirements and have proven to be able to maintain our own wrapper around it. From my perspective, Axios is a nice library, but adds another dependency that we do not strictly need.Usage of the library in this draft: