Skip to content

refactor: share lazily initialized editor dependencies - #1268

Open
Maxxich wants to merge 1 commit into
mainfrom
refactor/shared-editor-dependencies
Open

Maxxich wants to merge 1 commit into
mainfrom
refactor/shared-editor-dependencies

Conversation

@Maxxich

@Maxxich Maxxich commented Sep 24, 2026 •

Copy link
Copy Markdown

Prepare schema, Markdown parsers, serializer and actions once, independently of
ProseMirror view components. Extract the common configuration factory and let
WysiwygEditor consume an already prepared manager without mutating its plugin
array. This supports parsing before a view exists while retaining the existing
editor construction path.

Summary by Sourcery

Share lazily prepared editor dependencies across views while keeping view-specific components isolated.

New Features:

  • Add a reusable editor extensions manager that prepares schema, Markdown parsers, serializers, and actions independently of an editor view.
  • Allow editor dependencies to be built and used before creating a WysiwygEditor view.

Bug Fixes:

  • Prevent shared plugin arrays from accumulating view-specific plugins when creating multiple editors.

Enhancements:

  • Make dependency and editor-component construction lazy and idempotent while preserving the existing editor construction path.

Tests:

  • Add coverage for lazy dependency preparation, manager reuse, parser configuration, and plugin-array stability.

@gravity-ui

gravity-ui Bot commented Sep 24, 2026

Copy link
Copy Markdown

Storybook Deployed

@gravity-ui

gravity-ui Bot commented Sep 24, 2026

Copy link
Copy Markdown

🎭 Playwright Report

@Maxxich
Maxxich marked this pull request as ready for review September 24, 2026 10:48

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="packages/editor/src/core/Editor.ts" line_range="112" />
<code_context>
-
-        plugins.unshift(LoggerFacet.of(logger));
-        plugins.unshift(ParserFacet.of(parser));
+        } = manager.build();

         const state = EditorState.create({
</code_context>
<issue_to_address>
**issue (broader_impact):** `manager.build()` returns a shared `ActionsManager` when multiple `WysiwygEditor` instances reuse the same `extensionsManager`; each constructor then calls `actions.setActions(...)`, overwriting the action bindings for every earlier editor. Actions invoked through an earlier editor therefore dispatch against the most recently constructed editor view.

**Triggers:** When two or more WysiwygEditor instances are created with the same extensionsManager.

**Suggested fix:** Keep action bindings per WysiwygEditor, or create a fresh ActionsManager/action map for each view while sharing only the immutable dependency data.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨


plugins.unshift(LoggerFacet.of(logger));
plugins.unshift(ParserFacet.of(parser));
} = manager.build();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (broader_impact): manager.build() returns a shared ActionsManager when multiple WysiwygEditor instances reuse the same extensionsManager; each constructor then calls actions.setActions(...), overwriting the action bindings for every earlier editor. Actions invoked through an earlier editor therefore dispatch against the most recently constructed editor view.

Triggers: When two or more WysiwygEditor instances are created with the same extensionsManager.

Suggested fix: Keep action bindings per WysiwygEditor, or create a fresh ActionsManager/action map for each view while sharing only the immutable dependency data.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant