fix(fields): carry the spec's optional location keys across a coordinate edit

.changeset/6664-location-optional-keys-carry.md

@@ -0,0 +1,27 @@
+---
+'@object-ui/fields': patch
+---
+
+`LocationField` no longer discards a location's `altitude` / `accuracy` when the user
+retypes the coordinate pair (objectui#6664).
+
+The widget edits the pair as one comma-separated text box and rebuilt its emission as a
+fresh `{ lat, lng }` from the parsed text, so the two OPTIONAL keys `@objectstack/spec`
+declares alongside them — `LocationValue` is `{ lat, lng, altitude?, accuracy? }` — were
+gone the moment anyone edited the coordinates. Nothing warned; they simply were not in
+the object handed to `onChange`. Both keys are registered on the platform's authorable
+surface (`authorable-surface.base.json`), so a customer may author them even though the
+platform itself produces neither today — measured in both repos.
+
+The drop **predates** objectui#6272: before that flip the widget emitted
+`{ latitude, longitude }` and discarded the rest identically. What #6272 changed is only
+that the *declared* value type is now the spec's, so the type claimed four keys while the
+write path handled two. This closes that gap; it is not a regression #6272 introduced.
+
+The carry is a key-by-key pick of exactly those two keys out of a value that is already a
+valid `LocationValue` — deliberately **not** a spread of the incoming value, which would
+carry a stored record's retired `latitude` / `longitude` spelling straight back into the
+emitted object and undo #6272's rename. A negative control pins that. Each key is taken
+only when it is a usable number, because the spec's `z.number()` rejects `NaN`, `Infinity`
+and a numeric string alike; leaving such a value behind narrows the emission rather than
+widening what the widget accepts.

packages/fields/src/__tests__/LocationField.optionalKeys.test.tsx

@@ -0,0 +1,231 @@
+/**
+ * ObjectUI
+ * Copyright (c) 2024-present ObjectStack Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+/**
+ * objectui#6664 — editing the coordinate pair must not silently drop the
+ * spec's two optional keys.
+ *
+ * `@objectstack/spec` declares the stored shape of a `type: 'location'` value
+ * as `{ lat, lng, altitude?, accuracy? }`. `LocationField` edits the pair as
+ * one comma-separated text box and built its emission as a fresh `{ lat, lng }`
+ * from the parsed text, so a stored `altitude` / `accuracy` vanished the moment
+ * a user retyped the coordinates. Nothing warned; the two keys simply were not
+ * in the emitted object.
+ *
+ * The drop PREDATES objectui#6272 — before that flip the widget emitted
+ * `{ latitude, longitude }` and discarded the rest identically. #6272 changed
+ * only the DECLARED value type (to the spec's), so the type claimed four keys
+ * while the write path handled two. That mismatch is what made this visible; it
+ * is not a regression #6272 introduced, and keeping it out of that card's
+ * atomic fence (a bare two-sided rename, maintainer ruling 2026-08-28 「6272
+ * A1」) was the right call.
+ *
+ * Triage measured that the platform has NO producer for either key — not in
+ * objectui, not in objectstack — and queued the card anyway, because both keys
+ * are registered on the authorable surface (`objectstack
+ * packages/spec/authorable-surface.base.json:3438,3442`). The platform has
+ * already promised customers they may author them, so the population that
+ * decides this is "does a customer write it", which neither repo can measure.
+ *
+ * ⛔ THE FENCE. The carry is a key-by-key pick of the two spec-declared
+ * optional keys, taken from a value that is ALREADY a valid `LocationValue` —
+ * never a wholesale spread of the incoming value, never `Object.assign`. A
+ * spread would carry a deprecated `latitude` / `longitude` key straight back
+ * into the emitted object and undo #6272's rename.
+ *
+ * ⚠️ And the spec schema cannot be that guard. `valueSchemaFor({ type:
+ * 'location' })` is a plain, NON-STRICT `z.object` — measured, and pinned in
+ * the last test below: it ACCEPTS `{ lat, lng, latitude, longitude }` and
+ * merely strips the two unknown keys from its parsed output, while the object
+ * handed to `onChange` still carries them. So every anti-dialect assertion here
+ * reads the EMITTED object's own keys rather than `safeParse`.
+ */
+
+import { describe, it, expect, vi } from 'vitest';
+import React from 'react';
+import { render, screen, fireEvent, cleanup } from '@testing-library/react';
+import '@testing-library/jest-dom';
+import { valueSchemaFor } from '@objectstack/spec/data';
+
+import { LocationField, type LocationValue } from '../widgets/LocationField';
+
+const LOCATION_SCHEMA = valueSchemaFor({ type: 'location' } as any)!;
+
+/**
+ * The four keys the spec itself declares, read off the schema rather than
+ * copied into a literal here: the anti-spread assertions below are only worth
+ * anything if "outside the spec" means what the spec currently says.
+ */
+const SPEC_KEYS: readonly string[] = Object.keys((LOCATION_SCHEMA as any).shape);
+
+const field = { name: 'site', label: 'Site', type: 'location' } as any;
+
+/** Hangzhou, in the spec spelling, carrying both optional keys. */
+const STORED: LocationValue = { lat: 30.2741, lng: 120.1551, altitude: 5, accuracy: 12 };
+/** Shanghai — what the user retypes into the box. */
+const TYPED = '31.2304, 121.4737';
+const TYPED_PAIR = { lat: 31.2304, lng: 121.4737 };
+
+/** Render with `value` stored, retype the coordinates, return what was emitted. */
+function emitAfterEdit(value: unknown, text: string = TYPED): any {
+  cleanup();
+  const onChange = vi.fn();
+  render(
+    <LocationField field={field} value={value as LocationValue | null} onChange={onChange} />,
+  );
+  fireEvent.change(screen.getByRole('textbox'), { target: { value: text } });
+  expect(onChange).toHaveBeenCalledTimes(1);
+  return onChange.mock.calls[0][0];
+}
+
+describe('LocationField carries the spec optional keys across a coordinate edit (objectui#6664)', () => {
+  it('keeps altitude and accuracy when the user retypes the pair', () => {
+    // The defect itself. This emitted `{ lat, lng }` alone before the fix, so a
+    // customer-authored altitude/accuracy was gone after a single edit.
+    expect(emitAfterEdit(STORED)).toEqual({ ...TYPED_PAIR, altitude: 5, accuracy: 12 });
+  });
+
+  it('keeps whichever of the two the record actually carries', () => {
+    const altOnly = emitAfterEdit({ lat: 30.2741, lng: 120.1551, altitude: 5 });
+    expect(altOnly).toEqual({ ...TYPED_PAIR, altitude: 5 });
+    expect(altOnly).not.toHaveProperty('accuracy');
+
+    const accOnly = emitAfterEdit({ lat: 30.2741, lng: 120.1551, accuracy: 12 });
+    expect(accOnly).toEqual({ ...TYPED_PAIR, accuracy: 12 });
+    expect(accOnly).not.toHaveProperty('altitude');
+  });
+
+  it('keeps a zero altitude, which is a real elevation and not an absent key', () => {
+    // Sea level. The carry must be keyed on "the record has a usable number",
+    // never on truthiness — `altitude: 0` is exactly the value a falsy test
+    // would silently discard.
+    expect(emitAfterEdit({ lat: 30.2741, lng: 120.1551, altitude: 0, accuracy: 0 }))
+      .toEqual({ ...TYPED_PAIR, altitude: 0, accuracy: 0 });
+  });
+
+  it('emits a value the platform validator still accepts', () => {
+    // The load-bearing half is the spec's own validator, not a copy of it: the
+    // carry must not turn this widget into a producer of invalid values.
+    const parsed = LOCATION_SCHEMA.safeParse(emitAfterEdit(STORED));
+    expect(parsed.success).toBe(true);
+    expect(parsed.data).toEqual({ ...TYPED_PAIR, altitude: 5, accuracy: 12 });
+  });
+
+  it('does not let one unusable optional key cost the other one', () => {
+    // Measured against the spec schema: `z.number()` rejects NaN, Infinity and
+    // a numeric STRING alike — all three report `invalid_type` at `[altitude]`.
+    // So an unusable altitude cannot be carried without making the whole
+    // emission spec-invalid, and it is dropped by the same finite-number test
+    // the widget already applies to `lat`/`lng`. That is a NARROWING — it emits
+    // less, never more — and it must not take a perfectly good `accuracy` with
+    // it.
+    for (const altitude of [NaN, Infinity, '5' as unknown as number]) {
+      const emitted = emitAfterEdit({ lat: 30.2741, lng: 120.1551, altitude, accuracy: 12 });
+      expect(emitted).not.toHaveProperty('altitude');
+      expect(emitted).toEqual({ ...TYPED_PAIR, accuracy: 12 });
+      expect(LOCATION_SCHEMA.safeParse(emitted).success).toBe(true);
+    }
+  });
+});
+
+describe('LocationField invents optional keys it was never given (objectui#6664)', () => {
+  it('adds nothing to a record that carries neither key', () => {
+    expect(Object.keys(emitAfterEdit({ lat: 30.2741, lng: 120.1551 })).sort())
+      .toEqual(['lat', 'lng']);
+  });
+
+  it('adds nothing when there is no previously stored value', () => {
+    expect(Object.keys(emitAfterEdit(null)).sort()).toEqual(['lat', 'lng']);
+  });
+
+  it('still emits null when the box is cleared, carrying nothing forward', () => {
+    // Clearing the box means "unset", and an unset location has no altitude to
+    // preserve. The carry must not resurrect the old value as a partial object.
+    const onChange = vi.fn();
+    render(<LocationField field={field} value={STORED} onChange={onChange} />);
+    fireEvent.change(screen.getByRole('textbox'), { target: { value: '  ' } });
+    expect(onChange).toHaveBeenCalledWith(null);
+  });
+});
+
+/**
+ * ⚠️ NEGATIVE CONTROL — green BEFORE this fix as well as after, and that is the
+ * correct reading rather than a vacuous one. Today's code emits a freshly built
+ * `{ lat, lng }`, which trivially carries no dialect, so there is no red for
+ * these to show. They exist to guard the FUTURE regression the fence names:
+ * someone "simplifying" the key-by-key pick into `onChange({ ...value, lat,
+ * lng })`. Under that rewrite the first test below goes red, because its stored
+ * value carries BOTH spellings — which is exactly the guard the fence asked
+ * for, and exactly what no positive pin can provide.
+ */
+describe('NEGATIVE CONTROL — the carry is a key-by-key pick, never a spread (objectui#6664 fence)', () => {
+  it('never emits the deprecated latitude/longitude, even from a value carrying both spellings', () => {
+    // The sharp control: this value IS a valid `LocationValue` (`lat`/`lng` are
+    // finite), so the carry path really runs on it — and it must still leave
+    // the retired spelling behind. A spread would emit all six keys and undo
+    // #6272.
+    const emitted = emitAfterEdit({
+      lat: 30.2741,
+      lng: 120.1551,
+      latitude: 30.2741,
+      longitude: 120.1551,
+      altitude: 5,
+      accuracy: 12,
+    });
+    expect(emitted).not.toHaveProperty('latitude');
+    expect(emitted).not.toHaveProperty('longitude');
+  });
+
+  it('emits no key the spec does not declare, whatever the record carries', () => {
+    // The general form of the assertion above, derived from the spec's own
+    // declared key set: a spread of the incoming value would leak whatever
+    // extra keys a record happens to hold, of which the retired pair is only
+    // the most damaging example.
+    const emitted = emitAfterEdit({
+      lat: 30.2741,
+      lng: 120.1551,
+      latitude: 30.2741,
+      longitude: 120.1551,
+      provider: 'gps',
+      altitude: 5,
+    });
+    expect(SPEC_KEYS).toEqual(['lat', 'lng', 'altitude', 'accuracy']);
+    expect(Object.keys(emitted).filter((k) => !SPEC_KEYS.includes(k))).toEqual([]);
+  });
+
+  it('carries nothing at all out of a value the spec rejects outright', () => {
+    // A record in the retired `{ latitude, longitude }` spelling reads as unset
+    // here (ruled A1, pinned in LocationField.specShape.test.tsx). Its optional
+    // keys are not salvaged either — harvesting them would be reading the
+    // dialect through a side door, which the A1 ruling closed.
+    expect(LOCATION_SCHEMA.safeParse({ latitude: 30.2741, longitude: 120.1551 }).success).toBe(false);
+    const emitted = emitAfterEdit({
+      latitude: 30.2741,
+      longitude: 120.1551,
+      altitude: 5,
+      accuracy: 12,
+    });
+    expect(Object.keys(emitted).sort()).toEqual(['lat', 'lng']);
+    expect(emitted).toEqual(TYPED_PAIR);
+  });
+
+  it('pins WHY those assertions read keys and not safeParse: the schema is not strict', () => {
+    // Measured, and the reason the fence needs a guard of its own: the spec's
+    // `LocationValue` is a plain `z.object`, so an emission polluted by a
+    // spread still PASSES validation — zod merely strips the unknown keys from
+    // its parsed OUTPUT, while the object handed to `onChange` keeps them.
+    const polluted = { lat: 31.2304, lng: 121.4737, latitude: 30.2741, longitude: 120.1551 };
+    const parsed = LOCATION_SCHEMA.safeParse(polluted);
+    expect(parsed.success).toBe(true);
+    expect(Object.keys(parsed.data as object).sort()).toEqual(['lat', 'lng']);
+    // The object itself is where the dialect survives — hence the key-level
+    // assertions above. If the spec ever turns strict, this test is the note
+    // saying the guard could then be delegated to it.
+    expect(polluted).toHaveProperty('latitude');
+  });
+});

packages/fields/src/widgets/LocationField.tsx

@@ -41,8 +41,51 @@ export type { LocationValue } from '@objectstack/spec/data';
 function isLocationValue(value: unknown): value is LocationValue {
   if (!value || typeof value !== 'object' || Array.isArray(value)) return false;
   const { lat, lng } = value as Record<string, unknown>;
-  return typeof lat === 'number' && Number.isFinite(lat)
-    && typeof lng === 'number' && Number.isFinite(lng);
+  return isFiniteNumber(lat) && isFiniteNumber(lng);
+}
+
+/**
+ * A usable numeric component of a location: a real number, never `NaN` or an
+ * infinity. Named so the coordinates and the two optional keys below are held
+ * to the SAME test rather than to two copies of it that can drift apart.
+ */
+function isFiniteNumber(n: unknown): n is number {
+  return typeof n === 'number' && Number.isFinite(n);
+}
+
+/**
+ * Build the value emitted for a freshly typed coordinate pair, carrying the
+ * spec's two OPTIONAL keys across the edit (objectui#6664).
+ *
+ * The box edits `lat`/`lng` only, but a stored location may also carry
+ * `altitude` and `accuracy` — both declared by `LocationValueSchema`, and both
+ * registered on the platform's authorable surface, so a customer may write
+ * them. Rebuilding the emission as a bare `{ lat, lng }` dropped them silently
+ * the moment anyone retyped the coordinates, with nothing to warn them.
+ *
+ * ⛔ The carry is a KEY-BY-KEY pick out of an already-valid `LocationValue`,
+ * and deliberately NOT a spread of `previous` (nor `Object.assign`): a stored
+ * record may still hold the retired `{ latitude, longitude }` spelling, and
+ * spreading it would carry that dialect straight back into the emitted object
+ * and undo objectui#6272's rename. The spec schema cannot be that guard —
+ * `LocationValueSchema` is a plain, NON-STRICT `z.object`, so it ACCEPTS a
+ * polluted object and merely strips the unknown keys from its own parsed
+ * OUTPUT, while the value handed to `onChange` keeps them. Both facts are
+ * pinned in `__tests__/LocationField.optionalKeys.test.tsx`.
+ *
+ * Each optional key is taken only when it is a usable number. Measured against
+ * the spec: `z.number()` rejects `NaN`, `Infinity` and a numeric string alike
+ * (`invalid_type` at `[altitude]`), so carrying one of those forward would make
+ * this widget emit a value the platform's own validator refuses. Leaving it
+ * behind is a NARROWING — this emits less than it was handed, never more — not
+ * a tolerant fallback of the kind AGENTS.md #0.1 bans.
+ */
+function carryOptionalKeys(lat: number, lng: number, previous: unknown): LocationValue {
+  const emitted: LocationValue = { lat, lng };
+  if (!isLocationValue(previous)) return emitted;
+  if (isFiniteNumber(previous.altitude)) emitted.altitude = previous.altitude;
+  if (isFiniteNumber(previous.accuracy)) emitted.accuracy = previous.accuracy;
+  return emitted;
 }
 
 /**
@@ -82,7 +125,9 @@ export function LocationField({ value, onChange, field, readonly, error, ...prop
       const lat = parseFloat(parts[0]);
       const lng = parseFloat(parts[1]);
       if (!isNaN(lat) && !isNaN(lng)) {
-        onChange({ lat, lng });
+        // The typed pair replaces `lat`/`lng`; `altitude`/`accuracy` survive
+        // the edit (objectui#6664). Key-by-key, never a spread — see above.
+        onChange(carryOptionalKeys(lat, lng, value));
       }
       // If invalid, don't update the value
     }