Skip to content
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

FIX widget errors caught by error boundary persists after fixing and rerendering #68

Merged
merged 2 commits into from
Jun 19, 2024

Conversation

Charlie-XIAO
Copy link
Contributor

@Charlie-XIAO Charlie-XIAO commented Jun 17, 2024

Reproducing the error on main

Try an arbitrary widget. Make an error in it that will be caught by the error boundary (for instance, using an undeclared variable in the react component which will give a ReferenceError). You will then see the error display on the canvas. Now fix that error and click re-render in the manager, the error does not go away.

Explanation of the fix

In each error boundary fallback we try to reset the error boundary. I'm not sure about the mechanism behind the issue, but the obvious workaround is to try triggering a re-render. The problem with directly calling resetErrorBoundary in the fall back is that it will go into an infinite re-rendering loop until the error in the widget gets fixed, because it will try resetting, then detects the error, and falls back, and try resetting again. To avoid this we use a flag variable for which the details are in the comments of the code.

Now with this PR try the reproduction steps again. Re-rendering after fixing the error should automatically update the canvas display and render the correct, working widget.

@Charlie-XIAO Charlie-XIAO added bug Something isn't working deskulpt:frontend labels Jun 17, 2024
Copy link

github-actions bot commented Jun 17, 2024

✔️ Deskulpt Built Successfully!

Deskulpt binaries have been built successfully on all supported platforms. Your pull request is in excellent shape! You may check the built Deskulpt binaries here and download them to test locally.

Workflow file: .github/workflows/build.yaml. Generated for commit: c3f18c2.

@Charlie-XIAO
Copy link
Contributor Author

Charlie-XIAO commented Jun 19, 2024

I'm going to merge without approval. This is also unlikely to have OS-specific issues. It is also critical to the developer experience in #66.

@Charlie-XIAO Charlie-XIAO merged commit aa2759e into main Jun 19, 2024
13 checks passed
@Charlie-XIAO Charlie-XIAO deleted the err-bound-rerender branch June 19, 2024 01:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working deskulpt:frontend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant