From 2a995a3a215e16cf1d3333ed0fe72e46e303479f Mon Sep 17 00:00:00 2001 From: kogai Date: Sun, 13 Sep 2026 07:43:28 +0000 Subject: [PATCH 1/2] Stop the TypeScript reader coercing ONIX values to numbers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fast-xml-parser converts anything that looks numeric, which corrupts ONIX data. Measured: default {"NotificationType":1,"IDValue":62124983,"Price":1200.5} parseTagValue:false {"NotificationType":"01","IDValue":"062124983","Price":"1200.50"} The code "01" becomes 1, an identifier loses its leading zero, and a price loses a digit. None of this is new in v5 — v3 did the same — so it has been wrong from the start. The generated code already disagreed with it: code.ts declares `export type NotificationType = string` while the reader returned a number, so the types were lying. The Go templates generate string types from the same schema, so the two languages also disagreed with each other on identical input, which is the one thing a multi-language generator should not do. Set parseTagValue: false. Every ONIX element value is a string; the numeric-looking ones are codes, identifiers and amounts that happen to be made of digits. Fixing this per-element was rejected in ADR-0007: which elements look numeric depends on the data, so IDValue would convert or not depending on the value, and a type that varies with input is worse than one that is always a string. This does change output for consumers, from number to string. Recorded as such — the side that breaks is the side that was wrong. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01P8rZakwAiz1jpViUX34x1Z --- ...07-do-not-coerce-onix-values-to-numbers.md | 65 +++++++++++++++++++ docs/adr/README.md | 1 + generated/typescript/v2/reader.ts | 17 +++-- template/typescript/v2/reader.mustache | 17 +++-- 4 files changed, 90 insertions(+), 10 deletions(-) create mode 100644 docs/adr/0007-do-not-coerce-onix-values-to-numbers.md diff --git a/docs/adr/0007-do-not-coerce-onix-values-to-numbers.md b/docs/adr/0007-do-not-coerce-onix-values-to-numbers.md new file mode 100644 index 0000000..8f37728 --- /dev/null +++ b/docs/adr/0007-do-not-coerce-onix-values-to-numbers.md @@ -0,0 +1,65 @@ +# ADR-0007: TypeScript reader で値の型変換を行わない + +- **ステータス**: Accepted +- **日付**: 2026-09-13 +- **関連**: ADR-0005 が既知の問題として記録していた件への対応 + +## 背景 + +fast-xml-parser は既定で、要素の値が数値に見えればその型に変換する。ONIX のデータに +これを適用すると壊れる。実際に測った結果が次である。 + +``` +既定 {"NotificationType":1,"ProductIDType":2,"IDValue":62124983, + "ISBN13":9784062124983,"Price":1200.5} +parseTagValue:false {"NotificationType":"01","ProductIDType":"02","IDValue":"062124983", + "ISBN13":"9784062124983","Price":"1200.50"} +``` + +- `NotificationType` の `"01"` が `1` になる。ONIX のコード値は先頭のゼロを含めて意味を持つ + 2 桁の文字列であり、`1` は別物である。 +- `IDValue` の `"062124983"` が `62124983` になる。識別子から先頭ゼロが落ちている。 +- `Price` の `"1200.50"` が `1200.5` になる。金額の桁が落ちている。 +- `ISBN13` は 13 桁なので値としては保たれるが、型が `number` になる。 + +これは fast-xml-parser v5 で入った挙動ではない。v3 でも同じで、`parseTrueNumberOnly: false` +と v5 の `numberParseOptions.leadingZeros: true` は同じ結果を出す。つまり**最初から壊れていた**。 + +生成されるコードはこの挙動と矛盾している。`generated/typescript/v2/code.ts` は +コード型を文字列として宣言している。 + +```ts +export type NotificationType = string +``` + +宣言は `string`、実際に返るのは `number`。型が嘘をついている状態だった。 + +Go 側の生成コードは、同じスキーマから一貫して文字列型を生成している +(`type {{xmlReferenceName}} string`)。同じ入力に対して言語ごとに違う型が返るのは、 +「同じスキーマから複数言語のクライアントを生成する」というこのリポジトリの目的 +(AGENTS.md) に照らして不整合である。 + +## 決定 + +`parseTagValue: false` を指定し、要素の値を一切変換しない。 + +## 理由 + +ONIX のスキーマにおいて、要素の値はすべて文字列である。数値に見えるものも、 +コード値・識別子・金額といった「たまたま数字で構成された文字列」であって、数値ではない。 +パーサに推測させる余地は無い。 + +個別に対処する案 (コード型だけ文字列に戻す、など) は採らない。どの要素が数値に見えるかは +入力データ次第で、`IDValue` のように値によって変換されたりされなかったりする。 +入力に依存して型が変わるほうが、常に文字列であるより扱いにくい。 + +## 結果 + +- 生成される TypeScript reader の出力は、これまで数値だった値が文字列になる。 + **これは後方互換性を壊す変更である。** ただし、壊れる側が正しい出力なので受け入れる。 + 生成される型宣言 (`= string`) と実際の値がこれで一致する。 +- 属性の扱いはこの ADR の範囲外。fast-xml-parser v5 は既定で `ignoreAttributes: true` なので、 + `refname` / `shortname` / `datestamp` といった ONIX の属性は現在そもそも読まれていない。 + これも実装上の欠落だが、修正すると出力の形が変わるため別途扱う。 +- TypeScript reader は CI で実行されていない。この ADR の測定は手作業であり、 + 回帰を検出する仕組みは無い。 diff --git a/docs/adr/README.md b/docs/adr/README.md index 8409bf1..09f0314 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -9,6 +9,7 @@ | [0001](0001-record-architecture-decisions.md) | 設計判断を ADR として記録する | Accepted | | [0002](0002-decouple-unit-tests-from-the-vendored-schema.md) | ユニットテストを取得済みスキーマから切り離す | Accepted | | [0005](0005-fast-xml-parser-v5-behaviour-changes.md) | fxp v5 のエンティティ展開を受け入れる | Accepted | +| [0007](0007-do-not-coerce-onix-values-to-numbers.md) | ONIX の値を数値に変換しない | Accepted | ## 書き方 diff --git a/generated/typescript/v2/reader.ts b/generated/typescript/v2/reader.ts index 0c3c86f..030f1c5 100644 --- a/generated/typescript/v2/reader.ts +++ b/generated/typescript/v2/reader.ts @@ -2,12 +2,19 @@ import { promises as fs } from "fs"; import { XMLParser } from "fast-xml-parser"; import { ONIXMessage } from "./model" -// v5 surfaces the XML declaration and any other processing instruction as -// `?name` keys, which v3 did not; both are suppressed to keep the parsed shape -// to the document's own elements. Note that v5 also decodes entity references -// (v3 returned "&" verbatim) — that difference is deliberate, see +// Every ONIX element value is a string, and the generated types say so, so the +// parser must not coerce: without parseTagValue: false it turns the code "01" +// into 1 and the price "1200.50" into 1200.5. +// +// ignoreDeclaration and ignorePiTags keep the parsed shape to the document's +// own elements. Note that v5 decodes entity references where v3 did not; that +// change is deliberate, see // docs/adr/0005-fast-xml-parser-v5-behaviour-changes.md -const parser = new XMLParser({ ignoreDeclaration: true, ignorePiTags: true }); +const parser = new XMLParser({ + ignoreDeclaration: true, + ignorePiTags: true, + parseTagValue: false, +}); export const read = async (input: string): Promise => { try { diff --git a/template/typescript/v2/reader.mustache b/template/typescript/v2/reader.mustache index 0c3c86f..030f1c5 100644 --- a/template/typescript/v2/reader.mustache +++ b/template/typescript/v2/reader.mustache @@ -2,12 +2,19 @@ import { promises as fs } from "fs"; import { XMLParser } from "fast-xml-parser"; import { ONIXMessage } from "./model" -// v5 surfaces the XML declaration and any other processing instruction as -// `?name` keys, which v3 did not; both are suppressed to keep the parsed shape -// to the document's own elements. Note that v5 also decodes entity references -// (v3 returned "&" verbatim) — that difference is deliberate, see +// Every ONIX element value is a string, and the generated types say so, so the +// parser must not coerce: without parseTagValue: false it turns the code "01" +// into 1 and the price "1200.50" into 1200.5. +// +// ignoreDeclaration and ignorePiTags keep the parsed shape to the document's +// own elements. Note that v5 decodes entity references where v3 did not; that +// change is deliberate, see // docs/adr/0005-fast-xml-parser-v5-behaviour-changes.md -const parser = new XMLParser({ ignoreDeclaration: true, ignorePiTags: true }); +const parser = new XMLParser({ + ignoreDeclaration: true, + ignorePiTags: true, + parseTagValue: false, +}); export const read = async (input: string): Promise => { try { From 7332d96488b8818472e441238b5c8b112c3dd949 Mon Sep 17 00:00:00 2001 From: kogai Date: Sun, 13 Sep 2026 07:54:39 +0000 Subject: [PATCH 2/2] Correct ADR-0007: the Go side does not agree either MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ADR argued the fix restores cross-language consistency because Go generates string types from the same schema. Review showed both halves of that are wrong, and checking confirmed it: generated/go/v2/code.go: 107 struct, 8 string, 2 []string `type X string` only comes out of the template when a code is neither space-separatable nor has attributes, and NotificationType — the ADR's own example — is a struct in Go. Worse for the argument, Go's UnmarshalXML swaps the code for its description: case "01": c.Body = `Early notification` So after this change TypeScript returns "01" and Go returns "Early notification". The languages still disagree; the fix does not touch that. That is a bigger design split — only Go expands codes — and belongs in its own decision. The justification is narrowed to the thing that does hold: "01" becoming 1 and "1200.50" becoming 1200.5 corrupts data, whatever Go does. The code change is unaffected. Also recorded: parseAttributeValue must stay false when attributes are eventually read, or the same bug returns on that side; the generated types agreeing with the values is a convention rather than something typecheck enforces, since every interface in model.ts is empty and code.ts is imported by nothing; and parseTagValue: false does not stop trimValues, which still strips surrounding whitespace. The AGENTS.md citation is replaced with README, since AGENTS.md is not on this branch. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01P8rZakwAiz1jpViUX34x1Z --- ...07-do-not-coerce-onix-values-to-numbers.md | 36 ++++++++++++++++--- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/docs/adr/0007-do-not-coerce-onix-values-to-numbers.md b/docs/adr/0007-do-not-coerce-onix-values-to-numbers.md index 8f37728..632345b 100644 --- a/docs/adr/0007-do-not-coerce-onix-values-to-numbers.md +++ b/docs/adr/0007-do-not-coerce-onix-values-to-numbers.md @@ -34,10 +34,28 @@ export type NotificationType = string 宣言は `string`、実際に返るのは `number`。型が嘘をついている状態だった。 -Go 側の生成コードは、同じスキーマから一貫して文字列型を生成している -(`type {{xmlReferenceName}} string`)。同じ入力に対して言語ごとに違う型が返るのは、 -「同じスキーマから複数言語のクライアントを生成する」というこのリポジトリの目的 -(AGENTS.md) に照らして不整合である。 +当初この ADR は「Go 側は一貫して文字列型を生成しているので、言語間で型が揃う」と +主張していたが、**これは誤りだった**。レビューで指摘され、確認した結果は次のとおり。 + +- `generated/go/v2/code.go` の型は **struct 107 / string 8 / []string 2** で、一様ではない。 + `template/go/v2/code.mustache` は 4 分岐あり、`type X string` になるのは + `spaceSeparatable` でも `hasElements` でもない場合だけである。 + この ADR が例に挙げている `NotificationType` は Go では struct である。 +- さらに Go の `UnmarshalXML` は、コード値を**人間可読な説明文に置換する**。 + +```go + switch v { + // Use for a complete record issued earlier than ... + case "01": + c.Body = `Early notification` +``` + +つまりこの修正を入れても、同じ入力に対して TypeScript は `"01"`、Go は +`"Early notification"` を返す。**言語間の一貫性は、この修正では回復しない。** +それは型の問題ではなく、Go 側だけがコードを説明文に展開しているという、より大きな +設計上の食い違いである。別途扱う。 + +したがってこの ADR の根拠は、言語間の一貫性ではなく**データを壊さないこと**の一点に絞る。 ## 決定 @@ -61,5 +79,15 @@ ONIX のスキーマにおいて、要素の値はすべて文字列である。 - 属性の扱いはこの ADR の範囲外。fast-xml-parser v5 は既定で `ignoreAttributes: true` なので、 `refname` / `shortname` / `datestamp` といった ONIX の属性は現在そもそも読まれていない。 これも実装上の欠落だが、修正すると出力の形が変わるため別途扱う。 + **その際は `parseAttributeValue` を既定の `false` のまま保つこと。** さもないと + 同じ型変換のバグが属性側で再発する。 +- 型宣言と値が一致する、と書いたが、それを型検査が強制するわけではない。 + `generated/typescript/v2/code.ts` はどこからも import されておらず、 + `reader.ts` の戻り値型 `ONIXMessage` は `model.ts` で空の interface + (`export interface ONIXMessage {}`) として定義されている。一致は規約であって、 + 検査で守られてはいない。 +- `parseTagValue: false` は数値化を止めるが、値の加工をすべて止めるわけではない。 + `trimValues` は既定で true のままなので前後の空白は落ちる。真偽値らしき文字列の + 変換も同時に止まる。 - TypeScript reader は CI で実行されていない。この ADR の測定は手作業であり、 回帰を検出する仕組みは無い。