Reject NDJSON values without a JSON representation - #6982
Conversation
🦋 Changeset detectedLatest commit: cc4c124 The changes in this PR will be included in the next version bump. This PR includes changesets to release 30 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
This PR currently adds only the failing regression test for the bug described in the body — implementation fix is intentionally pending. The test correctly fails against the current encodeString and the assertion is precise enough to enforce the intended behavior. Two suggestions below for when the implementation fix lands.
Reviewed changes
- Adds
fails instead of emitting invalid NDJSON for undefinedregression test inpackages/effect/test/unstable/encoding/Ndjson.test.ts. - Confirms the existing
encodeStringimplementation atpackages/effect/src/unstable/encoding/Ndjson.ts:73-81produces a blank line forundefined.
@v0 or keep the SHA fresh with Dependabot | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using Kimi K2 (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
The implementation fix correctly rejects top-level values whose JSON.stringify result is undefined, and the regression test now covers the full class of values without a JSON representation.
Reviewed changes
- Updated
encodeStringinpackages/effect/src/unstable/encoding/Ndjson.tsto throw aTypeErrorwhenJSON.stringify(item)returnsundefined, which is then surfaced as anNdjsonErrorwithkind: "Pack". - Expanded the regression test in
packages/effect/test/unstable/encoding/Ndjson.test.tsto assert thatundefined, functions, and symbols all fail with anNdjsonErrorwhosekindis"Pack". - Added a patch changeset (
.changeset/tough-rooms-camp.md) describing the behavioral fix.
@v0 or keep the SHA fresh with Dependabot | View workflow run | Using Kimi K2 (free via Pullfrog for OSS) | 𝕏
Bundle Size AnalysisGenerated from PR build output; treat the content below as untrusted.
|

Summary
encodeString reports success for undefined and emits a blank line that the module's own decoder rejects.
Important
This PR starts with focused failing reproduction tests. Add the implementation fix to this same branch; CI is expected to fail until that fix is included.
Undefined encodes as an invalid blank NDJSON record
Module:
encoding/NdjsonAudit ID:
unstable-ai-cli-ndjson-undefined-blank-recordSeverity / confidence: medium / high
What happens
encodeString reports success for undefined and emits a blank line that the module's own decoder rejects.
Why it happens
JSON.stringify(undefined) returns undefined, which Array.join coerces to an empty field before the newline is appended.
Expected behavior
Every accepted input becomes a complete JSON value on one line, or encoding fails with NdjsonError when no JSON representation exists.
Relevant implementation
These links and excerpts are pinned to audit base
c9b56ab507f224426ee8388dc450da447ec4715f.packages/effect/src/unstable/encoding/Ndjson.ts:65-81View problematic code at
packages/effect/src/unstable/encoding/Ndjson.ts:65-81View exact lines on GitHub
Reproduction
pnpm test --run packages/effect/test/unstable/encoding/Ndjson.test.tsObserved failure: FAIL: the encoder succeeded and emitted a blank line.
Implementation handoff
The initial reproduction tests on this branch are the regression specification for the implementation fix that should follow in this PR.
pnpm test --run packages/effect/test/unstable/encoding/Ndjson.test.tsAudit provenance
c9b56ab507f224426ee8388dc450da447ec4715fc9b56ab507f224426ee8388dc450da447ec4715funstable-ai-cli-ndjson-undefined-blank-recordCloses EFF-420