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

Bug: CLI incorrectly parsing JSON Object as JSON Array #92

Closed

Conversation

ChazUK
Copy link

@ChazUK ChazUK commented Feb 4, 2021

Status

READY

Description

CLI incorrectly parses JSON Object as a JSON Array, leading to a non vertical output (key/value pairs) and instead places keys in header. #91

This PR removes the _wrapArray function which was added in 2017 and parses the chunk directly.

Steps to Test or Reproduce

Create a file in the main dir complex.json using the code below, run npm install then npm run build:dist to build lib, then run ./bin/jsonexport.js complex.json to see stdOut

{
  "complex": {
    "object": 1,
    "that": 2
  },
  "has": 3,
  "nested": {
    "keys": true
  }
}

Impacted Areas in Application

  • CLI

@ChazUK
Copy link
Author

ChazUK commented Feb 4, 2021

You might also want to consider using prettier to avoid mismatches of code styles.

@AckerApple
Copy link
Collaborator

Acknowledged.

You might also want to consider...

My own opinion: Wez a team of anybody and everybody. Please feel free to issue PR to adopt prettier. I imagine we’d debate and ultimately deem it too much and not necessary. I could be wrong. Be assured I use prettier and lint in every project however out in the open source, especially for a project in motion, it can be a tall feat to enforce such as thing

@ChazUK
Copy link
Author

ChazUK commented Feb 22, 2021

@kaue any chance you could take a look at this?

@kaue
Copy link
Owner

kaue commented Sep 29, 2021

Sure @ChazUK, sorry for the delay, i will take a look ASAP

@kaue
Copy link
Owner

kaue commented Sep 29, 2021

In order to merge this PR we first need to make sure the stream logic is still working the way it was before.

The issue here is that if we receive a big json file, we will read it in chunks, and those chunks may or may not contain an entire JSON object, or a partial one, so in order to parse we have to exclude those parcial objects and wait for the next chunk.

I hope i was clear enough to explain the issue in removing the split / join.

@kaue
Copy link
Owner

kaue commented Sep 29, 2021

So i would just test this branch with a big JSON file containing an array with multiple objects using the jsonexport stream API

@ChazUK
Copy link
Author

ChazUK commented Sep 29, 2021

I'll see if I can setup a test for that. Thanks for the response

@ChazUK ChazUK closed this Jun 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants