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

Add support for objectMode streaming #82

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

Conversation

air2
Copy link

@air2 air2 commented Sep 21, 2020

Status

READY

Description

This commit supports streaming objects, instead of stringified JSON, by adding objectMode to the options and implementing it in the stream.js

Related PRs

List related PRs against other branches:

branch PR
other_pr_production link
other_pr_master link

Todos

  • Tests
  • Documentation

Steps to Test or Reproduce

Outline the steps to test or reproduce the PR here.

git pull --prune
git checkout <feature_branch>
bundle; script/server

Impacted Areas in Application

List general components of the application that this PR will affect:

Added support for objectMode streaming.
Added support for object mode streaming.
@kaue kaue self-requested a review October 6, 2020 15:11
@AckerApple
Copy link
Collaborator

I have reviewed some of the code here. I haven’t had too much experience in our jsonexport in regards to streaming. However, what I think I am reading here sounds amazing.

@air2 could you please elaborate how this works, if I understand correctly we can now stream, with your code, in chunks of valid json-to-object... If you could elaborate please. I am a contributor and could help along if I also better understood a better picture here please and thank you

@AckerApple
Copy link
Collaborator

Also it would help me if you were able to provide a unit test in any fashion to accompany this PR

Copy link
Owner

@kaue kaue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be great if we can include some tests & docs for the new objectMode option

if (this._options.objectMode) {
json = Array.isArray(chunk) ? chunk : [chunk];
}
else {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would remove this else to keep this part less indented

@kaue kaue added the WIP work in progress label Sep 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WIP work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants