Fix for random UI crash #2840
Merged
Fix for random UI crash #2840
Conversation
This file contains 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
Closed
develohpanda
suggested changes
Nov 16, 2020
There was a problem hiding this comment.
Thanks for investigating a submitting a PR! 🎉
I think that instead of throwing an error, we simply return '\u2194' (the current fallback, see line 330) if either from or to are falsey, and allow CodeMirror to handle it internally. The event listener would likely be created regardless, because this code doesn't actually do any folding, it only provides the text content. There will be an arrow in the gutter (see below) to fold/unfold regardless.
develohpanda
suggested changes
Nov 16, 2020
packages/insomnia-app/app/ui/components/codemirror/code-editor.js
Outdated
Show resolved
Hide resolved
packages/insomnia-app/app/ui/components/codemirror/code-editor.js
Outdated
Show resolved
Hide resolved
develohpanda
approved these changes
Nov 17, 2020
DMarby
approved these changes
Nov 20, 2020
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.
After some analysis i found out the main problem it's related to the
from.linewhich has anundefinedvalue in some situations (which i was not able to reproduce).foldOptions.widget(from, to)is called internally by CodeMirror when you want to fold code inside the editor itself. This makes React crash when the "Send" button is pressed because the CodeMirror state is not consistent due to invalid folding triggering an infinite setState.I was thinking about simply return and do not execute any folding on the editor but it will create another inconsistency due to CodeMirror attaching an eventListener on an invalid point. So maybe throwing an error could be the best option.
At the least CodeMirror won't make the app crash when it's not able to retrieve from which line the folding happens and won't attach eventListener on invalid point.
This my proposal for a fix, it closes #2805
Any feedback?