Skip to content

fix: handle null declaration in DocxExporter to prevent export crash #2378

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

Open
aldrinjenson wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: handle null declaration in DocxExporter to prevent export crash #2378

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 5 additions & 1 deletion packages/super-editor/src/core/super-converter/exporter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,11 @@ export class DocxExporter {

#generate_xml_as_list(data, debug = false) {
const json = JSON.parse(JSON.stringify(data));
const declaration = this.converter.declaration.attributes;
const declaration = this.converter.declaration?.attributes ?? {
Copy link
Contributor

@caio-pizzol caio-pizzol Mar 18, 2026

Choose a reason for hiding this comment

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

this same fallback object is copy-pasted in 11+ files across the converter. there's already a constants.js next door — move it there and import it?

import { DEFAULT_XML_DECLARATION } from './constants.js';
// ...
const declaration = this.converter.declaration?.attributes ?? DEFAULT_XML_DECLARATION.attributes;

version: '1.0',
encoding: 'UTF-8',
standalone: 'yes',
};
const xmlTag = `<?xml${Object.entries(declaration)
.map(([key, value]) => ` ${key}="${value}"`)
.join('')}?>`;
Expand Down
42 changes: 42 additions & 0 deletions packages/super-editor/src/tests/export/docxExporter.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,48 @@ describe('DocxExporter', () => {
expect(xml).toContain('Format=&lt;&lt;NUM&gt;&gt;_&lt;&lt;VER&gt;&gt;');
});

describe('null declaration handling', () => {
Copy link
Contributor

@caio-pizzol caio-pizzol Mar 18, 2026

Choose a reason for hiding this comment

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

these two tests are the same except for null vs undefined. could be one test with it.each — less to read.

it('uses default XML declaration when converter.declaration is null', () => {
const exporter = new DocxExporter({ declaration: null });

const data = {
name: 'w:document',
attributes: {},
elements: [
{
name: 'w:t',
elements: [{ type: 'text', text: 'Hello' }],
},
],
};

const xml = exporter.schemaToXml(data);

expect(xml).toContain('<?xml version="1.0" encoding="UTF-8" standalone="yes"?>');
expect(xml).toContain('<w:t>Hello</w:t>');
});

it('uses default XML declaration when converter.declaration is undefined', () => {
const exporter = new DocxExporter({});

const data = {
name: 'w:document',
attributes: {},
elements: [
{
name: 'w:t',
elements: [{ type: 'text', text: 'Hello' }],
},
],
};

const xml = exporter.schemaToXml(data);

expect(xml).toContain('<?xml version="1.0" encoding="UTF-8" standalone="yes"?>');
expect(xml).toContain('<w:t>Hello</w:t>');
});
});

it('encodes all ampersands in text nodes including entity-like sequences', () => {
const exporter = new DocxExporter(createConverterStub());

Expand Down
Loading