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); }); });