Skip to content

fix: null doc crashes aem2doc #84

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.

Sign up for GitHub

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

Merged
kptdobe merged 4 commits into from
Aug 28, 2025
Merged

fix: null doc crashes aem2doc #84

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5e383e1
fix: null doc crashes aem2doc + more debug
kptdobe Aug 28, 2025
568396d
fix: can parse if no main
kptdobe Aug 28, 2025
ec646d8
chore: adjust test
kptdobe Aug 28, 2025
27ee350
chore: remove debugs
kptdobe Aug 28, 2025
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
8 changes: 7 additions & 1 deletion src/collab.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,15 @@ function removeComments(node) {
return node;
}

export const EMPTY_DOC = '<body><header></header><main><div></div></main><footer></footer></body>';

export function aem2doc(html, ydoc) {
if (!html) {
// eslint-disable-next-line no-param-reassign
html = EMPTY_DOC;
}
const tree = fromHtml(html, { fragment: true });
const main = tree.children.find((child) => child.tagName === 'main');
const main = tree.children.find((child) => child.tagName === 'main') || { children: [] };
fixImageLinks(main);
removeComments(main);
(main.children || []).forEach((parent) => {
Expand Down
26 changes: 16 additions & 10 deletions src/shareddoc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import * as awarenessProtocol from 'y-protocols/awareness.js';
import * as encoding from 'lib0/encoding.js';
import * as decoding from 'lib0/decoding.js';
import debounce from 'lodash/debounce.js';
import { aem2doc, doc2aem } from './collab.js';
import { aem2doc, doc2aem, EMPTY_DOC } from './collab.js';

const wsReadyStateConnecting = 0;
const wsReadyStateOpen = 1;
Expand All @@ -27,7 +27,6 @@ const gcEnabled = false;
// The local cache of ydocs
const docs = new Map();

const EMPTY_DOC = '<main></main>';
const messageSync = 0;
const messageAwareness = 1;
const MAX_STORAGE_KEYS = 128;
Expand Down Expand Up @@ -153,14 +152,19 @@ export const storeState = async (docName, state, storage, chunkSize = MAX_STORAG
};

export const showError = (ydoc, err) => {
const em = ydoc.getMap('error');
try {
const em = ydoc.getMap('error');

// Perform the change in a transaction to avoid seeing a partial error
ydoc.transact(() => {
em.set('timestamp', Date.now());
em.set('message', err.message);
em.set('stack', err.stack);
});
// Perform the change in a transaction to avoid seeing a partial error
ydoc.transact(() => {
em.set('timestamp', Date.now());
em.set('message', err.message);
em.set('stack', err.stack);
});
} catch (e) {
// eslint-disable-next-line no-console
console.error('Error showing error', e, err);
}
};

export const persistence = {
Expand Down Expand Up @@ -330,6 +334,8 @@ export const persistence = {
// The doc was not restored from worker persistence, so read it from da-admin,
// but do this async to give the ydoc some time to get synced up first. Without
// this timeout, the ydoc can get confused which may result in duplicated content.
// eslint-disable-next-line no-console
console.log('Could not be restored, trying to restore from da-admin', docName);
setTimeout(() => {
if (ydoc === docs.get(docName)) {
const rootType = ydoc.getXmlFragment('prosemirror');
Expand All @@ -344,7 +350,7 @@ export const persistence = {
console.log('Restored from da-admin', docName);
} catch (error) {
// eslint-disable-next-line no-console
console.error('Problem restoring state from da-admin', error);
console.error('Problem restoring state from da-admin', error, current);
showError(ydoc, error);
}
});
Expand Down
26 changes: 25 additions & 1 deletion test/collab.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import assert from 'assert';
import * as Y from 'yjs';
import { readFileSync } from 'fs';
import { aem2doc, doc2aem, tableToBlock } from '../src/collab.js';
import { aem2doc, doc2aem, tableToBlock, EMPTY_DOC } from '../src/collab.js';

const collapseTagWhitespace = (str) => str.replace(/>\s+</g, '><');
const collapseWhitespace = (str) => collapseTagWhitespace(str.replace(/\s+/g, ' ')).trim();
Expand Down Expand Up @@ -578,6 +578,30 @@ assert.equal(result, html);
const result = doc2aem(yDoc);
assert.equal(collapseWhitespace(result), collapseWhitespace(html));
});

it('can parse empty doc', async () => {
const html = EMPTY_DOC;
const yDoc = new Y.Doc();
aem2doc(html, yDoc);
const result = doc2aem(yDoc);
assert.equal(collapseWhitespace(result), collapseWhitespace(EMPTY_DOC));
});

it('can parse null', async () => {
const html = null;
const yDoc = new Y.Doc();
aem2doc(html, yDoc);
const result = doc2aem(yDoc);
assert.equal(collapseWhitespace(result), collapseWhitespace(EMPTY_DOC));
});

it('can parse no main', async () => {
const html = '<body><header></header><footer></footer></body>';
const yDoc = new Y.Doc();
aem2doc(html, yDoc);
const result = doc2aem(yDoc);
assert.equal(collapseWhitespace(result), collapseWhitespace(EMPTY_DOC));
});
});


4 changes: 2 additions & 2 deletions test/shareddoc.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {
closeConn, getYDoc, invalidateFromAdmin, messageListener, persistence,
readState, setupWSConnection, setYDoc, showError, storeState, updateHandler, WSSharedDoc,
} from '../src/shareddoc.js';
import { aem2doc, doc2aem } from '../src/collab.js';
import { aem2doc, doc2aem, EMPTY_DOC } from '../src/collab.js';

function isSubArray(full, sub) {
if (sub.length === 0) {
Expand Down Expand Up @@ -534,7 +534,7 @@ describe('Collab Test Suite', () => {

await ydocUpdateCB[0]();
await ydocUpdateCB[1]();
assert.deepStrictEqual(['<main></main>'], updateCalled);
assert.deepStrictEqual([EMPTY_DOC], updateCalled);
});

it('Test bindstate read from worker storage', async () => {
Expand Down