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 #2355 - check for correct DOM error on XML parse failure #2356

Merged
merged 4 commits into from
Jan 11, 2018

Conversation

davemevans
Copy link
Contributor

Also added a test.

Requires jsdom as no other node xml parser I can find seems to follow the spec wrt parsererror. We could consider replacing xmldom used elsewhere to cut out a dev dependency.

@epiclabsDASH
Copy link
Contributor

epiclabsDASH commented Jan 9, 2018

Good catch!

Just replaced xmldom with jsdom and everything worked fine, so we can remove xmldom safely and cut out a dependency. There is just one drawback on using jsdom: xmldom size is about 73Kb, jsdom is 2Mb. Anyway, given it is a dev dependency, size shouldn't be our biggest problem. This is going to impact a bit our build time in travis although I don't expect big differences (note: at some point we need to enable Travis cache mechanism for deps).

@epiclabsDASH epiclabsDASH added this to the v2.6.6 milestone Jan 9, 2018
@epiclabsDASH epiclabsDASH merged commit fb0454e into Dash-Industry-Forum:development Jan 11, 2018
@davemevans davemevans deleted the 2355 branch January 11, 2018 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants