Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transforms.setNodes unexpected behavior of children #5686

Open
electroluxcode opened this issue Jul 26, 2024 · 5 comments
Open

Transforms.setNodes unexpected behavior of children #5686

electroluxcode opened this issue Jul 26, 2024 · 5 comments
Labels

Comments

@electroluxcode
Copy link
Contributor

electroluxcode commented Jul 26, 2024

Description

it refer https://docs.slatejs.org/api/transforms#transforms.setnodes-editor-editor-props-partial-less-than-node-greater-than-options

Transforms.setNodes(editor: Editor, props: Partial<Node>, options?)

props: Partial<Node> has a prop named children ,However, when I read the source code in path packages\slate\src\interfaces\transforms\general.ts, I found that the props parameter named children did not render . There's this code

if (key === 'children' || key === 'text') {
  throw new Error(`Cannot set the "${key}" property of nodes! `)
}

this code makes it impossible for setNodes to handle nested cases with 'children'

Expectation

maybe setNodes should be able to handle nested cases with 'children'

Environment

  • Slate Version: 0.103
  • Operating System: window
  • Browser: Chrome

eg

for example

   const path = ReactEditor.findPath(editor, element)
      const newProperties = {
        "type": "one",
        "children": [
          {
            "type": "two",
            "children": [
              {
                "type": "three",
                children:[{
                  text:"shouldbe replace"
                }]
              }
            ]
          }
        ]
      } as any

      Transforms.setNodes(editor, newProperties, { at: path })

this code will only render first level and missing children

it seem deliberate in reading source,i have quesion why would do that

thanks

@electroluxcode electroluxcode changed the title 'Transforms.setNodes' unexpected behavior of children Transforms.setNodes unexpected behavior of children Jul 26, 2024
@WindRunnerMax
Copy link
Contributor

Adding nodes should not use setNodes, but rather APIs such as insertNodes

@electroluxcode
Copy link
Contributor Author

electroluxcode commented Jul 28, 2024

Adding nodes should not use setNodes, but rather APIs such as insertNodes

hi, @WindRunnerMax thanks for your reply
I understand your point; indeed, I can use the insertNode API. However, my requirement is to transform a section of nodes entirely into another node.

For example, in Markdown syntax, inputting - Your content automatically transforms into a ul (unordered list).

Currently, my workaround is to first use the removeNode API and then the insertNode API to replace setNodes, but this approach doesn't seem to be expectations and semantics.

for example,After removing and insert nodes, the cursor will be positioned on the next line. You need to manually control the cursor position by developer

@12joan
Copy link
Contributor

12joan commented Jul 30, 2024

@electroluxcode Changing the content using setNodes wouldn't solve the selection problem. This isn't supported by Slate for the same reason that Slate isn't a controlled component. I wrote the following essay explaining why.

#5281 (comment)

@electroluxcode
Copy link
Contributor Author

slate-diff can indeed solve the problem of setNodes. However, I think that slate should import the slate-diff library and use it as the logic of setNodes instead of making users perform additional operations.

@12joan
Copy link
Contributor

12joan commented Sep 14, 2024

slate-diff can indeed solve the problem of setNodes. However, I think that slate should import the slate-diff library and use it as the logic of setNodes instead of making users perform additional operations.

Unfortunately, that would severely increase Slate's bundle size and code complexity, and harm the predictability of its API. I don't think it's a good candidate for inclusion in core.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants