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

fix: unsupported node mark #135

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 56 additions & 7 deletions src/plugins/sync-plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import * as random from 'lib0/random'
import * as environment from 'lib0/environment'
import * as dom from 'lib0/dom'
import * as eventloop from 'lib0/eventloop'
import * as f from 'lib0/function'

/**
* @param {Y.Item} item
Expand Down Expand Up @@ -671,7 +672,19 @@ const createNodeFromYElement = (
: { type: 'added' }
}
}
const node = schema.node(el.nodeName, attrs, children)
const nodeAttrs = {};
const nodeMarks = [];

for (const key in attrs) {
if (key.startsWith('yelement_mark_')) {
const markName = key.replace('yelement_mark_','');
let markValue = attrs[key];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be const

nodeMarks.push(schema.mark(markName, markValue.attrs))
} else {
nodeAttrs[key] = attrs[key]
}
}
const node = schema.node(el.nodeName, nodeAttrs, children, nodeMarks);
mapping.set(el, node)
return node
} catch (e) {
Expand Down Expand Up @@ -750,12 +763,16 @@ const createTypeFromTextNodes = (nodes, mapping) => {
*/
const createTypeFromElementNode = (node, mapping) => {
const type = new Y.XmlElement(node.type.name)
const nodeMarksAttr = nodeMarksToAttributes(node.marks);
for (const key in node.attrs) {
const val = node.attrs[key]
if (val !== null && key !== 'ychange') {
type.setAttribute(key, val)
}
}
for (const key in nodeMarksAttr) {
type.setAttribute(key, nodeMarksAttr[key])
}
type.insert(
0,
normalizePNodeContent(node).map((n) =>
Expand All @@ -780,7 +797,7 @@ const createTypeFromTextOrElementNode = (node, mapping) =>
const isObject = (val) => typeof val === 'object' && val !== null

const equalAttrs = (pattrs, yattrs) => {
const keys = Object.keys(pattrs).filter((key) => pattrs[key] !== null)
const keys = Object.keys(pattrs).filter((key) => pattrs[key] !== null && !key.startsWith('yelement_mark_'))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you filtering yelement_mark_ here in the attributes that you get from the prosemirror node? If there is a reason, why are you not filtering in the next line when you are comparing?

let eq =
keys.length ===
Object.keys(yattrs).filter((key) => yattrs[key] !== null).length
Expand All @@ -794,6 +811,23 @@ const equalAttrs = (pattrs, yattrs) => {
return eq
}

const equalMarks = (yattrs, pmarks) => {
const keys = Object.keys(yattrs).filter((key) => key.startsWith('yelement_mark_'));

let eq =
keys.length === pmarks.length;

let pMarkAttr = nodeMarksToAttributes(pmarks);

for (let i = 0; i < keys.length && eq; i++) {
const key = keys[i];
const l = pMarkAttr[key]
const r = yattrs[key]
eq = key === 'ychange' || f.equalityDeep(l,r);
}
return eq
}

/**
* @typedef {Array<Array<PModel.Node>|PModel.Node>} NormalizedPNodeContent
*/
Expand Down Expand Up @@ -849,6 +883,7 @@ const equalYTypePNode = (ytype, pnode) => {
const normalizedContent = normalizePNodeContent(pnode)
return ytype._length === normalizedContent.length &&
equalAttrs(ytype.getAttributes(), pnode.attrs) &&
equalMarks(ytype.getAttributes(), pnode.marks) &&
ytype.toArray().every((ychild, i) =>
equalYTypePNode(ychild, normalizedContent[i])
)
Expand Down Expand Up @@ -965,6 +1000,17 @@ const marksToAttributes = (marks) => {
return pattrs
}

const nodeMarksToAttributes = (marks) => {
const pattrs = {}
const prefix = 'yelement_mark_'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's store this prefix globally somewhere as it is reused throughout the file. Also I suggest we use a shorter name that starts with an _. E.g. _mark_.

marks.forEach((mark) => {
if (mark.type.name !== 'ychange') {
pattrs[`${prefix}${mark.type.name}`] = mark.toJSON()
}
})
return pattrs
}

/**
* @private
* @param {{transact: Function}} y
Expand All @@ -984,18 +1030,21 @@ export const updateYFragment = (y, yDomFragment, pNode, mapping) => {
if (yDomFragment instanceof Y.XmlElement) {
const yDomAttrs = yDomFragment.getAttributes()
const pAttrs = pNode.attrs
for (const key in pAttrs) {
if (pAttrs[key] !== null) {
if (yDomAttrs[key] !== pAttrs[key] && key !== 'ychange') {
yDomFragment.setAttribute(key, pAttrs[key])
const pNodeMarksAttr = nodeMarksToAttributes(pNode.marks)
const attrs = { ...pAttrs, ...pNodeMarksAttr }

for (const key in attrs) {
if (attrs[key] !== null) {
if (yDomAttrs[key] !== attrs[key] && key !== 'ychange') {
yDomFragment.setAttribute(key, attrs[key])
}
} else {
yDomFragment.removeAttribute(key)
}
}
// remove all keys that are no longer in pAttrs
for (const key in yDomAttrs) {
if (pAttrs[key] === undefined) {
if (attrs[key] === undefined) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, we clean up unused attributes. When are unused marks removed?

yDomFragment.removeAttribute(key)
}
}
Expand Down