Skip to content
Merged
Show file tree
Hide file tree
Changes from 14 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
1 change: 1 addition & 0 deletions examples/vanilla-codemirror6/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
},
"dependencies": {
"@codemirror/state": "^6.5.2",
"@codemirror/view": "^6.4.1",
"@yorkie-js/sdk": "workspace:*",
"codemirror": "^6.0.2"
}
Expand Down
90 changes: 61 additions & 29 deletions examples/vanilla-codemirror6/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import type { EditOpInfo, OpInfo } from '@yorkie-js/sdk';
import yorkie, { DocEventType } from '@yorkie-js/sdk';
import { basicSetup, EditorView } from 'codemirror';
import { Transaction, TransactionSpec } from '@codemirror/state';
import { keymap } from '@codemirror/view';
import { network } from './network';
import { displayLog, displayPeers } from './utils';
import { YorkieDoc, YorkiePresence } from './type';
Expand All @@ -19,6 +20,7 @@ async function main() {
});
await client.activate();

const params = new URLSearchParams(window.location.search);
// 02-1. create a document then attach it into the client.
const doc = new yorkie.Document<YorkieDoc, YorkiePresence>(
`codemirror6-${new Date()
Expand Down Expand Up @@ -48,34 +50,8 @@ async function main() {
}
}, 'create content if not exists');

// 02-2. subscribe document event.
const syncText = () => {
const text = doc.getRoot().content;
const selection = doc.getMyPresence().selection;
const transactionSpec: TransactionSpec = {
changes: { from: 0, to: view.state.doc.length, insert: text.toString() },
annotations: [Transaction.remote.of(true)],
};

if (selection) {
// Restore the cursor position when the text is replaced.
const cursor = text.posRangeToIndexRange(selection);
transactionSpec['selection'] = {
anchor: cursor[0],
head: cursor[1],
};
}
view.dispatch(transactionSpec);
};
doc.subscribe((event) => {
if (event.type === 'snapshot') {
// The text is replaced to snapshot and must be re-synced.
syncText();
}
});

doc.subscribe('$.content', (event) => {
if (event.type === 'remote-change') {
if (event.type === 'remote-change' || event.source === 'undoredo') {
const { operations } = event.value;
handleOperations(operations);
}
Expand All @@ -87,7 +63,7 @@ async function main() {
const updateListener = EditorView.updateListener.of((viewUpdate) => {
if (viewUpdate.docChanged) {
for (const tr of viewUpdate.transactions) {
const events = ['select', 'input', 'delete', 'move', 'undo', 'redo'];
const events = ['select', 'input', 'delete', 'move'];
if (!events.map((event) => tr.isUserEvent(event)).some(Boolean)) {
continue;
}
Expand Down Expand Up @@ -138,11 +114,67 @@ async function main() {
const fixedHeightTheme = EditorView.theme({
'.cm-content, .cm-gutter': { minHeight: '210px' }, // ~10 lines (≈21px per line including padding)
});
const cmUndoRedoKeymap = keymap.of([
{
key: 'Mod-z',
preventDefault: true,
run: () => {
// To check undo works properly
console.log('undo');
if (doc.history.canUndo()) {
doc.history.undo();
}
return true;
},
},
{
key: 'Mod-Shift-z',
preventDefault: true,
run: () => {
// To check redo works properly
console.log('redo');
if (doc.history.canRedo()) {
doc.history.redo();
}
return true;
},
},
]);
const view = new EditorView({
doc: '',
extensions: [basicSetup, fixedHeightTheme, updateListener],
extensions: [
cmUndoRedoKeymap,
basicSetup,
fixedHeightTheme,
updateListener,
],
parent: editorParentElem,
});
// 02-2. subscribe document event.
const syncText = () => {
const text = doc.getRoot().content;
const selection = doc.getMyPresence().selection;
const transactionSpec: TransactionSpec = {
changes: { from: 0, to: view.state.doc.length, insert: text.toString() },
annotations: [Transaction.remote.of(true)],
};

if (selection) {
// Restore the cursor position when the text is replaced.
const cursor = text.posRangeToIndexRange(selection);
transactionSpec['selection'] = {
anchor: cursor[0],
head: cursor[1],
};
}
view.dispatch(transactionSpec);
};
doc.subscribe((event) => {
if (event.type === 'snapshot') {
// The text is replaced to snapshot and must be re-synced.
syncText();
}
});

// 03-3. define event handler that apply remote changes to local
function handleOperations(operations: Array<OpInfo>) {
Expand Down
6 changes: 4 additions & 2 deletions packages/sdk/src/document/change/change.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,12 @@ export class Change<P extends Indexable> {
presences: Map<ActorID, P>,
source: OpSource,
): {
operations: Array<Operation>;
opInfos: Array<OpInfo>;
reverseOps: Array<HistoryOperation<P>>;
} {
const changeOpInfos: Array<OpInfo> = [];
const changeOperations: Array<Operation> = [];
const reverseOps: Array<HistoryOperation<P>> = [];

for (const operation of this.operations) {
Expand All @@ -172,7 +174,7 @@ export class Change<P extends Indexable> {
if (!executionResult) continue;
const { opInfos, reverseOp } = executionResult;
changeOpInfos.push(...opInfos);

changeOperations.push(operation);
// TODO(hackerwins): This condition should be removed after implementing
// all reverse operations.
if (reverseOp) {
Expand All @@ -191,7 +193,7 @@ export class Change<P extends Indexable> {
}
}

return { opInfos: changeOpInfos, reverseOps };
return { operations: changeOperations, opInfos: changeOpInfos, reverseOps };
}

/**
Expand Down
71 changes: 68 additions & 3 deletions packages/sdk/src/document/crdt/rga_tree_split.ts
Original file line number Diff line number Diff line change
Expand Up @@ -597,14 +597,20 @@ export class RGATreeSplit<T extends RGATreeSplitValue> implements GCParent {
* @param range - range of RGATreeSplitNode
* @param editedAt - edited time
* @param value - value
* @returns `[RGATreeSplitPos, Array<GCPair>, Array<Change>]`
* @returns `[RGATreeSplitPos, Array<GCPair>, DataSize, Array<ValueChange<T>>, Array<T>]`
*/
public edit(
range: RGATreeSplitPosRange,
editedAt: TimeTicket,
value?: T,
versionVector?: VersionVector,
): [RGATreeSplitPos, Array<GCPair>, DataSize, Array<ValueChange<T>>] {
): [
RGATreeSplitPos,
Array<GCPair>,
DataSize,
Array<ValueChange<T>>,
Array<T>,
] {
const diff = { data: 0, meta: 0 };

// 01. split nodes with from and to
Expand Down Expand Up @@ -659,11 +665,13 @@ export class RGATreeSplit<T extends RGATreeSplitValue> implements GCParent {

// 04. add removed node
const pairs: Array<GCPair> = [];
const removedValues: Array<T> = [];
for (const [, removedNode] of removedNodes) {
pairs.push({ parent: this, child: removedNode });
removedValues.push(removedNode.getValue());
}

return [caretPos, pairs, diff, changes];
return [caretPos, pairs, diff, changes, removedValues];
}

/**
Expand Down Expand Up @@ -787,6 +795,63 @@ export class RGATreeSplit<T extends RGATreeSplitValue> implements GCParent {
return clone;
}

/**
* `normalizePos` converts a local position `(id, rel)` into a single
* absolute offset measured from the head `(0:0)` of the physical chain.
*/
public normalizePos(pos: RGATreeSplitPos): RGATreeSplitPos {
const index = this.posToIndex(pos, true);

return RGATreeSplitPos.of(this.getHead().getID(), index);
}

/**
* `refinePos` remaps the given pos to the current split chain.
*
* - Traverses the physical `next` chain (not `insNext`).
* - Counts only live characters: removed nodes are treated as length 0.
* - If the given offset exceeds the length of the current node,
* it moves forward through `next` nodes, subtracting lengths,
* until the offset fits in a live node.
* - If it runs out of nodes, it snaps to the end of the last node.
*
* Example:
* Before split: ["12345"](1:2:0), pos = (1:2:0, rel=5)
* After split : ["1"](1:2:0) - ["23"](1:2:1) - ["45"](1:2:3)
* refinePos(pos) -> (1:2:3, rel=2)
*
* Example:
* ["12"](1:2:0, live) and pos = (1:2:0, rel=4)
* refinePos(pos) -> points two chars after "12",
* i.e. advances into following nodes, skipping removed ones.
*/
public refinePos(pos: RGATreeSplitPos): RGATreeSplitPos {
let node = this.findFloorNode(pos.getID());
if (!node) {
throw new YorkieError(
Code.ErrInvalidArgument,
`the node of the given id should be found: ${pos.getID().toTestString()}`,
);
}

let offsetInPart = pos.getRelativeOffset();
let partLen = node.getContentLength();

while (offsetInPart > partLen) {
offsetInPart -= partLen;
const next: RGATreeSplitNode<T> | undefined = node!.getNext();

if (!next) {
return RGATreeSplitPos.of(node.getID(), partLen);
}

node = next;
partLen = node.getLength();
}

return RGATreeSplitPos.of(node.getID(), offsetInPart);
}
Comment on lines +844 to +869
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Search for tests that verify refinePos behavior
rg -n "refinePos" --type=ts -A 5 -B 2

Repository: yorkie-team/yorkie-js-sdk

Length of output: 3764


🏁 Script executed:

# Search for getContentLength and getLength definitions
rg -n "getContentLength\(\)|getLength\(\)" packages/sdk/src/document/crdt/rga_tree_split.ts --type=ts -B 2 -A 3

Repository: yorkie-team/yorkie-js-sdk

Length of output: 1498


🏁 Script executed:

# Examine the RGATreeSplitNode class to understand these methods
ast-grep --pattern 'class RGATreeSplitNode {
  $$$
}'

Repository: yorkie-team/yorkie-js-sdk

Length of output: 51


🏁 Script executed:

# Search for findFloorNode to understand if it can return removed nodes
rg -n "findFloorNode" packages/sdk/src/document/crdt/rga_tree_split.ts --type=ts -A 10 -B 2

Repository: yorkie-team/yorkie-js-sdk

Length of output: 2734


🏁 Script executed:

# Search for test cases that test refinePos with removed content
rg -n "refinePos" --type=ts -l | xargs grep -l "remov\|delete" | head -5

Repository: yorkie-team/yorkie-js-sdk

Length of output: 210


🏁 Script executed:

# Search specifically in test files for refinePos tests
fd ".*\.test\.ts$|.*\.spec\.ts$" --type=f | xargs rg -l "refinePos" 2>/dev/null | head -3

Repository: yorkie-team/yorkie-js-sdk

Length of output: 51


Fix inconsistent length calculation for initial node in refinePos.

Line 838 uses getContentLength() (ignores removal status) for the initial node, but line 849 uses getLength() (returns 0 for removed nodes) for subsequent nodes. This violates the documented behavior (line 812-813) that states: "Counts only live characters: removed nodes are treated as length 0."

When the initial node is removed, the current code incorrectly uses its full content length instead of treating it as 0. Change line 838 to use node.getLength() for consistency with the loop and the documented specification.

🤖 Prompt for AI Agents
In packages/sdk/src/document/crdt/rga_tree_split.ts around lines 828 to 853, the
initial node length is obtained with node.getContentLength() (line ~838) which
ignores removal status, causing removed nodes to be counted; change that call to
node.getLength() so the initial node is treated the same as subsequent nodes in
the loop, and keep the rest of the logic—assign partLen = node.getLength() and
let the while loop and return statements operate as before.


/**
* `toTestString` returns a String containing the meta data of the node
* for debugging purpose.
Expand Down
43 changes: 35 additions & 8 deletions packages/sdk/src/document/crdt/text.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
RGATreeSplit,
RGATreeSplitNode,
RGATreeSplitNodeID,
RGATreeSplitPos,
RGATreeSplitPosRange,
ValueChange,
} from '@yorkie-js/sdk/src/document/crdt/rga_tree_split';
Expand Down Expand Up @@ -239,20 +240,22 @@ export class CRDTText<A extends Indexable = Indexable> extends CRDTElement {
editedAt: TimeTicket,
attributes?: Record<string, string>,
versionVector?: VersionVector,
): [Array<TextChange<A>>, Array<GCPair>, DataSize, RGATreeSplitPosRange] {
): [
Array<TextChange<A>>,
Array<GCPair>,
DataSize,
RGATreeSplitPosRange,
Array<CRDTTextValue>,
] {
const crdtTextValue = content ? CRDTTextValue.create(content) : undefined;
if (crdtTextValue && attributes) {
for (const [k, v] of Object.entries(attributes)) {
crdtTextValue.setAttr(k, v, editedAt);
}
}

const [caretPos, pairs, diff, valueChanges] = this.rgaTreeSplit.edit(
range,
editedAt,
crdtTextValue,
versionVector,
);
const [caretPos, pairs, diff, valueChanges, removedValues] =
this.rgaTreeSplit.edit(range, editedAt, crdtTextValue, versionVector);

const changes: Array<TextChange<A>> = valueChanges.map((change) => ({
...change,
Expand All @@ -268,7 +271,7 @@ export class CRDTText<A extends Indexable = Indexable> extends CRDTElement {
type: TextChangeType.Content,
}));

return [changes, pairs, diff, [caretPos, caretPos]];
return [changes, pairs, diff, [caretPos, caretPos], removedValues];
}

/**
Expand Down Expand Up @@ -392,6 +395,20 @@ export class CRDTText<A extends Indexable = Indexable> extends CRDTElement {
return this.rgaTreeSplit.getTreeByID();
}

/**
* `refinePos` refines the given RGATreeSplitPos.
*/
public refinePos(pos: RGATreeSplitPos): RGATreeSplitPos {
return this.rgaTreeSplit.refinePos(pos);
}

/**
* `normalizePos` normalizes the given RGATreeSplitPos.
*/
public normalizePos(pos: RGATreeSplitPos): RGATreeSplitPos {
return this.rgaTreeSplit.normalizePos(pos);
}

/**
* `getDataSize` returns the data usage of this element.
*/
Expand Down Expand Up @@ -507,6 +524,16 @@ export class CRDTText<A extends Indexable = Indexable> extends CRDTElement {
return this.rgaTreeSplit.findIndexesFromRange(range);
}

/**
* `posToIndex` converts the given position to index.
*/
public posToIndex(
pos: RGATreeSplitPos,
preferToLeft: boolean = false,
): number {
return this.rgaTreeSplit.posToIndex(pos, preferToLeft);
}

/**
* `getGCPairs` returns the pairs of GC.
*/
Expand Down
Loading