fix(web): stop the read-only viewer reformatting a payload into a different one
Copilot's tenth pass generated no new inline comments. Of its two suppressed
ones, this fixes the first and declines the second.
`ContentViewer` pretty-prints by `JSON.parse` then `JSON.stringify`, which is a
round trip through JavaScript's number type and is not lossless. A server
sending `{"id":9007199254740993}` was shown `…992`, and one sending
`{"limit":1e400}` was shown `null`. Neither is a formatting difference: both
are the viewer telling the reader something untrue about the payload it
received, which for an inspector is the worst kind of bug. It predates this PR
— `formatJson` has always round-tripped — but this PR is where the read-only
JSON path was rebuilt, and the same two predicates the draft parsers use answer
it exactly.
Both formatters now display the payload as it arrived when reindenting would
alter a value: unindented, but honest. The copy button already copied the
original text either way, so this makes the screen agree with it.
Not changed: a literal whose *spelling* is normalized but whose value is not.
`0.10000000000000001` and `0.1` are the same double, as are `1e2` and `100`, so
those still format. Rejecting them would mean refusing any literal not already
in JS's shortest form — `1.50`, `1e2`, `0.5000` — which blocks drafts that send
exactly what they mean. The integer guard stays because there the digits name a
value the double is not.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UL9retfvAXRgvi6EWk4SY4
Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
03bf9ece42
commit
390e0c45ed
2 changed files with 70 additions and 5 deletions
|
|
@ -103,6 +103,26 @@ describe("formatJson", () => {
|
|||
it("returns the input unchanged when invalid", () => {
|
||||
expect(formatJson("{ broken")).toBe("{ broken");
|
||||
});
|
||||
|
||||
// Pretty-printing is a round trip through JS's number type, which is not
|
||||
// lossless. Reformatting these would show the reader something other than
|
||||
// what the sender put on the wire — the worst kind of bug in an inspector.
|
||||
it("leaves a payload alone when reindenting would change a value", () => {
|
||||
// Would render as …992.
|
||||
expect(formatJson('{"id":9007199254740993}')).toBe(
|
||||
'{"id":9007199254740993}',
|
||||
);
|
||||
// `JSON.parse` yields Infinity, which `JSON.stringify` writes as null.
|
||||
expect(formatJson('{"limit":1e400}')).toBe('{"limit":1e400}');
|
||||
});
|
||||
|
||||
// The spelling of a number changes on a round trip and its *value* does not,
|
||||
// so these still get indented: `0.10000000000000001` and `0.1` are the same
|
||||
// double, and `1e2` and `100` are the same number.
|
||||
it("still formats a payload whose numbers only change spelling", () => {
|
||||
expect(formatJson('{"n":0.10000000000000001}')).toBe('{\n "n": 0.1\n}');
|
||||
expect(formatJson('{"n":1e2}')).toBe('{\n "n": 100\n}');
|
||||
});
|
||||
});
|
||||
|
||||
describe("looksLikeJson", () => {
|
||||
|
|
@ -122,6 +142,15 @@ describe("formatJsonDocument", () => {
|
|||
// The stricter half of the heuristic (#2151). Text that merely *starts* like
|
||||
// JSON must not be handed to the JSON editor: a note beginning `{` would then
|
||||
// be presented as a document, in a JSON gutter, framed as malformed.
|
||||
// Still a JSON document — it just does not get reindented when reindenting
|
||||
// would change what it says.
|
||||
it("returns the source unindented when reindenting would change a value", () => {
|
||||
expect(formatJsonDocument('{"id":9007199254740993}')).toBe(
|
||||
'{"id":9007199254740993}',
|
||||
);
|
||||
expect(formatJsonDocument('{"limit":1e400}')).toBe('{"limit":1e400}');
|
||||
});
|
||||
|
||||
it("reports text that only looks like JSON as not JSON", () => {
|
||||
expect(formatJsonDocument("{ broken")).toBeNull();
|
||||
expect(formatJsonDocument("[unclosed")).toBeNull();
|
||||
|
|
|
|||
|
|
@ -1,8 +1,13 @@
|
|||
/**
|
||||
* Pure helpers backing the per-MIME dispatch in {@link ContentViewer}. Kept in
|
||||
* a dependency-free module so they can be unit-tested in isolation and reused by
|
||||
* the blob renderers (PDF / CSV / HTML) without dragging in React.
|
||||
* Pure helpers backing the per-MIME dispatch in {@link ContentViewer}. Kept
|
||||
* free of component dependencies so they can be unit-tested in isolation and
|
||||
* reused by the blob renderers (PDF / CSV / HTML) without dragging in React.
|
||||
* The two pure predicates imported below are the exception that proves it:
|
||||
* plain functions over a string, shared so the viewer and the JSON editors
|
||||
* cannot disagree about which payloads survive a round trip.
|
||||
*/
|
||||
import { isSerializableJson } from "@inspector/core/json/jsonUtils.js";
|
||||
import { hasImpreciseIntegerLiteral } from "../../../utils/jsonObjectDraft";
|
||||
|
||||
/**
|
||||
* The renderer family a MIME type maps to. `ContentViewer` switches on this to
|
||||
|
|
@ -130,12 +135,38 @@ export function tryDecodeBase64ToBytes(
|
|||
/** Pretty-print JSON text; returns the input unchanged when it doesn't parse. */
|
||||
export function formatJson(content: string): string {
|
||||
try {
|
||||
return JSON.stringify(JSON.parse(content), null, 2);
|
||||
const parsed: unknown = JSON.parse(content);
|
||||
return reformatsFaithfully(content, parsed)
|
||||
? JSON.stringify(parsed, null, 2)
|
||||
: content;
|
||||
} catch {
|
||||
return content;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether pretty-printing this payload would show the reader the same values
|
||||
* the sender put on the wire.
|
||||
*
|
||||
* Pretty-printing here is `JSON.parse` then `JSON.stringify`, which is a
|
||||
* *round trip through JavaScript's number type* — and that is not lossless.
|
||||
* A server sending `{"id":9007199254740993}` would be shown `…992`, and one
|
||||
* sending `{"limit":1e400}` would be shown `null`, because `JSON.parse` yields
|
||||
* `Infinity` and `JSON.stringify` writes that as null. Neither is a formatting
|
||||
* difference; both are the viewer telling the reader something untrue about
|
||||
* the payload it received, which for an inspector is the worst kind of bug.
|
||||
*
|
||||
* So when the round trip would alter a value, the payload is displayed exactly
|
||||
* as it arrived — unindented, but honest. The copy button already copied the
|
||||
* original text either way; this makes what is on screen agree with it.
|
||||
*
|
||||
* The same two checks the draft parsers use, which is why they live in
|
||||
* `utils/jsonObjectDraft` rather than beside either caller.
|
||||
*/
|
||||
function reformatsFaithfully(text: string, parsed: unknown): boolean {
|
||||
return isSerializableJson(parsed) && !hasImpreciseIntegerLiteral(text);
|
||||
}
|
||||
|
||||
/** Heuristic: does this plain text (no MIME) look like a JSON document? */
|
||||
export function looksLikeJson(text: string): boolean {
|
||||
const trimmed = text.trimStart();
|
||||
|
|
@ -162,7 +193,12 @@ export function looksLikeJson(text: string): boolean {
|
|||
export function formatJsonDocument(text: string): string | null {
|
||||
if (!looksLikeJson(text)) return null;
|
||||
try {
|
||||
return JSON.stringify(JSON.parse(text), null, 2);
|
||||
const parsed: unknown = JSON.parse(text);
|
||||
// Still a JSON document either way — it just does not get reindented when
|
||||
// reindenting would change what it says. See `reformatsFaithfully`.
|
||||
return reformatsFaithfully(text, parsed)
|
||||
? JSON.stringify(parsed, null, 2)
|
||||
: text;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue