You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Splitting this off from #776 as you asked, to settle the shape before touching code.
The concrete case
A key that is both merged and locally overridden throws when it's a non-string scalar, but works when it's a string:
base: &a {10: x}m:
<<: *a10: y
With the default !!map + mergeTag this throws duplicated mapping key; the same document with a string key resolves fine to {k: y}. The object map stringifies keys everywhere (addPair / has / get / keys all go through String(key)), but the constructor's duplicate check compares the constructed keys — number 10 from the explicit entry vs string "10" the merge brought in — so they don't line up and it reads as a real duplicate.
Your two questions
1. How often are non-string scalar keys needed? Rarely. The one real-world case is integer keys — status-code tables (200: OK), year/id-indexed config. Boolean/null keys are essentially never intentional. And even for integers the object map hands the value back under a string key anyway, so it's already degraded: you write 10, you read "10". Making the merge case stop throwing wouldn't make it correct, just consistent with the plain case.
2. Bug or feature? It's a limitation of representing a mapping as a plain JS object, not really a bug to patch away — and the project already has the right tool for full fidelity: realMapTag. I checked it against exactly this case:
load('base: &a {10: x}\nm:\n <<: *a\n 10: y',{schema: CORE_SCHEMA.withTags(mergeTag,realMapTag)})// m is Map { 10 => 'y' } — key stays a number, override applied, no stray "10"
It keeps the key's type, applies the override, no duplicate. So the full-key path already works, including with merge.
Prior art
This has come up before, and it points the same way:
Support YAML maps and sets to ES6/ES2015 Maps and Sets #329 (which gave us the ES6 Map tag) framed real Maps as the way to "support non-string keys and be convertible back to YAML" — i.e. the non-string-key path was always meant to be realMap, not the object map.
loading non-string keys with JSON_SCHEMA should error? #538 asked for the opposite of "make it work": a user wanted 1: 'one' under a JSON-ish schema to error rather than silently stringify to {'1': 'one'}. So there's real demand for the object map being explicit that it only reliably supports string keys, not for it to grow more key-coercion machinery.
Where I'd land
Matches your lean: treat the object !!map as reliably string-keys-only, document that, and point anyone needing typed keys at realMapTag. If you'd still want the object map to not throw here, the smallest honest fix is a consistency patch contained entirely in the map tag — reuse the String(key) identity the tag already applies so the override matches — without teaching the parser or constructor about key coercion.
On the current PR: it spreads into parser.ts, constructor.ts and a new tag-interface method, which is the upper-level leak you flagged. If we fix at all, I'll redo it as the tag-contained version; if we document-and-delegate, I'll close the PR and send a docs note instead.
Splitting this off from #776 as you asked, to settle the shape before touching code.
The concrete case
A key that is both merged and locally overridden throws when it's a non-string scalar, but works when it's a string:
With the default
!!map+mergeTagthis throwsduplicated mapping key; the same document with a string key resolves fine to{k: y}. The object map stringifies keys everywhere (addPair/has/get/keysall go throughString(key)), but the constructor's duplicate check compares the constructed keys — number10from the explicit entry vs string"10"the merge brought in — so they don't line up and it reads as a real duplicate.Your two questions
1. How often are non-string scalar keys needed? Rarely. The one real-world case is integer keys — status-code tables (
200: OK), year/id-indexed config. Boolean/null keys are essentially never intentional. And even for integers the object map hands the value back under a string key anyway, so it's already degraded: you write10, you read"10". Making the merge case stop throwing wouldn't make it correct, just consistent with the plain case.2. Bug or feature? It's a limitation of representing a mapping as a plain JS object, not really a bug to patch away — and the project already has the right tool for full fidelity:
realMapTag. I checked it against exactly this case:It keeps the key's type, applies the override, no duplicate. So the full-key path already works, including with merge.
Prior art
This has come up before, and it points the same way:
Maptag) framed realMaps as the way to "support non-string keys and be convertible back to YAML" — i.e. the non-string-key path was always meant to berealMap, not the object map.1: 'one'under a JSON-ish schema to error rather than silently stringify to{'1': 'one'}. So there's real demand for the object map being explicit that it only reliably supports string keys, not for it to grow more key-coercion machinery.Where I'd land
Matches your lean: treat the object
!!mapas reliably string-keys-only, document that, and point anyone needing typed keys atrealMapTag. If you'd still want the object map to not throw here, the smallest honest fix is a consistency patch contained entirely in the map tag — reuse theString(key)identity the tag already applies so the override matches — without teaching the parser or constructor about key coercion.On the current PR: it spreads into
parser.ts,constructor.tsand a new tag-interface method, which is the upper-level leak you flagged. If we fix at all, I'll redo it as the tag-contained version; if we document-and-delegate, I'll close the PR and send a docs note instead.