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

snapshot: adding snapshot.toJSON convenience method #55

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

thlorenz
Copy link
Contributor

In some cases you want JSON send or write to a file.
The intuitive way is to JSON.stringify a snapshot object but that fails due to circular references.

This method makes that easier.

@thlorenz
Copy link
Contributor Author

BTW if you want I can also supply a snapshot.toStream(stream) convenience which could of course also be used to achieve the same result without holding the entire snapshot in memory.

@3y3
Copy link
Member

3y3 commented May 1, 2015

Hello, @thlorenz , sorry for pending this pr. I have a lot of work on node-inspector.

I see a problems with using reserved method toJSON. What happens if someone will try to use JSON.stringify(snapshot)? At current time we'll receive an error about circular references, but after patching we'll silently pass this call.

I'd like to see also stream feature. How about a method that can work with callback and stream?
I'd like to have snapshot.export method and use it like:

var snapshot = profiler.takeSnapshot();
snapshot.export(function(err, result) {});
snapshot.export(writeStream);
snapshot.export().pipe(readStream||writeStream)

Something like this:

function ExportStream() {
  Stream.Transform.call(this);
  this._transform = function noTransform(chunk, encoding, done) {
    done(null, chunk);
  }
}
inherits(ExportStream, Stream.Transform);

/**
 * @param {Stream.Writable|function} dataReceiver
 * @returns {Stream|function}
 */
Snapshot.prototype.export = function(dataReceiver) {
  dataReceiver = dataReceiver || new ExportStream();

  var toStream = dataReceiver instanceof Stream,
      chunks = toStream ? null : [];

  function onChunk(chunk, len) {
    if (toStream) dataReceiver.write(chunk);
    else chunks.push(chunk);
  }

  function onDone() {
    if (toStream) dataReceiver.end();
    else dataReceiver(null, chunks.join(''));
  }

  this.serialize(onChunk, onDone);

  return dataReceiver;
};

In future this allows me to implement also snapshot.import;
If you agree, I'm ready to prepare new pr for your review.

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.

None yet

2 participants