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

Change API #142

Closed
3 tasks done
knownasilya opened this issue Sep 21, 2016 · 15 comments
Closed
3 tasks done

Change API #142

knownasilya opened this issue Sep 21, 2016 · 15 comments
Milestone

Comments

@knownasilya
Copy link
Collaborator

knownasilya commented Sep 21, 2016

  • Allowing a simpler invocation.
let results = json2csv(data, options)
json2csv.stream(data, options)
  .pipe(csv)
@knownasilya knownasilya added this to the v4 milestone Sep 21, 2016
@knownasilya
Copy link
Collaborator Author

knownasilya commented Jan 18, 2018

@juanjoDiaz if you are interested in doing the second item as well.

@juanjoDiaz
Copy link
Collaborator

juanjoDiaz commented Jan 18, 2018

Any reason to use stream instead of promises?
Or actually just return since there is nothing async about the library.

@juanjoDiaz
Copy link
Collaborator

Nevermind, I see the point. It parses lines one by one and you can even track progress.

I might take a look at it :)

@knownasilya
Copy link
Collaborator Author

@zemirco could you add @juanjoDiaz as a collaborator? Thanks!

@knownasilya
Copy link
Collaborator Author

I've pushed up changes to remove callback support.

@zemirco
Copy link
Owner

zemirco commented Jan 24, 2018

done

screenshot from 2018-01-24 11-52-23

@knownasilya
Copy link
Collaborator Author

Thanks! @zemirco could you add him in Github as well?

@zemirco
Copy link
Owner

zemirco commented Jan 24, 2018

done

screenshot from 2018-01-24 17-03-08

@juanjoDiaz
Copy link
Collaborator

Awesome! Thanks guys. Glad to be helpful :)

@juanjoDiaz
Copy link
Collaborator

juanjoDiaz commented Jan 27, 2018

@zemirco Where did you take the RegExp in https://github.com/zemirco/json2csv-stream/blob/master/index.js#L78 from?

It doesn't seem to work:

 [ '{\n  "field1": {\n    "embeddedField1": "embeddedValue1",\n    "embeddedField2": "embeddedValue2"\n  }',
      '{\n  "field1": {\n    "embeddedField1": "embeddedValue1",\n    "embeddedField2": "embeddedValue2"\n  }',
      '"embeddedValue2"\n  ',
      '2',
      index: 0,
      input: '{\n  "field1": {\n    "embeddedField1": "embeddedValue1",\n    "embeddedField2": "embeddedValue2"\n  }\n}' ]

Currently that's my only blocker for adding the streaming API to this project.

@knownasilya
Copy link
Collaborator Author

What about using https://www.npmjs.com/package/JSONStream

@juanjoDiaz
Copy link
Collaborator

I've looked into that one, and also into JSONparse which JSONStream uses under the hood.
The problem is that both actually caches the whole JSON in memory while processing it which totally defeats the purpose of having a streaming api imho....

@knownasilya
Copy link
Collaborator Author

Wow I didn't know that. Good catch.

@juanjoDiaz
Copy link
Collaborator

But no problem. I dug into the library and learn how to override it's behaviour to don't do that. You'll see. :)

@knownasilya
Copy link
Collaborator Author

Streaming api added by #235

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

No branches or pull requests

3 participants