Conversation
There was a problem hiding this comment.
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>| if (!this.#editorComponentsBuilt) { | ||
| this.createEditorComponents(); | ||
| this.#editorComponentsBuilt = true; |
There was a problem hiding this comment.
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.
| let transferContext: TransferContext | undefined; | ||
| const captureTransferContext = ({ | ||
| trigger, | ||
| data, | ||
| }: { | ||
| trigger: 'paste' | 'drop'; | ||
| data: DataTransfer | null; | ||
| }) => { | ||
| const context = { | ||
| trigger, | ||
| files: Boolean(data?.files.length), | ||
| }; | ||
| transferContext = context; |
There was a problem hiding this comment.
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.
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:
Bug Fixes:
Enhancements:
CI:
Tests: