node-xml2js
node-xml2js copied to clipboard
Why do we need a callback for parseString()?
Since the function parseString is not asynchronous at all ?
(() => {
var a;
new require('xml2js').Parser().parseString('<xml>asdf</xml>', (e, r) => { a = r });
console.log(a);
})();
and the output is:
{ xml: 'asdf' }
Backward compatibility, since changing the signature breaks all existing depending libraries.
Didn't stop the guy behind jsdom. He recently rewrote his API to be callback-less (though still provides a way to use the old API, at least temporarily.)
@Leonidas-from-XIV how do you feel about adding parseStringSync
which returns the result?
I am trying to use this in a Redux saga, but the callback structure is a showstopper since "yield" won't work inside the callback.
Yeah, it's a hassle to not have a synchronous version of parseString. Thanks @zszszsz for a concise workaround.
Anyways, @jeghers See https://redux-saga.js.org/docs/api/#cpsfn-args for using the node-style callback in Redux-Saga
e.g. something like:
const js = yield cps(parseString, xml);
Cool thanks!
On Jan 4, 2018 12:17 PM, "Philip Davis" [email protected] wrote:
Yeah, it's a hassle to not have a synchronous version of parseString. Thanks @zszszsz https://github.com/zszszsz for a concise workaround.
Anyways, @jeghers https://github.com/jeghers See https://redux-saga.js.org/docs/api/#cpsfn-args for using the node-style callback in Redux-Saga
e.g. something like:
const js = yield cps(parseString, xml);
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Leonidas-from-XIV/node-xml2js/issues/380#issuecomment-355387541, or mute the thread https://github.com/notifications/unsubscribe-auth/AGaRneIG0ue3IAuVE-WPVVXHw5kK0eltks5tHTHUgaJpZM4NLbyR .
@PhilipDavis This parseString
is itself synchronous in nature, it is not a workaround actually.
@jeghers you can write you own parseStringSync
using the techniques or "workaround" above, exp.
function parseStringSync (str) {
var result;
new require('xml2js').Parser().parseString(str, (e, r) => { result = r });
return result;
}
, and you can avoid using yield
in callback
That looks good, thanks for the input!
On Jan 4, 2018 12:26 PM, "zszszsz" [email protected] wrote:
@PhilipDavis https://github.com/philipdavis This parseString is itself synchronous in nature, it is not a workaround actually.
@jeghers https://github.com/jeghers you can write you own parseStringSync using the techniques or "workaround" above, exp.
function parseStringSync (str) { var result; new require('xml2js').Parser().parseString(str, (e, r) => { result = r }); return result; }
, and you can avoid using yield in callback
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Leonidas-from-XIV/node-xml2js/issues/380#issuecomment-355389602, or mute the thread https://github.com/notifications/unsubscribe-auth/AGaRnaY2nxjd_muG6doVZcQXZltXHKFDks5tHTQBgaJpZM4NLbyR .
@zszszsz parseString doesn't return the result directly. Hence, your workaround to retrieve it using a closure. What do you call it if not a workaround?
@jfsiii The parser library that we use, sax.js
also provides an async API AFAIK, so unless they provide one there is no clean way to implement parseStringSync
in a non-hacky way (like the closure workaround posted here).
I added this function to my copy of parser.coffee
. Seems to work quite nicely, and I don't think it's hacky. Should I submit a pull request?
parseStringSync: (str) =>
res = undefined
err = undefined
@on "end", (r) ->
@reset()
res = r
@on "error", (e) ->
@reset()
err = e
@saxParser.write(str).close()
if err?
throw err
else
return res
I haven't tested it thoroughly yet. The error handling in the original parseString
is much more elaborate. I omitted it because I didn't understand it. parseStringSync
seems to work fine as it is. When the SAX parser throws an exception, parseStringSync
simply lets that exception pass through to the caller, which is fine.
I also omitted the handling of BOMs and empty strings for my tests, but I can easily put that back in.
The process of parsing xml is CPU exclusive type, So async meaningless. Direct return is a better solution.
I'm want to build a package on top of xml2js to parse OPML specifically, to make it super easy for people to use. I want to emulate the way JSON.parse
and JSON.stringify
work. Do I really need to have a callback?
Two possible solutions:
- Couldn't the parse routine "just work" if the callback is undefined?
- Have another entry-point to the package called parseSync that returns the JavaScript structure for the XML and throws an error if there's an error.
This is how xml2js.parseStringSync would work.
function parse (xmlltext) {
var options = {
explicitArray: false
};
return (xml2js.parseStringSync (xmlltext, options));
}
I wonder, collectively, how many hours people have wasted stumbling upon this.... During an integration/refactoring, converting json to xml (dont ask), I almost made rewrote everything into promises/async because I assumed this is async. So tally up +1 hr and a 5 minutes debate from my end. 😄 One time rookie mistake - but expensive.