PapaParse icon indicating copy to clipboard operation
PapaParse copied to clipboard

Generate headers in `Parser`, based upon presence of `_fields`, without changing `_input` (fixes issue #985)

Open landisdesign opened this issue 1 year ago • 4 comments

Fixes issue #985.

Previously, headers were created in ParseHandle's processResults(), but also managed by Parser.parse() when baseIndex was falsy.

When headers were requested and baseIndex was false, parse() would modify _input without adjusting cursor, causing the parser to mistake where it needed to continue parsing. This would cause further rows to be shifted. In addition, baseIndex could be undefined or 0, depending on how it was called, causing this cursor shift to potentially happen twice.

This PR:

  • Updates _fields based upon the value of _results.meta.fields without changing _input
  • Uses _fields.length as the source of truth for whether or not headers should be generated when config.header is truthy
  • Consolidates header generation within Parser by passing _fields to the Parser constructor where the above test is performed
  • Uses the same parsing algorithms used for row generation for headers
  • Adds fields to results generated by Parser.parse() based upon whether or not headers are available

Additional impacts:

  • Because header generation is now performed within pushRow(), the header row is no longer returned as data by Parser.parse(). Instead, it is returned in meta.fields by returnable(). This also means we don't need to adjust config.preview to account for the header row.

landisdesign avatar Mar 24 '23 17:03 landisdesign

🎉

beckyconning avatar Apr 14 '23 13:04 beckyconning

Hey @landisdesign

Can't thank you enough.

PapaParse basically didn't work from in a stream pipeline without this.

You are a life saver.

beckyconning avatar Apr 14 '23 14:04 beckyconning

@mholt is there a different plan to resolve this issue? it seems quite pressing and this pr seems to resolve it nicely.

beckyconning avatar Apr 20 '23 12:04 beckyconning

As long as it works, and the tests pass, this is OK with me -- will probably defer to @pokoli for a closer look though. Ping me again if it isn't merged in a while.

mholt avatar Apr 20 '23 16:04 mholt