[Feature Request] Include `dompurify` as a regular NPM dependency

[jonkoops]

Context

• [x] This issue is not a bug report. (please use a different template for reporting a bug)
• [x] This issue is not a duplicate of an existing issue. (please use the search to find existing issues)

Description

I noticed that dompurify is included as a file copied from the VSCode repo. This makes it impossible to see what version of dompurify is depended on by monaco-editor, unless the code shipped in the NPM package is inspected. This especially troublesome when auditing the NPM dependencies for vulnerabilities, as inspecting the dependency graph will omit dompurify.

Would it be possible to include dompurify as a direct NPM dependency instead?

Monaco Editor Playground Link

No response

Monaco Editor Playground Code

[DCzajkowski]

The problematic line is:

https://github.com/ianstormtaylor/slate/blob/7a8ab18c527c97aebcafe5b88032da38aa1664b0/packages/slate-dom/src/plugin/dom-editor.ts#L433-L438

By skipping this if statement, the behavior is fixed.

Going back a step, is there really a reason to have such complicated code inside ReactEditor.focus()?

What are your thoughts @skogsmaskin?

[skogsmaskin]

@DCzajkowski - the reason for this complexity is that DOMEditor,focus blindly sets the focus on the selection it currently knows about. If the underlying editor is then is in the process of mutating the content and selection, DOMEditor might try to set a selection that no longer is valid, an throw unrecoverable errors like Error: Cannot resolve a DOM node from Slate node

I recommend to try to sync your changes before calling ReactEditor.focus(), with editor.onChange(), does that help?

I would also discourage calling ReactEditor.focus in the midst of a execution scope with withoutNormalizing, like you do in your example:

      Editor.withoutNormalizing(editor, () => {
        Transforms.removeNodes(editor, { at: path });
        Transforms.insertNodes(editor, { text: content }, { at: path });

        // This doesn't work (selection is put to the end of the editor):
        ReactEditor.focus(editor);
        // This works (selection is put where `Transforms.select` puts it):
        // document.querySelector('[data-slate-editor="true"]')!.focus();

        // This worked in 0.94.1 but doesn't work in 0.112.0:
        Transforms.select(editor, { path, offset: content.length });
      });
    },

Try instead:

      Editor.withoutNormalizing(editor, () => {
        Transforms.removeNodes(editor, { at: path });
        Transforms.insertNodes(editor, { text: content }, { at: path });
        Transforms.select(editor, { path, offset: content.length });
      });
      editor.onChange() // Sync changes
      ReactEditor.focus(editor); // Set the browser focus

[skogsmaskin]

@DCzajkowski - updated my reply with how I think your transformation code should rather be formulated. Can you try that?

[DCzajkowski]

Hi, thank you for your time! We really appreciate it.

We are planning to conduct more debugging to find the exact cause (in about a week, as we have other priorities right now). It seems like the problem is not in the focus itself, but the fact that focus now waits with setTimeout for 10ms.

During that time, Slate changes the selection to the end of the text node, seemingly ignoring the selection we've manually set.

Since the time I've created this issue, we've found a second scenario, where exactly the same code works fine. I'll make sure to update you with our examples and explanation once we have something to share.

[DCzajkowski]

We have recently upgraded to the newest slate version and everything seems to work fine. I don't know if this was fixed, but I don't have reproduction anymore, so I will close this issue :) Thanks again for your assistance @skogsmaskin