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

Editor: Fix "Attempt Recovery" error boundary handler #22396

Closed
wants to merge 2 commits into from

Conversation

aduth
Copy link
Member

@aduth aduth commented May 15, 2020

This pull request seeks to resolve an issue where "Attempt Recovery" (shown in the top-level error boundary) will never work correctly.

image

Status: In progress. The change in a27c055 fixes the issue. However, I'm not particularly confident this is the best solution.

The issue:

  • When "Attempt Recovery" is pressed, the editor is unmounted and then re-rendered in a "recovery" mode.
  • When rendered in recovery mode, EditorProvider skips most of its state initialization, since the state will have been preserved from before the error happens.
  • As of Editor: Implement meta as custom source #16402, an editor "tear down" mechanism was added to unset part of the editor state when the EditorProvider component unmounts (notably, the isReady state).
  • Thus, the editor will never render because it will be stuck in this ! isReady state and never initialize out of it (source)

The fix:

  • Expand ! isReady to ! isReady || recovery, such that the editor renders in recovery mode

Possible alternatives:

  • Get rid of isReady altogether
  • Don't unset isReady when performing editor teardown
  • Don't tear down any editor state on unmount
  • Tear down everything when unmounting editor, and always initialize state when mounting editor
  • Don't unmount editor when rebooting
  • When skipping initialization during reboot mount, also set isReady

@aduth aduth added [Type] Bug An existing feature does not function as intended [Package] Editor /packages/editor labels May 15, 2020
@github-actions
Copy link

Size Change: -152 B (0%)

Total Size: 833 kB

Filename Size Change
build/block-directory/index.js 6.59 kB +1 B
build/block-editor/index.js 105 kB -6 B (0%)
build/components/index.js 182 kB -1 B
build/data/index.js 8.42 kB -1 B
build/edit-post/index.js 28.1 kB -1 B
build/edit-site/index.js 12 kB -1 B
build/edit-widgets/index.js 7.87 kB -1 B
build/editor/index.js 44.2 kB -142 B (0%)
build/i18n/index.js 3.56 kB +1 B
build/shortcode/index.js 1.69 kB -1 B
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.02 kB 0 B
build/annotations/index.js 3.62 kB 0 B
build/api-fetch/index.js 3.39 kB 0 B
build/autop/index.js 2.83 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/style-rtl.css 764 B 0 B
build/block-directory/style.css 764 B 0 B
build/block-editor/style-rtl.css 10.8 kB 0 B
build/block-editor/style.css 10.8 kB 0 B
build/block-library/editor-rtl.css 7.19 kB 0 B
build/block-library/editor.css 7.19 kB 0 B
build/block-library/index.js 118 kB 0 B
build/block-library/style-rtl.css 7.48 kB 0 B
build/block-library/style.css 7.48 kB 0 B
build/block-library/theme-rtl.css 683 B 0 B
build/block-library/theme.css 685 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 48.1 kB 0 B
build/components/style-rtl.css 17.1 kB 0 B
build/components/style.css 17 kB 0 B
build/compose/index.js 6.67 kB 0 B
build/core-data/index.js 11.4 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/date/index.js 5.47 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 0 B
build/dom/index.js 3.11 kB 0 B
build/edit-navigation/index.js 5.77 kB 0 B
build/edit-navigation/style-rtl.css 709 B 0 B
build/edit-navigation/style.css 708 B 0 B
build/edit-post/style-rtl.css 12.2 kB 0 B
build/edit-post/style.css 12.2 kB 0 B
build/edit-site/style-rtl.css 5.22 kB 0 B
build/edit-site/style.css 5.22 kB 0 B
build/edit-widgets/style-rtl.css 4.69 kB 0 B
build/edit-widgets/style.css 4.69 kB 0 B
build/editor/editor-styles-rtl.css 425 B 0 B
build/editor/editor-styles.css 428 B 0 B
build/editor/style-rtl.css 5.07 kB 0 B
build/editor/style.css 5.08 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.63 kB 0 B
build/format-library/style-rtl.css 502 B 0 B
build/format-library/style.css 502 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 622 B 0 B
build/is-shallow-equal/index.js 711 B 0 B
build/keyboard-shortcuts/index.js 2.51 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.13 kB 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/media-utils/index.js 5.29 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 kB 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.5 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 14.8 kB 0 B
build/server-side-render/index.js 2.68 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.02 kB 0 B
build/viewport/index.js 1.84 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

Base automatically changed from master to trunk March 1, 2021 15:43
@aduth
Copy link
Member Author

aduth commented May 27, 2022

Closing as stale.

@aduth aduth closed this May 27, 2022
@aduth aduth deleted the fix/attempt-recovery branch May 27, 2022 22:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Editor /packages/editor [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant