docs(json): clarify suitability for NDJSON - #7205
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7205 +/- ##
========================================
Coverage 94.83% 94.84%
========================================
Files 617 617
Lines 51674 51422 -252
Branches 9350 9300 -50
========================================
- Hits 49007 48769 -238
+ Misses 2121 2112 -9
+ Partials 546 541 -5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
bartlomieju
left a comment
There was a problem hiding this comment.
Thanks — the second half of this is a genuinely useful clarification and I'd like to land it. Neither JsonParseStream nor ConcatenatedJsonParseStream parses incrementally inside an array, and people do arrive expecting otherwise, so calling that out at module level is the right instinct.
Two corrections needed on the rest of the wording, both inline. The short version: one of the new sentences is true for one of the two parse streams but not the other, and the format list ended up narrower than what the per-symbol docs already promise.
| * Utilities for parsing streaming JSON data. | ||
| * Utilities for parsing streaming JSON data (newline-delimited | ||
| * JSON Lines / NDJSON, or concatenated JSON). Each chunk must | ||
| * be a complete JSON value. Not suited for streaming elements |
There was a problem hiding this comment.
"Each chunk must be a complete JSON value" holds for JsonParseStream, but not for ConcatenatedJsonParseStream, which explicitly buffers across chunk boundaries. concatenated_json_parse_stream_test.ts:231-235 asserts that ["{", '\"foo\": \"bar\"}'] parses fine — a chunk that is not a complete JSON value.
As a module-level statement it therefore contradicts the documented behaviour of one of the module's own exports. It's also the reason JsonParseStream's docs pair it with TextLineStream while the concatenated one doesn't need to.
Scoping it fixes the whole problem — something like "JsonParseStream requires each chunk to be a complete JSON value; ConcatenatedJsonParseStream buffers across chunk boundaries."
|
|
||
| /** | ||
| * Utilities for parsing streaming JSON data. | ||
| * Utilities for parsing streaming JSON data (newline-delimited |
There was a problem hiding this comment.
The format list drops JSON Text Sequences (RFC 7464), which both parse_stream.ts:16 and stringify_stream.ts:24 document as supported. Since the point of this PR is to make the summary more informative, narrowing it below what the symbol docs already claim works against that.
Small wording nit while you're here: "newline-delimited JSON Lines / NDJSON" says newline-delimited twice — "JSON Lines (NDJSON)" covers it.
Also worth noting the module exports JsonStringifyStream and canonicalize too, so "Utilities for parsing streaming JSON data" undersells it — not introduced by this PR, but you're right here if you feel like fixing it.
Refrained from recommending any particular alternative since I haven't found one with Deno-level DX.