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

Convert to Stream object and allow options to be passed in #72

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

willfarrell
Copy link

BREAKING CHANGE
Closes #16

  • Stream part by @cjblomqvist
  • See README for options
  • See example for piping out json (examples/json.js)

@Amantel
Copy link

Amantel commented Feb 26, 2018

Should not this be merged?

@Amantel
Copy link

Amantel commented Feb 26, 2018

Hm, sorry, but should not it be possible to pipe this to the next stage?
like input.pipe(parse).pipe(fs.writeStream('xxx'))

lib/xml-stream.js Outdated Show resolved Hide resolved
@hubgit
Copy link

hubgit commented Aug 31, 2018

It does seem that piping the output stream to a file creates corrupted output, for some reason.

@jobe451
Copy link

jobe451 commented Sep 21, 2018

I inluded this pull request into one of my projects and it seems to perform just fine.

This example seems rather to be wrong. Obviously you can't pipe json to a stream-writer that expects to get string or buffer to be streamed to.

input.pipe(parse).pipe(fs.writeStream('xxx'))

Working example (extended from the example coming in this pull request)

const stream  = require('stream');
const fs = require('fs');
const xmlStream = require('../');

class ConsoleLogStream extends stream.Writable {
  _write(chunk, enc, next) {
  	console.log(chunk);
    next();
  }
}

const input = fs.createReadStream('json.xml');
const parse = new xmlStream({
  element: 'media',
  attributes: false,
  output:'json',
  preserve: {},
  collect:['id']
});
const logStream = new ConsoleLogStream();

input.pipe(parse).pipe(logStream);

@xShirase
Copy link

Yeah, that works and should be merged IMO

@jobe451
Copy link

jobe451 commented Oct 4, 2018

There is one issue I discovered with this pull-request. Once a file is streamed, the "finish" event is not triggered a the end of the pipe. Also if I inhearit from this class, _final won't be triggered. I assume that this is caused because those class inherit from the base class stream.Stream and not from Readable and Writeable. I will do my own implementation in Typescript by wrapping it somehow...

@jobe451
Copy link

jobe451 commented Oct 4, 2018

I removed xml-stream and am using now directly node-expat. Turns out also node-expat does not emit an "end" event, which probably is the root cause here.

So this my core typescript calls now:

import stream = require("stream");
import expat = require("node-expat");

export class XmlParser extends stream.Transform {
	private parser: expat.Parser;
	private parsingLevel = 0;

	constructor (config: any) {
		super({objectMode: true});

		this.parser  = new expat.Parser('UTF-8')

		this.parser.on('startElement', (name, attrs) => {
			this.parsingLevel++;
		});

		this.parser.on("error", (error) => {
			this.emit("error", error);
		});

		this.parser.on('endElement', (text) => {
			this.parsingLevel--;
			if(this.parsingLevel === 0) {
				this.emit("end");
			}
		});
	}

	_transform(chunk, encoding, callback) {
		this.parser.write(chunk, encoding, callback);
	}
}

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.

5 participants