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

Move xmldom parsing to internal interface #19

Merged
merged 3 commits into from
Feb 5, 2018
Merged
Show file tree
Hide file tree
Changes from all 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 CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.

## [Unreleased]
- Initial Release
- Changed XML parsing to use an internal interface.

<!-- Update the Unreleased comparison range with each release -->
[Unreleased]: https://github.com/CondeNast/xml-to-react/compare/x.y.z...master
20 changes: 3 additions & 17 deletions src/XMLToReact.js
Original file line number Diff line number Diff line change
@@ -1,18 +1,8 @@
import { DOMParser } from 'xmldom';
import { parse } from './parser';
import { validateConverters, visitNode } from './helpers';

const ERR_INVALID_XML = 'XMLToReact: Unable to parse invalid XML input. Please input valid XML.';
const ERR_INVALID_CONVERTERS = 'XMLToReact: Invalid value for converter map argument. Please use an object with functions as values.';

const throwError = (m) => { throw new Error(m); };

const parser = new DOMParser({
errorHandler: throwError,
fatalError: throwError,
warning: throwError,
});


/**
* Class representing an XML to React transformer.
* @public
Expand Down Expand Up @@ -48,12 +38,8 @@ export default class XMLToReact {
return null;
}

let tree;

try {
tree = parser.parseFromString(xml, 'text/xml');
} catch (e) {
console.warn(ERR_INVALID_XML);
const tree = parse(xml);
if (!tree) {
return null;
}

Expand Down
33 changes: 33 additions & 0 deletions src/parser.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { DOMParser } from 'xmldom';

export const ERR_INVALID_XML = 'XMLToReact: Unable to parse invalid XML input. Please input valid XML.';

const throwError = (m) => { throw new Error(m); };

const parser = new DOMParser({
errorHandler: throwError,
fatalError: throwError,
warning: throwError,
});


/**
* Parse an xml string
*
* @param {string} xml - xml to convert
* @returns {Object} - xml tree
* @public
*/
export function parse(xml) {
if (typeof xml !== 'string') {
return null;
}

try {
return parser.parseFromString(xml, 'text/xml');
} catch (e) {
console.warn(ERR_INVALID_XML);
Copy link
Contributor Author

@pgoldrbx pgoldrbx Dec 15, 2017

Choose a reason for hiding this comment

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

I notice now that we're not logging the exception raised by the parser. However, I think this is better addressed in #14

}

return null;
}
15 changes: 1 addition & 14 deletions test/helpers.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { expect } from 'chai';
import { DOMParser } from 'xmldom';
import { isValidElement } from 'react';
import sinon from 'sinon';
import { parse as parseXML } from '../src/parser';
import {
validateConverters,
getAttributes,
Expand All @@ -10,19 +10,6 @@ import {
} from '../src/helpers';

describe('helpers', () => {
let parseXML;

before(() => {
// `parseXML` test helper to create XML nodes from strings
const throwError = (m) => { throw new Error(m); };
const parser = new DOMParser({
errorHandler: throwError,
fatalError: throwError,
warning: throwError,
});
parseXML = xml => parser.parseFromString(xml, 'text/xml');
});

describe('validateConverters', () => {
it('should export a function', () => {
expect(validateConverters).to.be.a('function');
Expand Down
51 changes: 51 additions & 0 deletions test/parser.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import { DOMParser } from 'xmldom';
import { expect } from 'chai';
import sinon from 'sinon';

import { parse, ERR_INVALID_XML } from '../src/parser';

describe('XML parser', () => {
let sandbox;

before(() => {
sandbox = sinon.sandbox.create();
});

afterEach(() => {
sandbox.restore();
});

describe('parse', () => {
it('should export a function', () => {
expect(parse).to.be.a('function');
});

it('should return `null` when no text is passed', () => {
expect(parse()).to.equal(null);
expect(parse('foo')).not.to.equal(null);
});

it('should parse XML from a string', () => {
const mockXMLTree = {};
sandbox.stub(DOMParser.prototype, 'parseFromString').returns(mockXMLTree);
expect(parse('foo')).to.equal(mockXMLTree);
sinon.assert.calledWith(DOMParser.prototype.parseFromString, 'foo', 'text/xml');
});

describe('When an invalid XML string is passed', () => {
beforeEach(() => {
sandbox.stub(console, 'warn');
sandbox.stub(DOMParser.prototype, 'parseFromString').throws(new Error('boom'));
});

it('should return `null`', () => {
expect(parse('foo')).to.equal(null);
});

it('should log a warning', () => {
parse('foo');
sinon.assert.calledWith(console.warn, ERR_INVALID_XML);
});
});
});
});