ianstormtaylor / slate

A completely customizable framework for building rich text editors. (Currently in beta.)
http://slatejs.org
MIT License
29.33k stars 3.22k forks source link

Select all and paste html error all the time: Uncaught TypeError: Cannot read properties of null (reading 'length') #4857

Open stevemarksd opened 2 years ago

stevemarksd commented 2 years ago

Description

Error on select all and paste.

Recording

See steps below. Easy to reproduce in the demo.

Sandbox

https://www.slatejs.org/examples/paste-html

Steps

Go to: https://www.slatejs.org/examples/paste-html Copy h1 from https://www.nytimes.com/wirecutter/reviews/best-oven-mitts-and-pot-holders/ Go to editor, select all and paste.

It will work but with an error:

image

You can try to copy this part only:

image

Step

Another error happens if you do it here: https://www.slatejs.org/examples/richtext

Expectation

No error.

Environment

Same as demo and latest too. https://www.slatejs.org/examples/paste-html

Context

I wil try to investigate, but I still dont fully understand the path transforms in slatejs.

stevemarksd commented 2 years ago

Interesting, I kind of found a fix by cleaning the result this way:

      const fragmentsRaw = deserialize(parsed.body);
      if(!Array.isArray(fragmentsRaw)){
        throw new Error(`Unexepected fragmentsRaw is not an array`)
      }
      const fragmentsCleaned = fragmentsRaw.filter((_fragment, index) => {
        if (index === 0) return false;
        if (index === fragmentsRaw.length - 1) return false;
        return true;
      })
      Transforms.insertFragment(editor, fragmentsCleaned)

Basically I noticed the first and last item from fragmentsRaw are just \n and \n\n respectively, no matter where you copy and paste from. Removing these 2 removes the errors I was seeing above. I wouldn't mark it as solved as it cannot handle \n text.

To recap:

Doesn't work and errors:

[
    {
        "text": "\n"
    },
    {
        "type": "heading",
        "level": 2,
        "children": [
            {
                "text": "On the server"
            }
        ]
    },
    {
        "type": "paragraph",
        "children": [
            {
                "text": "You can still use hyperscript on the server, the limitation is that events don't make sense anymore, but you can use it to generate html:"
            }
        ]
    },
    {
        "text": "\n\n"
    }
]

Works:

[
    {
        "type": "heading",
        "level": 2,
        "children": [
            {
                "text": "On the server"
            }
        ]
    },
    {
        "type": "paragraph",
        "children": [
            {
                "text": "You can still use hyperscript on the server, the limitation is that events don't make sense anymore, but you can use it to generate html:"
            }
        ]
    }
]
stevemarksd commented 2 years ago

After further tweaking I noticed that in some cases the above fix is not good enough. \n gets prepended to the text elements so I came up with this fix instead:


export const deserialize = (el: ChildNode): DeserializeType => {
  if (el.nodeType === 3) {
    if(el.textContent === `\n`) return null
    if(el.textContent === `\n\n`) return null
    return el.textContent;
  } else if (el.nodeType !== 1) {
    return null;
  } else if (el.nodeName === 'BR') {
    return '\n';
  }

and then on insert data:


  editor.insertData = data => {
    const html = data.getData('text/html')

    if (html) {
      const parsed = new DOMParser().parseFromString(html, 'text/html')
      const fragmentsRaw = deserialize(parsed.body) as Array<SlateDescendant>;
      if (!Array.isArray(fragmentsRaw)) {
        throw new Error(`Unexpected fragmentsRaw is not an array`)
      }
      const fragmentsCleaned = fragmentsRaw.filter((slateNode) => {
        if ('text' in slateNode && (slateNode.text === `\n` || slateNode.text === `\n\n`)) return false;
        return true;
      })
      Transforms.insertFragment(editor, fragmentsCleaned)
      return
    }

    insertData(data)
  }

This fixes most of the issues.

Now another problem is pasting h1 + a bit of text.

image

Gets converted into

[
    {
        "type": "heading",
        "level": 1,
        "children": [
            {
                "text": "The Best Oven Mitts and Pot Holders"
            }
        ]
    },
    {
        "text": "B"
    }
]

And then errors here because path is null

image

Debugging up I see:

endRef === {
    "current": null,
    "affinity": "forward"
}

I guess the bug might be in

   // isInlineEnd === true
   // inlinePath ===[0,0]
   var endRef = Editor.pathRef(editor, isInlineEnd ? Path.next(inlinePath) : inlinePath);

But I can't tell exactly why it breaks.

Looking at next(path):

  next(path) {
    if (path.length === 0) {
      throw new Error("Cannot get the next path of a root path [".concat(path, "], because it has no next index."));
    }

    var last = path[path.length - 1];
    return path.slice(0, -1).concat(last + 1);
  },

So looks like between this code below the endRef.current becomes null from [0,0], so it's being mutated, not sure if it's expected or not.

image

It's really unfortunate that many operations end up in Uncaught TypeError: Cannot read properties of null (reading 'length') and is very often. Like pasting an image in the middle of the text or similar stuff. I wonder how to use this in production with so many errors, or maybe is just with my code.. not sure yet.


Another bug is when I copy and paste an image block, it pastes the image block as html.

The tutorial could add some code like if (!editor.insertFragmentData(data)) { to still enable fragment copied from slate editor.


  editor.insertData = data => {
    if (!editor.insertFragmentData(data)) {
      // editor.insertTextData(data);

      const html = data.getData('text/html')

      if (true) {
        const parsed = new DOMParser().parseFromString(html, 'text/html')
        const fragmentsRaw = deserialize(parsed.body) as Array<SlateDescendant>;
        if (!Array.isArray(fragmentsRaw)) {
          throw new Error(`Unexpected fragmentsRaw is not an array`)
        }
        const fragmentsCleaned = fragmentsRaw.filter((slateNode) => {
          if ('text' in slateNode && (slateNode.text === `` || slateNode.text === `\n` || slateNode.text === `\n\n`)) return false;
          return true;
        })
        Transforms.insertFragment(editor, fragmentsCleaned)
        return
      }
    }
  }
stevemarksd commented 2 years ago

The final code for insertData

editor.insertData = data => {
    if (!editor.insertFragmentData(data)) {
      const html = data.getData('text/html')

      if (true) {
        const parsed = new DOMParser().parseFromString(html, 'text/html')
        const fragmentsRaw = deserialize(parsed.body) as Array<SlateDescendant>;
        if (!Array.isArray(fragmentsRaw)) {
          throw new Error(`Unexpected fragmentsRaw is not an array`)
        }
        const fragmentsCleaned = fragmentsRaw.filter((slateNode) => {
          if ('text' in slateNode && (slateNode.text === `` || slateNode.text === `\n` || slateNode.text === `\n\n`)) return false;
          return true;
        })
        try {
          Transforms.insertFragment(editor, fragmentsCleaned)
        } catch (e) {
          console.log(`Error inserting data`, e)
        }
        return
      }
    }
  }

Looks like if you ignore the error from Transforms.insertFragment(editor, fragmentsCleaned) things still works just ok. Looks like only the cursor goes to the wrong location.

dylans commented 2 years ago

I think this was fixed in #4858 by reverting #4804. Reverting #4804 isn't ideal as it was added for a reason. Let us know if you think your proposed solution is still worth considering.

robinjoseph08 commented 2 years ago

I'm not sure if it was fixed in #4858. Based on the released, it looks like #4858 is included in slate@0.73.1, but I'm running 0.73.1 and am still seeing the error. Weirdly, I'm seeing this error in production caused by end users (being reporting in Sentry), but even with the same fragment and editor, I can't seem to reproduce it locally.