jsonexport icon indicating copy to clipboard operation
jsonexport copied to clipboard

Bug: CLI incorrectly parsing JSON Object as JSON Array

Open ChazUK opened this issue 4 years ago • 7 comments

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. https://github.com/kaue/jsonexport/issues/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 avatar Feb 04 '21 18:02 ChazUK

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

ChazUK avatar Feb 04 '21 18:02 ChazUK

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

AckerApple avatar Feb 04 '21 19:02 AckerApple

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

ChazUK avatar Feb 22 '21 11:02 ChazUK

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

kaue avatar Sep 29 '21 09:09 kaue

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 avatar Sep 29 '21 09:09 kaue

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

kaue avatar Sep 29 '21 09:09 kaue

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

ChazUK avatar Sep 29 '21 09:09 ChazUK