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

[Feature Request]: Sync empty text nodes #343

Open
productivityindustries opened this issue Apr 25, 2022 · 3 comments
Open

[Feature Request]: Sync empty text nodes #343

productivityindustries opened this issue Apr 25, 2022 · 3 comments
Labels
enhancement New feature or request question Further information is requested

Comments

@productivityindustries
Copy link

productivityindustries commented Apr 25, 2022

Hi, I wrote a simple unit test to validate the conversion from Slate nodes to Yjs shared objects and reverse and, to my surprise, it fails.

import { slateNodesToInsertDelta, yTextToSlateElement } from '@slate-yjs/core';
import { expect } from 'chai';
import { Node } from 'slate';
import * as Y from 'yjs';

describe('slate', () => {
  it('should correctly convert Slate nodes', () => {
    const nodes = [
      {
        children: [
          {
            text: 'Test:',
          },
        ],
        type: 'paragraph',
        id: 'ed99db3b-9dc4-47ad-b8b7-4bfb44e3cd1e',
      },
      {
        children: [
          {
            text: '',
          },
          {
            children: [
              {
                text: '',
                hyperlink: {
                  url: 'https://www.example.com',
                },
                underlined: true,
                fontColor: '#0000ff',
              },
            ],
            type: 'inline-image',
            transform: {
              angle: 0,
              width: '134px',
              height: '50px',
            },
            blobID: '2694b3a2-afae-4fcc-8a60-44334bc3a04b',
            blobURL: 'https://www.example.com/image.png',
            blobFormats: ['original', 'dataUrl'],
          },
          {
            text: '',
          },
        ],
        type: 'paragraph',
        id: 'ca804088-30ff-4f51-b8ea-4f99c846fa0e',
      },
      {
        children: [
          {
            text: '',
          },
        ],
        type: 'paragraph',
        id: '53bf071f-4929-4c8c-94a4-cebd33c48ddc',
      },
      {
        children: [
          {
            text: '',
          },
        ],
        type: 'paragraph',
        id: '9326fbf8-f41c-44c3-9954-c06b0ccfbc30',
      },
    ];
    const ydoc = new Y.Doc();
    const delta = slateNodesToInsertDelta(nodes);
    const xmlText = ydoc.get('test', Y.XmlText) as Y.XmlText;
    xmlText.applyDelta(delta, { sanitize: false });
    const node = yTextToSlateElement(xmlText);
    expect(node.children).to.deep.equal(nodes);
  });
});

Test output:

  slate
    1) should correctly convert Slate nodes


  0 passing (31ms)
  1 failing

  1) slate
       should correctly convert Slate nodes:

      AssertionError: expected [ { type: 'paragraph', …(2) }, …(3) ] to deeply equal [ { …(3) }, …(3) ]
      + expected - actual

         }
         {
           "children": [
             {
      +        "text": ""
      +      }
      +      {
               "blobFormats": [
                 "original"
                 "dataUrl"
               ]
               "blobID": "2694b3a2-afae-4fcc-8a60-44334bc3a04b"
               "blobURL": "https://www.example.com/image.png"
               "children": [
                 {
      +            "fontColor": "#0000ff"
      +            "hyperlink": {
      +              "url": "https://www.example.com"
      +            }
                   "text": ""
      +            "underlined": true
                 }
               ]
               "transform": {
                 "angle": 0
--
                 "width": "134px"
               }
               "type": "inline-image"
             }
      +      {
      +        "text": ""
      +      }
           ]
           "id": "ca804088-30ff-4f51-b8ea-4f99c846fa0e"
           "type": "paragraph"
         }

I took as an example a document created in our webapp with an inline image (Slate automatically surrounds inline elements with empty texts) with an hyperlink attached (an empty text with attributes is perfectly valid in Slate).
Digging into the code, I realized the issue root cause is in the Yjs business logic skipping any insertion of empty strings.
This misalignment breaks synchronization between clients, and also the storing of document data on the server side.
The bridge between Slate and Yjs should be able to fully reconstruct the original node structure, as far as it complies with the Slate model.

@BitPhinix
Copy link
Owner

Hi @productivityindustries,

This isn't a bug - it's an intentional tradeoff of the representation of the slate document inside yjs. Slate-yjs relies on slates default normalization rules to be run after applying a remote change, which will add the empty text elements back in, causing the remote and local state to match (except for formatting attributes on blank text nodes). Appling changes with mismatches in the empty text nodes won't cause issues.

While we could sync the formatting attributes of empty text nodes using yjs embeds, I intentionally decided not to support it for now, as slate treats them as disposable anyway (e.g., it will merge them, ignoring the formatting attributes). So storing any critical information on them should be avoided. Instead, you could probably add back the URL attribute using a normalization rule in your example, although it's hard to say without knowing the specifics.

@BitPhinix BitPhinix added the enhancement New feature or request label Apr 25, 2022
@BitPhinix BitPhinix changed the title Slate nodes conversion not fully supported [Feature Request]: Sync empty text nodes Apr 25, 2022
@BitPhinix BitPhinix added the question Further information is requested label Apr 25, 2022
@eagowang
Copy link

Hi @productivityindustries,
I have the same problem, because the empty text not sync, the doc init but link(inline element) not surround with empty text node after refresh browser. Did you solve the problem?

@eagowang
Copy link

Hi @productivityindustries, I have the same problem, because the empty text not sync, the doc init but link(inline element) not surround with empty text node after refresh browser. Did you solve the problem?

Hi @productivityindustries @BitPhinix,
Call Editor.normalize(editor, {force: true}) when first applyRemoteEvents can solve this problem, but I don't know if there other problems

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants