Fix: ensure tools are configured before editor renders on note switch - #355
Fix: ensure tools are configured before editor renders on note switch#3557eliassen wants to merge 5 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR addresses an intermittent Editor.js rendering issue when switching between notes by ensuring the editor does not render until tool scripts/configuration have been rebuilt for the newly selected note.
Changes:
- Reset
isEditorReadyandtoolsUserConfigLoadedbefore re-loading tool scripts onnoteAndUserToolschanges. - Ensure the editor component remounts only after tools are configured, preventing blocks from rendering with stale/missing tool configuration.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| isEditorReady.value = false; | ||
| toolsUserConfigLoaded.value = false; | ||
|
|
||
| await loadToolsScripts(tools); |
| isEditorReady.value = false; | ||
| toolsUserConfigLoaded.value = false; | ||
|
|
||
| const loadedConfig = await loadToolsScripts(tools); |
There was a problem hiding this comment.
loadToolsScripts can throw. Since isEditorReady is now reset to false before this await, a failed download leaves the note stuck with no editor rendered at all.
Might be worth wrapping this in a try/catch and surfacing an error on failure? Or maybe restoring some usable state?
| * If a newer load request has superseded this one — discard stale results | ||
| * to prevent overwriting state with tools from a previous note. | ||
| */ | ||
| if (loadId !== currentLoadId) { |
There was a problem hiding this comment.
Could you add test case so the race condition this is meant to fix doesn't regress silently?
| await loadToolsScripts(tools); | ||
| const loadId = ++currentLoadId; | ||
|
|
||
| isEditorReady.value = false; |
There was a problem hiding this comment.
isEditorReady now resets on every noteAndUserTools recompute, not just on note switch — e.g. installing a tool from the marketplace live-updates the shared userEditorTools ref while a note is open. Worth checking that case: does <Editor> visibly flash/remount instead of just picking up the new tool like before?
| * @returns loaded tools config | ||
| */ | ||
| async function loadToolsScripts(toolsConfigs: EditorTool[]): Promise<void> { | ||
| async function loadToolsScripts(toolsConfigs: EditorTool[]): Promise<Record<string, { class: EditorjsConfigTool; inlineToolbar: boolean }>> { |
There was a problem hiding this comment.
Might be worth extracting a shared type alias so loadToolsScripts and toolsUserConfig can't drift apart?
Problem:
Root Cause:
This issue occurs because the toolsUserConfigLoaded and isEditorReady flags are not properly reset. As a result, the editor may render before the tools are fully configured, causing certain blocks to fail to display.
What done: