Skip to content

feat: support resource replacement in ProseMirror - #1270

Open
Maxxich wants to merge 1 commit into
feat/resource-replacement-corefrom
feat/resource-replacement-prosemirror
Open

Maxxich wants to merge 1 commit into
feat/resource-replacement-corefrom
feat/resource-replacement-prosemirror

Conversation

@Maxxich

@Maxxich Maxxich commented Sep 24, 2026 •

Copy link
Copy Markdown

Depends on PR: feat: add the resource replacement controller and schema contract

Collect configured resources from accepted paste/drop transactions after supported
normalization, then apply validated replacements to current document matches.
Track local history locks and require synchronous service dispatch and cleanup.
Preserve remote transaction handling and binary-upload behavior. Include PM adapter.

Summary by Sourcery

Support validated asynchronous replacement of resources inserted through ProseMirror paste and drop transactions while preserving editor history, collaboration, and upload behavior.

New Features:

  • Add ProseMirror resource replacement for configured resources inserted through supported paste and drop workflows.
  • Expose a ProseMirror resource extension and controller lifecycle for resolving, validating, applying, cancelling, and timing out replacements.

Bug Fixes:

  • Preserve resource replacement correctness across normalization, parent reconstruction, deletion, movement, undo/redo, remote transactions, and synchronous service callbacks.
  • Prevent invalid or stale replacements, cascading updates, duplicate requests, and late callbacks from mutating editor state.

Enhancements:

  • Share lazily built editor dependencies across views through reusable extension managers.
  • Lock local history while resource replacement operations are pending while preserving remote transaction handling.
  • Add resource schema metadata, URL handling, trigger detection, and comprehensive controller and ProseMirror integration coverage.

CI:

  • Adjust Vitest module resolution to use compatible ESM ProseMirror builds in tests.

Tests:

  • Add extensive tests covering resource collection, replacement validation, controller lifecycle, transaction acceptance, history behavior, normalization, cleanup contracts, and regressions.

@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:49

@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 2 issues

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

## Individual Comments

### Comment 1
<location path="packages/editor/src/core/ExtensionsManager.ts" line_range="107-109" />
<code_context>
-        this.createDeps();
-        this.createDerived();
+        this.buildDeps();
+        if (!this.#editorComponentsBuilt) {
+            this.createEditorComponents();
+            this.#editorComponentsBuilt = true;
+        }

</code_context>
<issue_to_address>
**issue (broader_impact):** Reusing one `ExtensionsManager` across editor views also reuses the same `ActionsManager`; each `WysiwygEditor` calls `actions.setActions(...)` for its own view, so the last-created editor overwrites the actions used by earlier editors.

**Triggers:** When two `WysiwygEditor` instances share an `extensionsManager` and an action is invoked through the first editor.

**Suggested fix:** Keep actions view-specific, for example by creating a per-view `ActionsManager` or by binding actions without storing them in the shared dependencies.
</issue_to_address>

### Comment 2
<location path="packages/editor/src/extensions/behavior/ResourceReplacement/source.ts" line_range="20-32" />
<code_context>
+    createPlugin: () => Plugin;
+    shouldProcessTransaction: ResourceReplacementOptions['shouldProcessTransaction'];
+} {
+    let transferContext: TransferContext | undefined;
+    const captureTransferContext = ({
+        trigger,
+        data,
+    }: {
+        trigger: 'paste' | 'drop';
+        data: DataTransfer | null;
+    }) => {
+        const context = {
+            trigger,
+            files: Boolean(data?.files.length),
+        };
+        transferContext = context;
+        queueMicrotask(() => {
+            if (transferContext === context) transferContext = undefined;
</code_context>
<issue_to_address>
**issue (bug_risk):** The transfer context is stored in the plugin factory closure rather than per editor view, so a paste or drop event in one view can be consumed by the next matching transaction from another view sharing the cached plugin instance; that transaction is then incorrectly marked as eligible or incorrectly excluded for resource replacement.

**Triggers:** When multiple editor views share the lazily cached editor components and a second view dispatches a paste/drop transaction before the first view's microtask clears the context.

**Suggested fix:** Store transfer context in per-view plugin state or create a separate source plugin instance for each editor view.
</issue_to_address>

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

Comment on lines +107 to +109
if (!this.#editorComponentsBuilt) {
this.createEditorComponents();
this.#editorComponentsBuilt = true;

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): Reusing one ExtensionsManager across editor views also reuses the same ActionsManager; each WysiwygEditor calls actions.setActions(...) for its own view, so the last-created editor overwrites the actions used by earlier editors.

Triggers: When two WysiwygEditor instances share an extensionsManager and an action is invoked through the first editor.

Suggested fix: Keep actions view-specific, for example by creating a per-view ActionsManager or by binding actions without storing them in the shared dependencies.

Comment on lines +20 to +32
let transferContext: TransferContext | undefined;
const captureTransferContext = ({
trigger,
data,
}: {
trigger: 'paste' | 'drop';
data: DataTransfer | null;
}) => {
const context = {
trigger,
files: Boolean(data?.files.length),
};
transferContext = context;

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 (bug_risk): The transfer context is stored in the plugin factory closure rather than per editor view, so a paste or drop event in one view can be consumed by the next matching transaction from another view sharing the cached plugin instance; that transaction is then incorrectly marked as eligible or incorrectly excluded for resource replacement.

Triggers: When multiple editor views share the lazily cached editor components and a second view dispatches a paste/drop transaction before the first view's microtask clears the context.

Suggested fix: Store transfer context in per-view plugin state or create a separate source plugin instance for each editor view.

@Maxxich
Maxxich changed the base branch from main to feat/resource-replacement-core October 5, 2026 09:06

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