diff --git a/.changeset/6272-locationfield-spec-shape.md b/.changeset/6272-locationfield-spec-shape.md new file mode 100644 index 0000000000..51b44d1f3c --- /dev/null +++ b/.changeset/6272-locationfield-spec-shape.md @@ -0,0 +1,45 @@ +--- +'@object-ui/fields': minor +--- + +**BREAKING (stored data): `LocationField` reads and writes the spec's `{ lat, lng }`** + +FROM — the widget read `value.latitude` / `value.longitude`, each behind `|| 0`, and emitted +`{ latitude, longitude } | null`. + +TO — it reads and writes `LocationValue` from `@objectstack/spec/data` +(`{ lat, lng, altitude?, accuracy? }`), re-exported here rather than re-declared, and reads +nothing else. A pair is read only when BOTH `lat` and `lng` are finite numbers. + +**The behaviour change, stated plainly:** a `type: 'location'` record stored in the retired +`{ latitude, longitude }` spelling — including one this widget itself wrote before this +release — now renders **EMPTY in the edit surface**, where it used to render its +coordinates. It keeps rendering correctly in detail views, list cells and on the map, which +read `lat`/`lng` first. Re-saving the record through this widget, or fixing the value at the +data layer, restores it. There is deliberately **no compatibility fallback**: the maintainer +ruled the bare flip (2026-08-28, objectui#6272 option A1) explicitly over a dated read-side +shim, choosing zero dialect over softening this cost. + +Marked `minor` per AGENTS.md §版本号策略 (this repo never publishes `major` outside an +`@objectstack` major sync); the break is real and is stated here. + +**Why the widget was the side that moved** + +`@objectstack/spec@17.2.0` exports `LocationValue = { lat, lng, altitude?, accuracy? }` as +the canonical stored shape and deprecates `LocationCoordinates` (`{ latitude, longitude }`). +Measured through the contract itself, `valueSchemaFor({ type: 'location' })` **rejects** +`{ latitude, longitude }` with `invalid_type` at `[lat]` and `[lng]`, and **accepts** +`{ lat, lng }`. So this widget was the one `location` surface producing a shape the +platform's own validator refuses, and `LocationCellRenderer` / `ObjectMap` reading +`lat`/`lng` first is correct by contract, not tolerance. + +The user-visible defect it fixes: a spec-canonical `{ lat, lng }` record rendered **`0, 0`** +in the edit box — not an error state but a valid coordinate in the Gulf of Guinea — while +the same record rendered correctly one panel away. The `|| 0` defaults are gone with the +rename, so a half-stored pair (`{ lat }` alone) no longer invents the coordinate it is +missing; it reads as unset. A stored `{ lat: 0, lng: 0 }` still renders `0, 0`, because that +is now the only way those digits can appear. + +`GeolocationField` is **not** part of this change: `geolocation` is not a member of the +spec's closed `FieldType` union and its value schema accepts both spellings, so it keeps its +own `{ latitude, longitude }` shape. diff --git a/content/docs/fields/location.mdx b/content/docs/fields/location.mdx index 85210c637e..fdc21813d6 100644 --- a/content/docs/fields/location.mdx +++ b/content/docs/fields/location.mdx @@ -98,7 +98,7 @@ const field: LocationFieldMetadata = { }; export function StoreLocationInput() { - const [value, setValue] = useState<{ latitude: number; longitude: number } | null>(null); + const [value, setValue] = useState<{ lat: number; lng: number } | null>(null); return ; } ``` diff --git a/examples/schema-catalog/src/schemas/fields-location/read-only-location.json b/examples/schema-catalog/src/schemas/fields-location/read-only-location.json index 51b5fcc637..3fceb12eb8 100644 --- a/examples/schema-catalog/src/schemas/fields-location/read-only-location.json +++ b/examples/schema-catalog/src/schemas/fields-location/read-only-location.json @@ -4,8 +4,8 @@ "showCancel": false, "defaultValues": { "headquarters": { - "latitude": 40.7128, - "longitude": -74.006 + "lat": 40.7128, + "lng": -74.006 } }, "fields": [ diff --git a/examples/schema-catalog/src/schemas/fields-location/san-francisco-coordinates.json b/examples/schema-catalog/src/schemas/fields-location/san-francisco-coordinates.json index 1947f74368..903fd9cd59 100644 --- a/examples/schema-catalog/src/schemas/fields-location/san-francisco-coordinates.json +++ b/examples/schema-catalog/src/schemas/fields-location/san-francisco-coordinates.json @@ -4,8 +4,8 @@ "showCancel": false, "defaultValues": { "store_location": { - "latitude": 37.7749, - "longitude": -122.4194 + "lat": 37.7749, + "lng": -122.4194 } }, "fields": [ diff --git a/packages/fields/src/__tests__/LocationField.specShape.test.tsx b/packages/fields/src/__tests__/LocationField.specShape.test.tsx new file mode 100644 index 0000000000..58eb18e34c --- /dev/null +++ b/packages/fields/src/__tests__/LocationField.specShape.test.tsx @@ -0,0 +1,181 @@ +/** + * 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#6272 — `LocationField` reads and writes `@objectstack/spec`'s + * `LocationValue` (`{ lat, lng }`), the shape the platform's own contract + * enforces. + * + * The defect: this widget was the ONE `type: 'location'` surface reading + * `value.latitude` / `value.longitude`, each behind `|| 0`. So a spec-canonical + * `{ lat, lng }` record rendered `0, 0` in the edit box — not an error state + * but a valid coordinate in the Gulf of Guinea — while the SAME record rendered + * correctly one panel away in the detail view and on the map, which read + * `lat`/`lng` first. `valueSchemaFor({ type: 'location' })` REJECTS + * `{ latitude, longitude }` (`invalid_type` at `[lat]`, `[lng]`) and ACCEPTS + * `{ lat, lng }`, so the widget — not the display side — was the producer at + * fault. + * + * Maintainer ruling 2026-08-28 (「6272 A1 其他同意」), option A1, chosen + * explicitly over a read-side compatibility shim: the flip is BARE. A record + * stored in the deprecated `{ latitude, longitude }` spelling now renders EMPTY + * here until it is re-saved. That is pinned below as ruled behaviour, not + * tolerated behaviour — `renders empty` is the assertion that fails the day + * somebody adds the fallback back. + * + * Every emission is checked against the SPEC SCHEMA rather than against a + * hand-written expected object alone: an equality assertion on `{ lat, lng }` + * would keep passing if the spec moved, and the whole point of this card is + * that the widget must agree with the contract, not with a copy of it. + */ + +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 type { LocationValue as SpecLocationValue } from '@objectstack/spec/data'; + +import { LocationField, type LocationValue } from '../widgets/LocationField'; +import { getCellRenderer, resolveCellRendererType } from '../index'; + +/* -------------------------------------------------------------------------- */ +/* Compile-time pin: the exported name IS the spec's, not a local re-spelling. */ +/* This package's tsconfig includes its tests, so `type-check` compiles this. */ +/* -------------------------------------------------------------------------- */ +type Equal = (() => T extends A ? 1 : 2) extends () => T extends B ? 1 : 2 ? true : false; +type Assert = T; +type _LocationValueIsTheSpecs = Assert>; + +const LOCATION_SCHEMA = valueSchemaFor({ type: 'location' } as any)!; + +/** The card's coordinate, in the spec spelling. */ +const CANONICAL: LocationValue = { lat: 30.2741, lng: 120.1551 }; +/** The same point in the spelling the spec deprecates and rejects. */ +const DEPRECATED = { latitude: 30.2741, longitude: 120.1551 }; + +const field = { name: 'site', label: 'Site', type: 'location' } as any; + +function renderReadonly(value: unknown) { + return render( + , + ); +} + +describe('LocationField reads the spec shape (objectui#6272)', () => { + it('renders a spec-canonical { lat, lng } as its coordinate pair', () => { + // The regression itself: this rendered `0, 0` before the flip. + const { container } = renderReadonly(CANONICAL); + expect(container.textContent).toBe('30.2741, 120.1551'); + }); + + it('agrees with the display surface on the same stored value', () => { + // The card's decisive comparison — one record, two consumers. The edit + // surface and the display registry the detail page calls must now read the + // same value the same way. + const edit = renderReadonly(CANONICAL); + const Renderer = getCellRenderer(resolveCellRendererType({ type: 'location' } as any) || 'location'); + const display = render(); + expect(edit.container.textContent).toContain('30.2741'); + expect(display.container.textContent).toContain('30.2741'); + expect(display.container.textContent).toContain('120.1551'); + }); + + it('carries a zero coordinate as the real place it is, not as a default', () => { + // `0, 0` must still be renderable — it is a valid coordinate. What changed + // is that it can now ONLY come from a stored `{ lat: 0, lng: 0 }`, never + // from a `|| 0` standing in for a key the widget could not find. + const { container } = renderReadonly({ lat: 0, lng: 0 }); + expect(container.textContent).toBe('0, 0'); + }); + + it('keeps the optional spec keys out of the rendered pair', () => { + const { container } = renderReadonly({ lat: 30.2741, lng: 120.1551, altitude: 5, accuracy: 1 }); + expect(container.textContent).toBe('30.2741, 120.1551'); + }); +}); + +describe('LocationField has NO fallback to the deprecated spelling (ruled A1)', () => { + it('renders a { latitude, longitude } record EMPTY, not as coordinates', () => { + // Ruled consequence, pinned: the maintainer chose the bare flip over a + // read-side shim, so a record in the retired spelling reads as unset here + // (it stays correct in detail views and on the map). If a compatibility + // fallback is ever reintroduced, this assertion is what fails. + const { container } = renderReadonly(DEPRECATED); + expect(container.textContent).not.toContain('30.2741'); + expect(container.textContent).not.toContain('120.1551'); + // The empty-value placeholder, i.e. the widget's own "nothing stored" face. + expect(screen.getByLabelText('No value')).toBeInTheDocument(); + }); + + it('renders the editable box empty for a { latitude, longitude } record', () => { + render(); + expect(screen.getByRole('textbox')).toHaveValue(''); + }); + + it('never invents the missing half of a pair', () => { + // `{ lat }` alone used to read `0, 0` and `{ latitude }` alone `30.2741, 0` + // — a longitude the record does not carry. Both are unreadable values now. + for (const half of [{ lat: 30.2741 }, { lng: 120.1551 }, { latitude: 30.2741 }]) { + const { container } = renderReadonly(half); + expect(container.textContent).not.toContain(', 0'); + expect(container.textContent).not.toContain('30.2741'); + cleanup(); + } + }); + + it('reads nothing out of the shapes the spec rejects outright', () => { + for (const rejected of ['30.2741,120.1551', [30.2741, 120.1551], { lat: '30.2741', lng: '120.1551' }]) { + expect(LOCATION_SCHEMA.safeParse(rejected).success).toBe(false); + const { container } = renderReadonly(rejected); + expect(container.textContent).not.toContain('30.2741'); + cleanup(); + } + }); +}); + +describe('LocationField writes the spec shape (objectui#6272)', () => { + it('emits a value the spec ACCEPTS', () => { + const onChange = vi.fn(); + render(); + fireEvent.change(screen.getByRole('textbox'), { target: { value: '30.2741, 120.1551' } }); + + expect(onChange).toHaveBeenCalledTimes(1); + const emitted = onChange.mock.calls[0][0]; + expect(emitted).toEqual({ lat: 30.2741, lng: 120.1551 }); + // The load-bearing half: the platform's own validator, not a copy of it. + const parsed = LOCATION_SCHEMA.safeParse(emitted); + expect(parsed.success).toBe(true); + // …and the shape it replaced is the one that validator rejects. + expect(LOCATION_SCHEMA.safeParse(DEPRECATED).success).toBe(false); + }); + + it('round-trips: what it emits is what it reads back', () => { + const onChange = vi.fn(); + render(); + fireEvent.change(screen.getByRole('textbox'), { target: { value: '40.7128, -74.006' } }); + cleanup(); + + const { container } = renderReadonly(onChange.mock.calls[0][0]); + expect(container.textContent).toBe('40.7128, -74.006'); + }); + + it('emits null when the box is cleared', () => { + const onChange = vi.fn(); + render(); + fireEvent.change(screen.getByDisplayValue('30.2741, 120.1551'), { target: { value: ' ' } }); + expect(onChange).toHaveBeenCalledWith(null); + }); + + it('leaves the value alone when the text is not a coordinate pair', () => { + const onChange = vi.fn(); + render(); + fireEvent.change(screen.getByDisplayValue('30.2741, 120.1551'), { target: { value: 'somewhere' } }); + expect(onChange).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/fields/src/__tests__/readonly-host-plumbing-e2e.test.tsx b/packages/fields/src/__tests__/readonly-host-plumbing-e2e.test.tsx index 0939cd11d6..650fc0fb53 100644 --- a/packages/fields/src/__tests__/readonly-host-plumbing-e2e.test.tsx +++ b/packages/fields/src/__tests__/readonly-host-plumbing-e2e.test.tsx @@ -178,7 +178,7 @@ const VALUES: Record = { lookup: 'rec_1', master_detail: 'rec_1', image: 'https://example.com/a.png', - location: { latitude: 1, longitude: 2 }, + location: { lat: 1, lng: 2 }, formula: 'computed', summary: 7, auto_number: 'A-0001', diff --git a/packages/fields/src/widgets/LocationField.tsx b/packages/fields/src/widgets/LocationField.tsx index 9d9db58a6c..a1c38fbb65 100644 --- a/packages/fields/src/widgets/LocationField.tsx +++ b/packages/fields/src/widgets/LocationField.tsx @@ -1,19 +1,69 @@ import React from 'react'; import { Input, EmptyValue } from '@object-ui/components'; +import type { LocationValue } from '@objectstack/spec/data'; import { FieldWidgetComponentProps } from './types.js'; import { toDomProps } from './toDomProps.js'; /** - * LocationField - Geographic coordinate input for latitude and longitude - * Stores location as { latitude, longitude } object and displays as comma-separated pair + * The stored shape of a `type: 'location'` value — RE-EXPORTED from + * `@objectstack/spec/data`, never re-declared here (objectui#6272). + * + * `LocationValue` is `z.input` of the spec's `LocationValueSchema` + * (`{ lat, lng, altitude?, accuracy? }`), which is what `valueSchemaFor({ type: + * 'location' })` validates a stored location against. A second local + * declaration under the spec's own name is objectstack#4115's failure class and + * is refused by `scripts/check-spec-symbol-derivation.mjs` — it was refused on + * the closed PR #6418, which is why this is a bare re-export. */ -export function LocationField({ value, onChange, field, readonly, error, ...props }: FieldWidgetComponentProps) { +export type { LocationValue } from '@objectstack/spec/data'; + +/** + * A stored value is a location only when it carries BOTH spec coordinates as + * finite numbers. + * + * ⛔ Deliberately no fallback to the deprecated `{ latitude, longitude }` + * spelling, and no `|| 0` default on a missing coordinate — both were the + * defect (objectui#6272, maintainer ruling 2026-08-28 「6272 A1」, option A1 + * chosen explicitly over a read-side compatibility shim): + * + * - reading `latitude`/`longitude` made this widget the one surface that + * disagreed with the platform contract. `valueSchemaFor({type:'location'})` + * REJECTS `{ latitude, longitude }` (`invalid_type` at `[lat]`, `[lng]`) and + * ACCEPTS `{ lat, lng }`, and the display surfaces (`LocationCellRenderer`, + * `ObjectMap`) already read `lat`/`lng` first — that is correct by contract, + * not tolerance. + * - `|| 0` turned every unreadable value into `0, 0`, a VALID coordinate in + * the Gulf of Guinea. A field that renders a plausible wrong place is worse + * than one that renders nothing: an empty box is visibly unset, `0, 0` is + * not. The same applies to a half value — `{ lat }` alone must not invent a + * longitude. + */ +function isLocationValue(value: unknown): value is LocationValue { + if (!value || typeof value !== 'object' || Array.isArray(value)) return false; + const { lat, lng } = value as Record; + return typeof lat === 'number' && Number.isFinite(lat) + && typeof lng === 'number' && Number.isFinite(lng); +} + +/** + * LocationField - Geographic coordinate input for a `type: 'location'` value. + * + * Reads and writes `@objectstack/spec`'s `LocationValue` (`{ lat, lng }`) and + * displays it as the comma-separated pair a user types. The coordinates are + * still called latitude and longitude to a human — only the STORED key names + * are the spec's, which is why the placeholder is unchanged. + * + * ⚠️ BREAKING (objectui#6272): a record stored in the deprecated + * `{ latitude, longitude }` spelling — including one this widget itself wrote + * before the flip — now renders EMPTY here until it is re-saved or fixed at the + * data layer. It keeps rendering correctly in detail views and on the map, + * which read the spec spelling first. That cost was presented and accepted as + * part of the A1 ruling; it is not an oversight to route around with a shim. + */ +export function LocationField({ value, onChange, field, readonly, error, ...props }: FieldWidgetComponentProps) { const config = field; - // Location is stored as { latitude, longitude } object - // For display, convert to "latitude, longitude" string format - const displayValue = value && typeof value === 'object' - ? `${value.latitude || 0}, ${value.longitude || 0}` - : ''; + // For display, convert the stored pair to a "lat, lng" string. + const displayValue = isLocationValue(value) ? `${value.lat}, ${value.lng}` : ''; if (readonly) { return {displayValue || }; @@ -25,14 +75,14 @@ export function LocationField({ value, onChange, field, readonly, error, ...prop onChange(null); return; } - + // Parse as coordinates (latitude, longitude) const parts = val.split(',').map(p => p.trim()); if (parts.length === 2) { const lat = parseFloat(parts[0]); const lng = parseFloat(parts[1]); if (!isNaN(lat) && !isNaN(lng)) { - onChange({ latitude: lat, longitude: lng }); + onChange({ lat, lng }); } // If invalid, don't update the value } diff --git a/packages/plugin-detail/src/__tests__/InlineFieldInput.composite.test.tsx b/packages/plugin-detail/src/__tests__/InlineFieldInput.composite.test.tsx index 7bc6648c4d..7a9eb3f5b7 100644 --- a/packages/plugin-detail/src/__tests__/InlineFieldInput.composite.test.tsx +++ b/packages/plugin-detail/src/__tests__/InlineFieldInput.composite.test.tsx @@ -163,11 +163,19 @@ describe('InlineFieldInput — structured composite values (objectui#4216)', () }); describe('location', () => { + // ⚠️ `location` values here are the SPEC spelling `{ lat, lng }` + // (objectui#6272). `@objectstack/spec/data` exports `LocationValue = { lat, + // lng, altitude?, accuracy? }` as canonical and `valueSchemaFor({ type: + // 'location' })` REJECTS `{ latitude, longitude }`, so these assertions used + // to pin — on both the read and the produce side — a shape the platform's own + // contract refuses. `geolocation` below is deliberately NOT the same case: it + // is not a member of the spec's closed `FieldType` union, so it keeps its own + // `{ latitude, longitude }` value shape. it('renders the coordinate pair, never "[Object]"', () => { render( , ); @@ -180,7 +188,7 @@ describe('InlineFieldInput — structured composite values (objectui#4216)', () render( , ); @@ -190,7 +198,7 @@ describe('InlineFieldInput — structured composite values (objectui#4216)', () expect(onChange).toHaveBeenCalledTimes(1); const next = onChange.mock.calls[0][0]; expect(typeof next).toBe('object'); - expect(next).toEqual({ latitude: 40.7128, longitude: -74.006 }); + expect(next).toEqual({ lat: 40.7128, lng: -74.006 }); }); it('never emits a bare STRING that replaces the whole object', () => { @@ -198,7 +206,7 @@ describe('InlineFieldInput — structured composite values (objectui#4216)', () const { container } = render( , );