matteodelabre / saxophone Goto Github PK

Hey @matteodelabre, I am considering using your library for my XML minifier minify-xml, however parsing documents with a DOCTYPE resulting in a error is a blocker for me at this point in time. How about and how would you judge the effort adding basic DOCTYPE support, similar to processing instructions soon? Content parsing, similar to parseAttrs could be added later on and for my (many other purpose) getting an event with a content field would be enough for me and is in fact much better than just throwing an error for an otherwise valid document.

What do you think? Any chance of you adding this, or could you explain what was the complication to not having it added in the first place? Thanks!

As per the xml specification, the character > needn’t be escaped in attributes (was also discussed on stackoverflow). However, saxophone’s tag matching gets messed up in this case.
I’ll have a look and see if I can create a PR for this.

Would it be possible to remove (or update) the dependency on Node.js versions, see this line? Node.js version 20.9 is a LTS release and as far as I can tell this package works just fine on 20.9.

With nodejs 14 now on LTS, the library fails to install (due to the limitations set in the package.json).
I tested it briefly and I had no issues with node 14.

Would it be possible to support it or do you think there could be any issues?
Thank you!

npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '[email protected]',
npm WARN EBADENGINE   required: { node: '10 || 12 || 14 || 15' },
npm WARN EBADENGINE   current: { node: 'v16.13.0', npm: '8.1.0' }
npm WARN EBADENGINE }

Node 16 is in Active LTS https://nodejs.org/en/about/releases/

The way the parser handles comments is not safe at the moment.
One in every ~10,000 comments in a stream throws an error.

This is because of the check for comments containing "--" in the section starting line 284.

I have fixed this by changing the code as below and opened a PR.
The important part is the check:

input[commentClose + 2] === undefined

Full code:

if (
	nextNextChar === '-' &&
	(input[chunkPos + 1] === undefined || input[chunkPos + 1] === '-')
) {
	chunkPos += 2;
	const commentClose = input.indexOf('--', chunkPos);

	// Incomplete comment node, we need to wait for
	// upcoming data
	if (commentClose === -1 || input[commentClose + 2] === undefined) {
		this._wait(Node.comment, input.slice(chunkPos - 4));
		break;
	}

	if (input[commentClose + 2] !== '>') {
		callback(new Error('Unexpected -- inside comment: ' + input.slice(chunkPos - 4)));
		return;
	}

	this.emit(Node.comment, { contents: input.slice(chunkPos, commentClose) });

	chunkPos = commentClose + 3;
	continue;
}

It would be nice to have type declarations for TypeScript for this parser. Unfortunately the JSDoc type declarations aren't enough for TypeScript to work.

I will create the declarations based on your code and submit a pull request when it's done.

In my fork https://github.com/scalableminds/saxophone/tree/pub, I added a check that keeps track of the opening and closing tags to confirm the basic validity of the XML document. See these tests for an example.

This changes the semantics of the parser a bit. Would you be interested in merging these changes into your repo? If so, I'd be happy to adapt my changes to your recent code cleanup and create a PR. Let me know what your thoughts are.

Please bump the version to support node v15. There is xlstream package I use that depends on yours.

Thanks