[Live] Rendering any errors in a simple modal #467
Merged
Conversation
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
weaverryan
force-pushed
the
render-exceptions
branch
from
9c271b0 to
08e7ed7
Compare
weaverryan
force-pushed
the
render-exceptions
branch
from
08e7ed7 to
24bd3a9
Compare
This was referenced Sep 21, 2022
weaverryan
added a commit
that referenced
this pull request
Sep 27, 2022
…ued" changes (weaverryan, kbond) This PR was merged into the 2.x branch. Discussion ---------- [WIP] [Live] Make Ajax calls happen in serial, with "queued" changes | Q | A | ------------- | --- | Bug fix? | yes | New feature? | yes | Tickets | None | License | MIT Hi! Consider the following situation: A) Model update is made: Ajax call starts B) An action is triggered BEFORE the Ajax call from (A) finishes. Previously, we would start a 2nd Ajax call for (B) before the Ajax call from (A) finished... meaning two Ajax calls were happening at the same time. There are a few problems with this: (i) Ajax call (B) is missing any potential data changes from Ajax call (A) and (i) complexity of multiple things loading at once, and potentially finishing in a different order. This PR simplifies things, which matches Livewire's behavior anyways. Now, the action from (B) will WAIT until the Ajax call from (A) finishes and THEN start. In fact, while an Ajax call is being made, all changes (potentially multiple model updates or actions) will be "queued" and then all set at once on the next request. TODO: * [X] Update the backend: for POST requests, the "data" was moved under a `data` key in JSON. Previously the entire body was the "data". * [X] Update the backend: for POST/action requests, action "arguments" were moved from query parameters to an `args` key in the JSON. * [x] Frontend: add tests for the `batch` action Ajax calls * [x] Update the backend: a new fake `/batch` action needs to be added that can handle multiple actions at once * [ ] A new `updatedModels` is sent on the ajax requests. If the signature fails, use this to give a better error message about what readonly properties were just modified. (in fact, this is the only purpose of sending this new field to the backend at this time). * [X] ~~Pass a `includeUpdatedModels` value to the Stimulus controller ONLY when in `dev` mode.~~ For consistency, we will always pass the `updatedModels` in our Ajax requests, though this is intended to be "internal" and we won't use it other than to throw better errors. * [X] ~~(Optional) If the backend has an unexpected exception (i.e. 5xx or maybe some 4xx where we mark that this is a problem we should show the developer), then render the entire HTML exception page (in dev mode only) so the user can see the error.~~ Done in #467 Cheers! Commits ------- 4fbcebe cs fix and small tweaks a0c4d3f use `Request::toArray()` 13d7110 mildly reworking to follow the logic of what is happening more clearly (#5) b47ece1 [Live] add batch action controller 8b1e879 Refactoring towards single request & pending updates
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.
Beautiful error handling! Inspired by Livewire:
You can even
dd()from your code and that will render in the modal:If an Ajax call returns anything OTHER than a rendered component (i.e. there was SOME sort of error), it's rendered in a modal. This is mostly for development errors. The modal will show on production also (and would show, for example, the 500 production page), but that should not happen in normal situations.
Cheers!