From c20cbd03d2e930042895c226886eff6a3cf42b1a Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 19 Sep 2026 07:24:35 +0000 Subject: [PATCH] fix(plugin-list,plugin-view): deliver an authored map `style` as `mapStyle` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ObjectMapConfigSchema` declares `style`, and both view flatteners dropped it before the renderer saw it: a view authoring `map: { style: '' }` parsed green and the map painted MapLibre's public demo tiles (objectui#9950). Each flattener's whitelist becomes a TOTAL spelling table, `FLAT_MAP_CONFIG_SPELLING`, mapping every key the declaration carries to the name the internal flat form uses. Identity everywhere except `style`, which travels as `mapStyle` — the spelling `getMapConfig` reads first (`schema.mapStyle || schema.map?.style`) and a declared member of `ObjectMapSchema`. The top-level `style` namespace stays the base face's inline CSS, so the collision objectui#5177 closed stays closed, and an undeclared key in the block still never reaches the product. Both anti-drift pins were green while the key was dropped: each compared its hand list against `Object.keys(ObjectMapConfigSchema.shape).filter((key) => key !== 'style')`, a comparison set narrowed by the same subtraction the defect was made of. They now assert the relation "every declared key is delivered under its flat spelling" against the declaration read whole, with a control in the same body that feeds the pre-fix whitelist to that assertion and shows it rejected. The table's `satisfies Record` makes the coverage a typecheck failure as well. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_018HrVaotisyhgmot9o2MLRq --- .../9950-map-style-reaches-flat-renderer.md | 42 ++++ packages/plugin-list/src/ListView.tsx | 82 ++++--- .../__tests__/ListView.mapFlatten.test.tsx | 198 +++++++++++++++-- packages/plugin-view/src/ObjectView.tsx | 82 ++++--- .../__tests__/ObjectView.mapFlatten.test.tsx | 203 +++++++++++++++--- 5 files changed, 515 insertions(+), 92 deletions(-) create mode 100644 .changeset/9950-map-style-reaches-flat-renderer.md diff --git a/.changeset/9950-map-style-reaches-flat-renderer.md b/.changeset/9950-map-style-reaches-flat-renderer.md new file mode 100644 index 0000000000..69e2f83ed0 --- /dev/null +++ b/.changeset/9950-map-style-reaches-flat-renderer.md @@ -0,0 +1,42 @@ +--- +'@object-ui/plugin-list': patch +'@object-ui/plugin-view': patch +--- + +Deliver an authored map `style` on the list-view and object-view paths (objectui#9950). + +`ObjectMapConfigSchema` declares `style` ("MapLibre style URL/spec (overrides the +public demo default)"), and both view flatteners dropped it before the renderer ever +saw it. A view authoring `map: { style: 'https://…/style.json' }` parsed green, nothing +refused it, nothing warned, and the map rendered on MapLibre's **public demo tiles**. + +Each flattener's key whitelist is now a TOTAL spelling table, +`FLAT_MAP_CONFIG_SPELLING`, mapping every key `ObjectMapConfigSchema` declares to the +name the internal flat form uses for it. Every entry is the identity except `style`, +which is delivered as `mapStyle`. + +`mapStyle`, not `style`, because the top-level `style` key is `BaseSchema.style` — +inline CSS, legal on every node. Collapsing the two namespaces onto one key is the +defect objectui#5177 closed and this change deliberately keeps closed: the flatten +still never writes a top-level `style`. `mapStyle` is a declared member of +`ObjectMapSchema` and is the first spelling `getMapConfig` reads +(`schema.mapStyle || schema.map?.style`), so the authored style now reaches the +renderer without inventing an undeclared transport key. + +An undeclared key in the `map` block still never reaches the product — that half of +objectui#5177 is unchanged and pinned in both packages' `mapFlatten` suites. + +**Why the anti-drift pins did not catch this.** Both sites already pinned their hand +list against the declaration, and both pins were green. They compared the list against +`Object.keys(ObjectMapConfigSchema.shape).filter((key) => key !== 'style')` — the +comparison set had the same key subtracted from it that the whitelist was missing, so +the pin agreed with the omission. Those pins now measure the relation "every declared +key is delivered, under its flat spelling" against the declaration read whole, and each +suite carries a control that feeds the pre-fix whitelist to the same assertion and +shows it rejected. The spelling table is additionally a `Record` over every +`keyof ObjectMapConfig`, so a key added to the declaration fails `tsc` until it is +given a flat spelling. + +**Migration.** None. Views that never authored `map.style` are byte-identical in +behaviour; views that did now render with the style they declared instead of the +public demo tiles. diff --git a/packages/plugin-list/src/ListView.tsx b/packages/plugin-list/src/ListView.tsx index 6d677861f7..85b21312fc 100644 --- a/packages/plugin-list/src/ListView.tsx +++ b/packages/plugin-list/src/ListView.tsx @@ -36,11 +36,22 @@ import { usePermissions } from '@object-ui/permissions'; /** * The `case 'map'` branch below builds an `object-map` schema by flattening - * `schema.options.map`'s CONTENTS to the top level. Whitelisted to these keys — - * `ObjectMapConfigSchema`'s shape minus `style` — rather than the whole bag: - * `style` is ALSO `BaseSchema.style` (inline CSS, legal on every node), and - * spreading the raw `map` block collapsed the two namespaces onto one key - * (objectui#5177). + * `schema.options.map`'s CONTENTS to the top level — one entry per key + * `ObjectMapConfigSchema` declares, written under the name the FLAT form uses + * for that key. Whitelisted rather than a whole-bag spread: `style` is ALSO + * `BaseSchema.style` (inline CSS, legal on every node), and spreading the raw + * `map` block collapsed the two namespaces onto one key (objectui#5177). That + * reason is unchanged, and so is its consequence: a key the declaration does + * NOT carry never reaches the product. + * + * `style` IS delivered (objectui#9950) — under its flat spelling `mapStyle`, + * NOT by widening the whitelist to let `style` through unrenamed. + * `getMapConfig` in `ObjectMap.tsx` reads `schema.mapStyle || schema.map?.style` + * and deliberately does NOT read a top-level `style`, because that key is the + * base face's inline CSS (objectui#5017). `mapStyle` is itself a declared + * member of `ObjectMapSchema`, so the flat product stays inside the declaration + * at both ends. Before this, a view authoring `map: { style: '' }` parsed + * green, was dropped here, and the map painted the PUBLIC DEMO TILES. * * HAND-LISTED, not derived at runtime — deliberately, and only here (`plugin- * map`'s own `FLAT_MAP_CONFIG_KEYS` in `ObjectMap.tsx` DOES derive from @@ -57,28 +68,50 @@ import { usePermissions } from '@object-ui/permissions'; * gets away with the runtime import only because nothing in * console-starter's graph reaches `@object-ui/plugin-map` today. * - * Anti-drift is a TEST, not this comment: `ListView.mapFlatten.test.tsx` pins - * this exact list against `ObjectMapConfigSchema.shape` — imported only from - * that TEST file, which the alias-closure walker explicitly excludes from - * traversal — so a key added to or removed from the declaration still fails - * here, loudly and by name, without reintroducing the runtime edge that - * breaks the walker. + * Anti-drift is TWO mechanisms, neither of them this comment: + * - the type below is TOTAL — a `Record` over EVERY `keyof ObjectMapConfig`, + * not a list of some of them — so a key added to the declaration fails + * `tsc` here until it is given a flat spelling. A key can no longer be + * left out by simply not being written down, which is how `style` was. + * - `ListView.mapFlatten.test.tsx` pins this object's key set against + * `ObjectMapConfigSchema.shape` — imported only from that TEST file, which + * the alias-closure walker explicitly excludes from traversal — and asserts + * the RELATION (every declared key is delivered under its flat spelling). + * The pre-#9950 pin could not see the omission because it compared the hand + * list against `shape` MINUS `style`: the set it measured against was + * narrowed by the same subtraction the defect was made of, so it stayed + * green while an authored style was being discarded. + */ +export const FLAT_MAP_CONFIG_SPELLING = { + latitudeField: 'latitudeField', + longitudeField: 'longitudeField', + locationField: 'locationField', + titleField: 'titleField', + descriptionField: 'descriptionField', + zoom: 'zoom', + center: 'center', + // The one key whose flat spelling differs from its declared name — see the + // objectui#9950 paragraph above for why it is `mapStyle` and not `style`. + style: 'mapStyle', +} as const satisfies Record; + +/** + * Copy the declared map keys an author actually wrote onto the flat product, + * each under its flat spelling. + * + * Values travel AS WRITTEN: this is transport, not a second validation of the + * declared block — that reading belongs to `getMapConfig` in `ObjectMap.tsx` + * and stays there (objectui#5018). Discarding an ill-typed value here would + * reintroduce exactly the silent drop objectui#9950 closed. */ -export const FLAT_MAP_CONFIG_KEYS = [ - 'latitudeField', - 'longitudeField', - 'locationField', - 'titleField', - 'descriptionField', - 'zoom', - 'center', -] as const satisfies readonly (keyof Omit)[]; - -/** Pick only the declared flat map keys present on an authored `map` block. */ function pickFlatMapConfig(mapConfig: unknown): Record { if (!mapConfig || typeof mapConfig !== 'object') return {}; const source = mapConfig as Record; - return Object.fromEntries(FLAT_MAP_CONFIG_KEYS.filter((key) => key in source).map((key) => [key, source[key]])); + return Object.fromEntries( + Object.entries(FLAT_MAP_CONFIG_SPELLING) + .filter(([declared]) => declared in source) + .map(([declared, flat]) => [flat, source[declared]]), + ); } /** @@ -3108,7 +3141,8 @@ export const ListView = React.forwardRef(({ }; } case 'map': { - // Whitelisted flatten (objectui#5177) — see `FLAT_MAP_CONFIG_KEYS`. + // Whitelisted flatten (objectui#5177) — see `FLAT_MAP_CONFIG_SPELLING`, + // which also carries `style` out as `mapStyle` (objectui#9950). // `schema.options.map` is an untyped bag; a raw spread here forwarded // every key the author wrote, including `style`, which `ObjectMap`'s // `FlatMapConfigKeys` declares OUT of this flat form. diff --git a/packages/plugin-list/src/__tests__/ListView.mapFlatten.test.tsx b/packages/plugin-list/src/__tests__/ListView.mapFlatten.test.tsx index 07f7dc5239..aca117b5df 100644 --- a/packages/plugin-list/src/__tests__/ListView.mapFlatten.test.tsx +++ b/packages/plugin-list/src/__tests__/ListView.mapFlatten.test.tsx @@ -16,23 +16,33 @@ * legal on every node), so a `map: { style: '' }` authoring intent * arrived at the top level as that CSS-shaped key. * - * These pin the whitelist (`FLAT_MAP_CONFIG_KEYS`, hand-listed in + * These pin the whitelist (`FLAT_MAP_CONFIG_SPELLING`, hand-written in * `ListView.tsx` itself — see the comment on that constant for why it is not * derived from `ObjectMapConfigSchema` at runtime there) rather than the raw - * spread: a declared key still travels, `style` does not, and neither does an - * arbitrary undeclared one. The bottom of this file also pins the hand list - * against `ObjectMapConfigSchema` directly, so it cannot silently drift. - * Mirrors `ObjectView.mapFlatten.test.tsx`'s #5177 coverage for the sibling + * spread: a declared key still travels, an arbitrary undeclared one does not, + * and the top-level `style` namespace is never written. + * + * objectui#9950 — `style` is no longer DROPPED either. It is delivered under + * its flat spelling `mapStyle`, which is what `ObjectMap.getMapConfig` reads + * (`schema.mapStyle || schema.map?.style`) and a declared member of + * `ObjectMapSchema`; the top-level `style` it must not collide with stays the + * base face's inline CSS. The bottom of this file pins the spelling table + * against the WHOLE declaration and asserts the relation "every declared key + * is delivered" — the pin it replaces compared the hand list against + * `shape` MINUS `style`, so it was green exactly while an authored map style + * was being discarded before the renderer. + * + * Mirrors `ObjectView.mapFlatten.test.tsx`'s coverage for the sibling * flattener. */ import React from 'react'; import { describe, it, expect, vi, beforeEach } from 'vitest'; import { ComponentRegistry } from '@object-ui/core'; -import { render, waitFor } from '@testing-library/react'; -import { ListView, FLAT_MAP_CONFIG_KEYS } from '../ListView'; +import { render, waitFor, cleanup } from '@testing-library/react'; +import { ListView, FLAT_MAP_CONFIG_SPELLING } from '../ListView'; import { SchemaRendererProvider } from '@object-ui/react'; -// Test-only: `ListView.tsx` hand-lists `FLAT_MAP_CONFIG_KEYS` rather than +// Test-only: `ListView.tsx` hand-writes `FLAT_MAP_CONFIG_SPELLING` rather than // importing this schema at runtime, precisely so this module never appears in // `examples/console-starter`'s walked import graph (see the comment on the // constant). A test file is explicitly excluded from that walk, so pinning @@ -95,7 +105,7 @@ describe('ListView flattens `options.map` through a whitelist, not a raw spread expect(schema.longitudeField).toBe('lng'); }); - it('does NOT let `style` in the `map` block reach the top level — the specimen the card measured', async () => { + it('does NOT let `style` in the `map` block reach the TOP-LEVEL `style` — it travels as `mapStyle`', async () => { const schema = await mapSchemaFor({ latitudeField: 'lat', longitudeField: 'lng', @@ -103,8 +113,13 @@ describe('ListView flattens `options.map` through a whitelist, not a raw spread }); expect(schema.latitudeField).toBe('lat'); + // objectui#5177 — the top-level `style` namespace belongs to + // `BaseSchema.style` (inline CSS) and the flatten never writes it. expect(schema.style).toBeUndefined(); expect(Object.prototype.hasOwnProperty.call(schema, 'style')).toBe(false); + // objectui#9950 — and the authored map style is no longer discarded on + // the way: it arrives under the spelling `getMapConfig` reads. + expect(schema.mapStyle).toBe('https://tiles.example.com/style.json'); }); it('does NOT let an arbitrary undeclared key reach the top level', async () => { @@ -138,20 +153,163 @@ describe('ListView flattens `options.map` through a whitelist, not a raw spread }); /** - * `FLAT_MAP_CONFIG_KEYS` is hand-listed in `ListView.tsx` itself (not derived + * objectui#9950 — THE RELATION, replacing a pin that could not see the defect. + * + * The pin this supersedes read + * `Object.keys(ObjectMapConfigSchema.shape).filter((key) => key !== 'style')` + * and compared the hand list to THAT. The subtraction in the comparison set is + * the defect itself, spelled a second time: the list was measured against a + * declaration that had already had `style` removed from it, so the assertion + * was green precisely while an authored map style was dropped before the + * renderer and the map painted the public demo tiles. A pin narrowed to match + * the bug reports "the list is correct" to the next reader. + * + * What is asserted instead is a RELATION over the declaration read WHOLE: + * every key `ObjectMapConfigSchema` declares is delivered by the flatten, + * under its flat spelling. There is no literal key list here to narrow. + * + * The whitelist's own reason survives unchanged, in the file's own words + * (objectui#5177): `style` is ALSO `BaseSchema.style` — inline CSS, legal on + * every node — so a whole-bag spread put two namespaces on one key. The map + * style is therefore delivered as `mapStyle`: the spelling + * `ObjectMap.getMapConfig` actually reads (`schema.mapStyle || schema.map?.style`, + * never a top-level `style` — objectui#5017) and a declared member of + * `ObjectMapSchema`. Honouring the declaration and keeping the collision shut + * are not in tension; the old flattener just did neither for this key. + */ +describe('every declared ObjectMapConfigSchema key is delivered by the flatten (objectui#9950)', () => { + /** The declaration, read WHOLE. Nothing is filtered out of it here. */ + const declared = Object.keys(ObjectMapConfigSchema.shape); + + /** + * One value per declared key, chosen by asking the DECLARATION which shapes + * it accepts rather than hand-listing a type per key. A key added later with + * a shape none of these fit fails loudly here instead of being skipped. + */ + const CANDIDATE_VALUES: unknown[] = ['probe-value', 7, [1, 2]]; + function sampleFor(key: string): unknown { + const member = (ObjectMapConfigSchema.shape as Record { success: boolean } }>)[key]; + for (const candidate of CANDIDATE_VALUES) { + if (member.safeParse(candidate).success) return candidate; + } + throw new Error(`No sample value is accepted by ObjectMapConfigSchema.${key} — extend CANDIDATE_VALUES`); + } + + /** An authored `map` block carrying EVERY declared key, legal by the declaration. */ + const authoredEverything: Record = Object.fromEntries(declared.map((key) => [key, sampleFor(key)])); + + /** Which declared keys did NOT arrive, under their flat spelling, carrying the authored value. */ + function undeliveredKeys(product: Record): string[] { + return declared.filter((key) => { + const flat = (FLAT_MAP_CONFIG_SPELLING as Record)[key]; + if (!flat || !(flat in product)) return true; + return JSON.stringify(product[flat]) !== JSON.stringify(authoredEverything[key]); + }); + } + + /** + * The PRE-FIX flattener, reproduced: the hand list narrowed by the very + * `key !== 'style'` subtraction the old pin also used on its comparison set. + */ + function preFixFlatten(source: Record): Record { + const preFixKeys = declared.filter((key) => key !== 'style'); + return Object.fromEntries(preFixKeys.filter((key) => key in source).map((key) => [key, source[key]])); + } + + it('the probe config is legal by the declaration, and the declaration does carry `style`', () => { + // The card's shape: an authored map style PARSES GREEN. If this stopped + // being true the relation below would be measuring an illegal document. + expect(ObjectMapConfigSchema.safeParse(authoredEverything).success).toBe(true); + expect(declared).toContain('style'); + }); + + it('delivers every declared key, from a config carrying every declared key', async () => { + const schema = await mapSchemaFor(authoredEverything); + + expect(undeliveredKeys(schema)).toEqual([]); + // The map style specifically: it reaches the renderer under the spelling + // `getMapConfig` reads, and the top-level CSS namespace stays untouched. + expect(schema.mapStyle).toBe(authoredEverything.style); + expect(Object.prototype.hasOwnProperty.call(schema, 'style')).toBe(false); + }); + + it('CONTROL — the same assertion REJECTS the pre-fix flatten, so the instrument can fail', () => { + const preFixProduct = preFixFlatten(authoredEverything); + + // Named, not merely non-empty: the one key the pre-fix whitelist drops. + expect(undeliveredKeys(preFixProduct)).toEqual(['style']); + expect(() => expect(undeliveredKeys(preFixProduct)).toEqual([])).toThrow(); + }); + + it('CONTROL — the narrowed `declared` set is what let the old pin pass', () => { + // Reproduced from the pin this file replaces. Against THIS set the + // pre-fix hand list was a perfect match, which is why it was green. + const narrowed = declared.filter((key) => key !== 'style').sort(); + + expect(narrowed).not.toContain('style'); + expect(preFixFlatten(authoredEverything)).not.toHaveProperty('style'); + expect(Object.keys(FLAT_MAP_CONFIG_SPELLING).sort()).not.toEqual(narrowed); + }); + + it('a config authoring ONLY `style` still delivers it — the single-key case a whitelist drops silently', async () => { + const schema = await mapSchemaFor({ style: 'https://tiles.example.com/only.json' }); + + expect(schema.mapStyle).toBe('https://tiles.example.com/only.json'); + expect(Object.prototype.hasOwnProperty.call(schema, 'style')).toBe(false); + }); + + it('forwards NO key the declaration does not carry — objectui#5177 still holds', async () => { + // The keys the `map` block CONTRIBUTES to the product: what a fully + // authored block adds over what an empty one produces. That set must be + // exactly the image of the spelling table — nothing undeclared added, + // nothing declared missing. Both directions, one assertion. + // ⛔ `cleanup()` between the two renders is load-bearing, not tidiness: the + // spy collects from EVERY mounted tree, so a second `render` in one test + // leaves the first one free to re-render and append, and the baseline read + // below would then be the other config's product (measured: `contributed` + // came back empty, i.e. the assertion silently compared a product to + // itself). + const withNothing = await mapSchemaFor({}); + const baselineKeys = Object.keys(withNothing); + cleanup(); + + const withEverything = await mapSchemaFor({ + ...authoredEverything, + totallyUndeclaredKey: 'nope', + style2: 'nope', + // ⛔ `mapStyle` is the flat OUTPUT spelling, not a declared member of + // `ObjectMapConfigSchema` — writing it INSIDE the block is undeclared + // authoring and must not travel. The whitelist is keyed on declared + // SOURCE names; only `style` produces the `mapStyle` output below. + mapStyle: 'nope', + }); + const contributed = Object.keys(withEverything).filter((key) => !baselineKeys.includes(key)); + + expect(contributed.sort()).toEqual([...Object.values(FLAT_MAP_CONFIG_SPELLING)].sort()); + expect(Object.prototype.hasOwnProperty.call(withEverything, 'totallyUndeclaredKey')).toBe(false); + expect(Object.prototype.hasOwnProperty.call(withEverything, 'style2')).toBe(false); + expect(withEverything.mapStyle).toBe(authoredEverything.style); + }); +}); + +/** + * `FLAT_MAP_CONFIG_SPELLING` is hand-written in `ListView.tsx` itself (not derived * at runtime — see the comment on the constant for why), so THIS is the - * mechanism that keeps it from silently drifting off `ObjectMapConfigSchema` - * — the same role `packages/core/src/actions/__tests__/actionKeys.pin.test.ts` - * plays for `SPEC_ACTION_KEYS`. A key added to or removed from the schema - * fails this test by name, without requiring a runtime import of + * mechanism that keeps it from silently drifting off `ObjectMapConfigSchema` — + * the same role `packages/core/src/actions/__tests__/actionKeys.pin.test.ts` + * plays for `SPEC_ACTION_KEYS`. A key added to or removed from the schema fails + * this test by name, without requiring a runtime import of * `@object-ui/types/zod` from production code that reaches * `examples/console-starter`. + * + * The comparison set is the WHOLE declaration (objectui#9950). Nothing is + * filtered out of it — that filter is what made the previous version of this + * pin agree with the bug it was supposed to catch. */ -describe('FLAT_MAP_CONFIG_KEYS pins against ObjectMapConfigSchema (objectui#5177)', () => { - it('matches the schema minus `style`, so the hand list cannot silently drift', () => { - const declared = Object.keys(ObjectMapConfigSchema.shape) - .filter((key) => key !== 'style') - .sort(); - expect([...FLAT_MAP_CONFIG_KEYS].sort()).toEqual(declared); +describe('FLAT_MAP_CONFIG_SPELLING pins against ObjectMapConfigSchema (objectui#5177, objectui#9950)', () => { + it('covers the declaration in full, so the table cannot silently drift', () => { + const declared = Object.keys(ObjectMapConfigSchema.shape).sort(); + + expect(Object.keys(FLAT_MAP_CONFIG_SPELLING).sort()).toEqual(declared); }); }); diff --git a/packages/plugin-view/src/ObjectView.tsx b/packages/plugin-view/src/ObjectView.tsx index cdc06adab5..35c52ff096 100644 --- a/packages/plugin-view/src/ObjectView.tsx +++ b/packages/plugin-view/src/ObjectView.tsx @@ -78,11 +78,22 @@ const SchemaRendererComponent: React.FC = ImportedSchemaRenderer; /** * The `case 'map'` branch below builds an `object-map` schema by flattening - * `viewOptions.map`'s CONTENTS to the top level. Whitelisted to these keys — - * `ObjectMapConfigSchema`'s shape minus `style` — rather than the whole bag: - * `style` is ALSO `BaseSchema.style` (inline CSS, legal on every node), and - * spreading the raw `map` block collapsed the two namespaces onto one key - * (objectui#5177). + * `viewOptions.map`'s CONTENTS to the top level — one entry per key + * `ObjectMapConfigSchema` declares, written under the name the FLAT form uses + * for that key. Whitelisted rather than a whole-bag spread: `style` is ALSO + * `BaseSchema.style` (inline CSS, legal on every node), and spreading the raw + * `map` block collapsed the two namespaces onto one key (objectui#5177). That + * reason is unchanged, and so is its consequence: a key the declaration does + * NOT carry never reaches the product. + * + * `style` IS delivered (objectui#9950) — under its flat spelling `mapStyle`, + * NOT by widening the whitelist to let `style` through unrenamed. + * `getMapConfig` in `ObjectMap.tsx` reads `schema.mapStyle || schema.map?.style` + * and deliberately does NOT read a top-level `style`, because that key is the + * base face's inline CSS (objectui#5017). `mapStyle` is itself a declared + * member of `ObjectMapSchema`, so the flat product stays inside the declaration + * at both ends. Before this, a view authoring `map: { style: '' }` parsed + * green, was dropped here, and the map painted the PUBLIC DEMO TILES. * * HAND-LISTED, not derived at runtime — deliberately, and only here (`plugin- * map`'s own `FLAT_MAP_CONFIG_KEYS` in `ObjectMap.tsx` DOES derive from @@ -99,28 +110,50 @@ const SchemaRendererComponent: React.FC = ImportedSchemaRenderer; * gets away with the runtime import only because nothing in * console-starter's graph reaches `@object-ui/plugin-map` today. * - * Anti-drift is a TEST, not this comment: `ObjectView.mapFlatten.test.tsx` - * pins this exact list against `ObjectMapConfigSchema.shape` — imported only - * from that TEST file, which the alias-closure walker explicitly excludes - * from traversal — so a key added to or removed from the declaration still - * fails here, loudly and by name, without reintroducing the runtime edge that - * breaks the walker. + * Anti-drift is TWO mechanisms, neither of them this comment: + * - the type below is TOTAL — a `Record` over EVERY `keyof ObjectMapConfig`, + * not a list of some of them — so a key added to the declaration fails + * `tsc` here until it is given a flat spelling. A key can no longer be + * left out by simply not being written down, which is how `style` was. + * - `ObjectView.mapFlatten.test.tsx` pins this object's key set against + * `ObjectMapConfigSchema.shape` — imported only from that TEST file, which + * the alias-closure walker explicitly excludes from traversal — and asserts + * the RELATION (every declared key is delivered under its flat spelling). + * The pre-#9950 pin could not see the omission because it compared the hand + * list against `shape` MINUS `style`: the set it measured against was + * narrowed by the same subtraction the defect was made of, so it stayed + * green while an authored style was being discarded. + */ +export const FLAT_MAP_CONFIG_SPELLING = { + latitudeField: 'latitudeField', + longitudeField: 'longitudeField', + locationField: 'locationField', + titleField: 'titleField', + descriptionField: 'descriptionField', + zoom: 'zoom', + center: 'center', + // The one key whose flat spelling differs from its declared name — see the + // objectui#9950 paragraph above for why it is `mapStyle` and not `style`. + style: 'mapStyle', +} as const satisfies Record; + +/** + * Copy the declared map keys an author actually wrote onto the flat product, + * each under its flat spelling. + * + * Values travel AS WRITTEN: this is transport, not a second validation of the + * declared block — that reading belongs to `getMapConfig` in `ObjectMap.tsx` + * and stays there (objectui#5018). Discarding an ill-typed value here would + * reintroduce exactly the silent drop objectui#9950 closed. */ -export const FLAT_MAP_CONFIG_KEYS = [ - 'latitudeField', - 'longitudeField', - 'locationField', - 'titleField', - 'descriptionField', - 'zoom', - 'center', -] as const satisfies readonly (keyof Omit)[]; - -/** Pick only the declared flat map keys present on an authored `map` block. */ function pickFlatMapConfig(mapConfig: unknown): Record { if (!mapConfig || typeof mapConfig !== 'object') return {}; const source = mapConfig as Record; - return Object.fromEntries(FLAT_MAP_CONFIG_KEYS.filter((key) => key in source).map((key) => [key, source[key]])); + return Object.fromEntries( + Object.entries(FLAT_MAP_CONFIG_SPELLING) + .filter(([declared]) => declared in source) + .map(([declared, flat]) => [flat, source[declared]]), + ); } /** @@ -1729,7 +1762,8 @@ export const ObjectView: React.FC = ({ ...(viewOptions.gantt || {}), }; case 'map': - // Whitelisted flatten (objectui#5177) — see `FLAT_MAP_CONFIG_KEYS`. + // Whitelisted flatten (objectui#5177) — see `FLAT_MAP_CONFIG_SPELLING`, + // which also carries `style` out as `mapStyle` (objectui#9950). // `viewOptions.map` is an untyped bag (`NamedListView.options`); a raw // spread here forwarded every key the author wrote, including `style`, // which `ObjectMap`'s `FlatMapConfigKeys` declares OUT of this flat form. diff --git a/packages/plugin-view/src/__tests__/ObjectView.mapFlatten.test.tsx b/packages/plugin-view/src/__tests__/ObjectView.mapFlatten.test.tsx index a0bd8b7455..e6021de1ad 100644 --- a/packages/plugin-view/src/__tests__/ObjectView.mapFlatten.test.tsx +++ b/packages/plugin-view/src/__tests__/ObjectView.mapFlatten.test.tsx @@ -31,14 +31,14 @@ */ import { describe, it, expect, vi } from 'vitest'; -import { render, waitFor } from '@testing-library/react'; -import { ObjectView, FLAT_MAP_CONFIG_KEYS } from '../ObjectView'; +import { render, waitFor, cleanup } from '@testing-library/react'; +import { ObjectView, FLAT_MAP_CONFIG_SPELLING } from '../ObjectView'; import type { ObjectViewSchema } from '@object-ui/types'; -// Test-only: `ObjectView.tsx` hand-lists `FLAT_MAP_CONFIG_KEYS` rather than -// importing this schema at runtime, precisely so this module never appears in -// `examples/console-starter`'s walked import graph (see the comment on the -// constant). A test file is explicitly excluded from that walk, so pinning -// against the real declaration HERE is safe. +// Test-only: `ObjectView.tsx` hand-writes `FLAT_MAP_CONFIG_SPELLING` rather +// than importing this schema at runtime, precisely so this module never +// appears in `examples/console-starter`'s walked import graph (see the comment +// on the constant). A test file is explicitly excluded from that walk, so +// pinning against the real declaration HERE is safe. import { ObjectMapConfigSchema } from '@object-ui/types/zod'; /** Every schema the view hands to SchemaRenderer, in order. */ @@ -136,10 +136,17 @@ describe('ObjectView flattens `options.map` and emits NO `map` key (objectui#501 * `BaseSchema.style` (inline CSS, legal on every node), so a `map: { style: * '' }` authoring intent arrived as a top-level CSS-shaped key. * - * These pin the whitelist (`FLAT_MAP_CONFIG_KEYS`, derived from - * `ObjectMapConfigSchema` in `@object-ui/types/zod`) rather than the raw - * spread: a declared key still travels, `style` does not, and neither does an - * arbitrary undeclared one. + * These pin the whitelist (`FLAT_MAP_CONFIG_SPELLING`, hand-written in + * `ObjectView.tsx` itself — ⛔ NOT derived from `ObjectMapConfigSchema` at + * runtime there, see the comment on that constant for why) rather than the raw + * spread: a declared key still travels, an arbitrary undeclared one does not, + * and the top-level `style` namespace is never written. + * + * objectui#9950 — `style` is no longer DROPPED either. It is delivered under + * its flat spelling `mapStyle`, which is what `ObjectMap.getMapConfig` reads + * (`schema.mapStyle || schema.map?.style`) and a declared member of + * `ObjectMapSchema`; the top-level `style` it must not collide with stays the + * base face's inline CSS. */ describe('ObjectView flattens `map` through a whitelist, not a raw spread (objectui#5177)', () => { it('still reaches a declared key (locationField) in the flattened product', async () => { @@ -148,7 +155,7 @@ describe('ObjectView flattens `map` through a whitelist, not a raw spread (objec expect(schema.locationField).toBe('geo'); }); - it('does NOT let `style` in the `map` block reach the top level — the specimen the card measured', async () => { + it('does NOT let `style` in the `map` block reach the TOP-LEVEL `style` — it travels as `mapStyle`', async () => { const schema = await renderMapView({ latitudeField: 'lat', longitudeField: 'lng', @@ -156,8 +163,13 @@ describe('ObjectView flattens `map` through a whitelist, not a raw spread (objec }); expect(schema.latitudeField).toBe('lat'); + // objectui#5177 — the top-level `style` namespace belongs to + // `BaseSchema.style` (inline CSS) and the flatten never writes it. expect(schema.style).toBeUndefined(); expect(Object.prototype.hasOwnProperty.call(schema, 'style')).toBe(false); + // objectui#9950 — and the authored map style is no longer discarded on + // the way: it arrives under the spelling `getMapConfig` reads. + expect(schema.mapStyle).toBe('https://tiles.example.com/style.json'); }); it('does NOT let an arbitrary undeclared key reach the top level', async () => { @@ -173,20 +185,163 @@ describe('ObjectView flattens `map` through a whitelist, not a raw spread (objec }); /** - * `FLAT_MAP_CONFIG_KEYS` is hand-listed in `ObjectView.tsx` itself (not - * derived at runtime — see the comment on the constant for why), so THIS is - * the mechanism that keeps it from silently drifting off `ObjectMapConfigSchema` - * — the same role `packages/core/src/actions/__tests__/actionKeys.pin.test.ts` - * plays for `SPEC_ACTION_KEYS`. A key added to or removed from the schema - * fails this test by name, without requiring a runtime import of + * objectui#9950 — THE RELATION, replacing a pin that could not see the defect. + * + * The pin this supersedes read + * `Object.keys(ObjectMapConfigSchema.shape).filter((key) => key !== 'style')` + * and compared the hand list to THAT. The subtraction in the comparison set is + * the defect itself, spelled a second time: the list was measured against a + * declaration that had already had `style` removed from it, so the assertion + * was green precisely while an authored map style was dropped before the + * renderer and the map painted the public demo tiles. A pin narrowed to match + * the bug reports "the list is correct" to the next reader. + * + * What is asserted instead is a RELATION over the declaration read WHOLE: + * every key `ObjectMapConfigSchema` declares is delivered by the flatten, + * under its flat spelling. There is no literal key list here to narrow. + * + * The whitelist's own reason survives unchanged, in the file's own words + * (objectui#5177): `style` is ALSO `BaseSchema.style` — inline CSS, legal on + * every node — so a whole-bag spread put two namespaces on one key. The map + * style is therefore delivered as `mapStyle`: the spelling + * `ObjectMap.getMapConfig` actually reads (`schema.mapStyle || schema.map?.style`, + * never a top-level `style` — objectui#5017) and a declared member of + * `ObjectMapSchema`. Honouring the declaration and keeping the collision shut + * are not in tension; the old flattener just did neither for this key. + */ +describe('every declared ObjectMapConfigSchema key is delivered by the flatten (objectui#9950)', () => { + /** The declaration, read WHOLE. Nothing is filtered out of it here. */ + const declared = Object.keys(ObjectMapConfigSchema.shape); + + /** + * One value per declared key, chosen by asking the DECLARATION which shapes + * it accepts rather than hand-listing a type per key. A key added later with + * a shape none of these fit fails loudly here instead of being skipped. + */ + const CANDIDATE_VALUES: unknown[] = ['probe-value', 7, [1, 2]]; + function sampleFor(key: string): unknown { + const member = (ObjectMapConfigSchema.shape as Record { success: boolean } }>)[key]; + for (const candidate of CANDIDATE_VALUES) { + if (member.safeParse(candidate).success) return candidate; + } + throw new Error(`No sample value is accepted by ObjectMapConfigSchema.${key} — extend CANDIDATE_VALUES`); + } + + /** An authored `map` block carrying EVERY declared key, legal by the declaration. */ + const authoredEverything: Record = Object.fromEntries(declared.map((key) => [key, sampleFor(key)])); + + /** Which declared keys did NOT arrive, under their flat spelling, carrying the authored value. */ + function undeliveredKeys(product: Record): string[] { + return declared.filter((key) => { + const flat = (FLAT_MAP_CONFIG_SPELLING as Record)[key]; + if (!flat || !(flat in product)) return true; + return JSON.stringify(product[flat]) !== JSON.stringify(authoredEverything[key]); + }); + } + + /** + * The PRE-FIX flattener, reproduced: the hand list narrowed by the very + * `key !== 'style'` subtraction the old pin also used on its comparison set. + */ + function preFixFlatten(source: Record): Record { + const preFixKeys = declared.filter((key) => key !== 'style'); + return Object.fromEntries(preFixKeys.filter((key) => key in source).map((key) => [key, source[key]])); + } + + it('the probe config is legal by the declaration, and the declaration does carry `style`', () => { + // The card's shape: an authored map style PARSES GREEN. If this stopped + // being true the relation below would be measuring an illegal document. + expect(ObjectMapConfigSchema.safeParse(authoredEverything).success).toBe(true); + expect(declared).toContain('style'); + }); + + it('delivers every declared key, from a config carrying every declared key', async () => { + const schema = await renderMapView(authoredEverything); + + expect(undeliveredKeys(schema)).toEqual([]); + // The map style specifically: it reaches the renderer under the spelling + // `getMapConfig` reads, and the top-level CSS namespace stays untouched. + expect(schema.mapStyle).toBe(authoredEverything.style); + expect(Object.prototype.hasOwnProperty.call(schema, 'style')).toBe(false); + }); + + it('CONTROL — the same assertion REJECTS the pre-fix flatten, so the instrument can fail', () => { + const preFixProduct = preFixFlatten(authoredEverything); + + // Named, not merely non-empty: the one key the pre-fix whitelist drops. + expect(undeliveredKeys(preFixProduct)).toEqual(['style']); + expect(() => expect(undeliveredKeys(preFixProduct)).toEqual([])).toThrow(); + }); + + it('CONTROL — the narrowed `declared` set is what let the old pin pass', () => { + // Reproduced from the pin this file replaces. Against THIS set the + // pre-fix hand list was a perfect match, which is why it was green. + const narrowed = declared.filter((key) => key !== 'style').sort(); + + expect(narrowed).not.toContain('style'); + expect(preFixFlatten(authoredEverything)).not.toHaveProperty('style'); + expect(Object.keys(FLAT_MAP_CONFIG_SPELLING).sort()).not.toEqual(narrowed); + }); + + it('a config authoring ONLY `style` still delivers it — the single-key case a whitelist drops silently', async () => { + const schema = await renderMapView({ style: 'https://tiles.example.com/only.json' }); + + expect(schema.mapStyle).toBe('https://tiles.example.com/only.json'); + expect(Object.prototype.hasOwnProperty.call(schema, 'style')).toBe(false); + }); + + it('forwards NO key the declaration does not carry — objectui#5177 still holds', async () => { + // The keys the `map` block CONTRIBUTES to the product: what a fully + // authored block adds over what an empty one produces. That set must be + // exactly the image of the spelling table — nothing undeclared added, + // nothing declared missing. Both directions, one assertion. + // ⛔ `cleanup()` between the two renders is load-bearing, not tidiness: the + // spy collects from EVERY mounted tree, so a second `render` in one test + // leaves the first one free to re-render and append, and the baseline read + // below would then be the other config's product (measured: `contributed` + // came back empty, i.e. the assertion silently compared a product to + // itself). + const withNothing = await renderMapView({}); + const baselineKeys = Object.keys(withNothing); + cleanup(); + + const withEverything = await renderMapView({ + ...authoredEverything, + totallyUndeclaredKey: 'nope', + style2: 'nope', + // ⛔ `mapStyle` is the flat OUTPUT spelling, not a declared member of + // `ObjectMapConfigSchema` — writing it INSIDE the block is undeclared + // authoring and must not travel. The whitelist is keyed on declared + // SOURCE names; only `style` produces the `mapStyle` output below. + mapStyle: 'nope', + }); + const contributed = Object.keys(withEverything).filter((key) => !baselineKeys.includes(key)); + + expect(contributed.sort()).toEqual([...Object.values(FLAT_MAP_CONFIG_SPELLING)].sort()); + expect(Object.prototype.hasOwnProperty.call(withEverything, 'totallyUndeclaredKey')).toBe(false); + expect(Object.prototype.hasOwnProperty.call(withEverything, 'style2')).toBe(false); + expect(withEverything.mapStyle).toBe(authoredEverything.style); + }); +}); + +/** + * `FLAT_MAP_CONFIG_SPELLING` is hand-written in `ObjectView.tsx` itself (not derived + * at runtime — see the comment on the constant for why), so THIS is the + * mechanism that keeps it from silently drifting off `ObjectMapConfigSchema` — + * the same role `packages/core/src/actions/__tests__/actionKeys.pin.test.ts` + * plays for `SPEC_ACTION_KEYS`. A key added to or removed from the schema fails + * this test by name, without requiring a runtime import of * `@object-ui/types/zod` from production code that reaches * `examples/console-starter`. + * + * The comparison set is the WHOLE declaration (objectui#9950). Nothing is + * filtered out of it — that filter is what made the previous version of this + * pin agree with the bug it was supposed to catch. */ -describe('FLAT_MAP_CONFIG_KEYS pins against ObjectMapConfigSchema (objectui#5177)', () => { - it('matches the schema minus `style`, so the hand list cannot silently drift', () => { - const declared = Object.keys(ObjectMapConfigSchema.shape) - .filter((key) => key !== 'style') - .sort(); - expect([...FLAT_MAP_CONFIG_KEYS].sort()).toEqual(declared); +describe('FLAT_MAP_CONFIG_SPELLING pins against ObjectMapConfigSchema (objectui#5177, objectui#9950)', () => { + it('covers the declaration in full, so the table cannot silently drift', () => { + const declared = Object.keys(ObjectMapConfigSchema.shape).sort(); + + expect(Object.keys(FLAT_MAP_CONFIG_SPELLING).sort()).toEqual(declared); }); });