diff --git a/.changeset/spec-bridge-export-options-lift-4585.md b/.changeset/spec-bridge-export-options-lift-4585.md new file mode 100644 index 000000000..3912d1cda --- /dev/null +++ b/.changeset/spec-bridge-export-options-lift-4585.md @@ -0,0 +1,33 @@ +--- +'@object-ui/react': patch +--- + +SpecBridge lifts a legacy bare `exportOptions` array to the spec's object form, so a +spec-authored view's declared export formats reach the grid (objectui#4585). + +A spec `ListView` may spell `exportOptions` either way, and `@objectstack/spec` lifts the +legacy bare format array to `{ formats: [...] }` when it parses one (objectstack#8010). +That lift never ran on the bridge path: the bridge's input is a TypeScript type, not a +parsed value — there is no `parse`/`safeParse` anywhere under `spec-bridge/` — so a host +forwarding raw stored metadata handed the array straight through, and the bridge copied it +onto the `object-grid` node verbatim. ObjectGrid reads the object form and only that, and +`.formats` on an array is `undefined`, so the renderer's `['csv', 'json']` default won. + +A view declaring `['csv', 'xlsx']` therefore rendered an export menu offering CSV and +JSON: the declared xlsx never appeared, an undeclared json did, and nothing said so — the +export button still showed, because a non-empty array is truthy. The bridge now applies +the spec's own transform at the assignment site, so both spellings leave it as one shape. + +Deliberately narrow: this mirrors the spec's lift and nothing else. The object form passes +through by reference, unread and unrewritten; a view with no `exportOptions` is untouched; +and a `'pdf'` stored before its retirement is carried rather than filtered, because the +spec refuses that value at parse with a migration prescription instead of silently +dropping it — such a format still dies downstream in ObjectGrid's format-agnostic menu +filter (objectui#4535). The fix is at the producer for the same reason: a tolerant +`Array.isArray` fallback in the renderer would make a second de-facto contract out of one +spec key. + +One behavior follows from reading the lift literally: `exportOptions: []` now lifts to +`{ formats: [] }` and the export button is hidden, where before the unreadable `[]` was +merely truthy and produced a menu built entirely from the `['csv', 'json']` default. A +view that declares zero formats now offers zero. diff --git a/packages/plugin-grid/src/__tests__/specBridgeExportFormats.test.tsx b/packages/plugin-grid/src/__tests__/specBridgeExportFormats.test.tsx new file mode 100644 index 000000000..db25043ae --- /dev/null +++ b/packages/plugin-grid/src/__tests__/specBridgeExportFormats.test.tsx @@ -0,0 +1,153 @@ +/** + * 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. + */ + +/** + * SpecBridge to ObjectGrid — a spec-authored view's declared export formats + * reach the menu (objectui#4585). + * + * This is the card's repro, end to end and in one process: build a spec + * `ListView` that declares `exportOptions: ['csv', 'xlsx']`, route it through + * `SpecBridge`, render the resulting node. Before the fix the bridge copied the + * bare array to the node verbatim, ObjectGrid read `exportOptions.formats` off + * an array (`undefined`) and fell back to its `['csv', 'json']` default — so the + * menu offered CSV and JSON: the declared xlsx never appeared and an undeclared + * json did, silently. The bridge now applies the spec's own parse-time lift + * (objectstack#8010), so the declared set is what the renderer sees. + * + * The two levels are pinned separately on purpose: the bridge's output shape in + * `@object-ui/react`'s `ListViewExportOptionsLift.test.ts`, and the rendered + * consequence here. The old bridge pin was green precisely because it asserted + * a shape it never rendered — this file is the half that could not have been. + * + * It lives in plugin-grid because the dependency direction decides: + * `@object-ui/plugin-grid` depends on `@object-ui/react`, so it can see both + * `SpecBridge` and `ObjectGrid`; react cannot import the grid without inverting + * the graph. `ObjectGrid` itself is untouched read-only context here — the + * server-stream gate below is landed behavior (objectui#2942 / #4535), reused, + * not modified. + */ +import { describe, it, expect, vi, beforeAll } from 'vitest'; +import { render, screen, fireEvent } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import React from 'react'; + +import { ObjectGrid } from '../ObjectGrid'; +import { registerAllFields } from '@object-ui/fields'; +import { ActionProvider, SpecBridge } from '@object-ui/react'; + +registerAllFields(); + +beforeAll(() => { + // jsdom has no object-URL plumbing; the download path calls these. + if (!URL.createObjectURL) (URL as any).createObjectURL = () => 'blob:export'; + if (!URL.revokeObjectURL) (URL as any).revokeObjectURL = () => {}; +}); + +/** + * `withStream: true` reproduces a server-backed grid — `exportDownload` present + * — which is the only configuration in which xlsx is deliverable. Without it + * the client fallback path supports csv and json alone, and a declared xlsx is + * dropped from the menu by ObjectGrid's own gate. + */ +function makeDataSource(withStream: boolean) { + const ds: Record = { + find: vi.fn(async () => ({ data: [], total: 0, hasMore: false, pageSize: 50 })), + getObjectSchema: async (name: string) => ({ + name, + fields: { id: { type: 'text' }, name: { type: 'text' } }, + }), + }; + if (withStream) { + ds.exportDownload = vi.fn().mockResolvedValue(new Blob(['ID,Name\n'], { type: 'text/csv' })); + } + return ds as any; +} + +/** Author a spec ListView, bridge it, render the node the bridge produced. */ +function renderBridgedView(exportOptions: unknown, withStream: boolean) { + const node = new SpecBridge().transformListView({ + name: 'tasks_all', + label: 'All Tasks', + columns: [{ field: 'name', label: 'Name' }], + exportOptions, + }); + + // `objectName` is the host's binding, not the view's — the bridge maps the + // ListView's own keys. Everything else is the bridge's output, untouched. + return render( + + + , + ); +} + +/** Opens the export popover; the trigger is the only `/^export$/i` button. */ +async function openExportMenu() { + fireEvent.click(await screen.findByRole('button', { name: /^export$/i })); +} + +describe('SpecBridge to ObjectGrid — declared export formats (#4585)', () => { + it('offers the formats a legacy bare array declared, not the renderer default', async () => { + // The card's repro. Server stream available, so the declared xlsx is + // deliverable and must appear; json was never declared and must not. + renderBridgedView(['csv', 'xlsx'], true); + await openExportMenu(); + + expect(await screen.findByRole('button', { name: /export as csv/i })).toBeInTheDocument(); + expect(await screen.findByRole('button', { name: /export as xlsx/i })).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: /export as json/i })).not.toBeInTheDocument(); + }); + + it('still lets the server-stream gate drop a declared xlsx when there is no stream', async () => { + // Same declaration, client fallback only: the gate — landed behavior, not + // this change — keeps csv and drops xlsx. The lift decides what is + // DECLARED; the gate decides what is DELIVERABLE. Both halves must hold, or + // "declared formats now reach the menu" would just mean "the menu stopped + // filtering". + renderBridgedView(['csv', 'xlsx'], false); + await openExportMenu(); + + expect(await screen.findByRole('button', { name: /export as csv/i })).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: /export as xlsx/i })).not.toBeInTheDocument(); + expect(screen.queryByRole('button', { name: /export as json/i })).not.toBeInTheDocument(); + }); + + it('hides the export button entirely when the view declares an empty array', async () => { + // `[]` lifts to `{ formats: [] }` verbatim (the spec's transform wraps, it + // does not default), and ObjectGrid reads that literally: no format is + // offered, so `exportableFormats.length > 0` fails and the toolbar button + // is gone. Before the fix the bare `[]` was truthy but unreadable, so the + // button showed and offered the `['csv', 'json']` default — an export menu + // built entirely out of formats the view never declared. + renderBridgedView([], true); + + expect(await screen.findByText('All Tasks')).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: /^export$/i })).not.toBeInTheDocument(); + }); + + it('leaves a view with no exportOptions without an export button', async () => { + // Unchanged by this card, pinned so the lift cannot start inventing a + // declaration where the view made none. + renderBridgedView(undefined, true); + + expect(await screen.findByText('All Tasks')).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: /^export$/i })).not.toBeInTheDocument(); + }); + + it('renders the object form the same way — one shape reaches the menu', async () => { + // The object spelling was already correct and passes through by reference; + // this pins that both spellings land on the same rendered menu, which is + // what "the bridge emits one shape" has to mean downstream. + renderBridgedView({ formats: ['csv', 'xlsx'] }, true); + await openExportMenu(); + + expect(await screen.findByRole('button', { name: /export as csv/i })).toBeInTheDocument(); + expect(await screen.findByRole('button', { name: /export as xlsx/i })).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: /export as json/i })).not.toBeInTheDocument(); + }); +}); diff --git a/packages/react/src/spec-bridge/__tests__/ListViewExportOptionsLift.test.ts b/packages/react/src/spec-bridge/__tests__/ListViewExportOptionsLift.test.ts new file mode 100644 index 000000000..64bd08c59 --- /dev/null +++ b/packages/react/src/spec-bridge/__tests__/ListViewExportOptionsLift.test.ts @@ -0,0 +1,99 @@ +/** + * 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. + */ + +import { describe, it, expect } from 'vitest'; +import { SpecBridge } from '../SpecBridge'; + +/** + * The `exportOptions` lift pin (objectui#4585). + * + * The bridge used to copy `spec.exportOptions` across verbatim, so a legacy + * bare format array reached the `object-grid` node unchanged — and ObjectGrid + * reads the OBJECT form and only that (`schema.exportOptions?.formats`). + * `.formats` on an array is `undefined`, so the renderer's `['csv', 'json']` + * default won: a view declaring `['csv', 'xlsx']` offered csv and json, losing + * the declared xlsx and gaining an undeclared json, with no error, no warning + * and no console line. `!!schema.exportOptions` stayed truthy for a non-empty + * array, so the export button still showed — the failure was silent, not + * absent. + * + * The fix mirrors the spec's OWN parse-time lift and nothing more + * (`@objectstack/spec` `ui/view.zod.ts`, objectstack#8010): + * + * z.array(ListViewExportFormatSchema).transform((formats) => ({ formats })) + * + * so this is one contract applied where `parse` cannot reach, not a second one. + * The bridge's input is a TypeScript type, never a parsed value — there is no + * `parse`/`safeParse` anywhere under `spec-bridge/` — which is why bumping the + * spec pin alone would not have closed #4585. + * + * The lifted value is typed `ListViewExportOptions` from `@object-ui/types` + * (landed objectui#4535 / PR #4584) — one spelling of the spec's five-key + * shape, no third copy. That half is enforced by `tsc` on the bridge source, + * not by an assertion here. + * + * RESIDUE (recorded, not built — #4585 item 4). objectui pins + * `@objectstack/spec@17.0.0-rc.6`, where `ListView.exportOptions` is still the + * bare array and the lift does not exist yet; measured in the pinned dist: + * `exportOptions: z.array(z.enum(['csv','xlsx','pdf','json'])).optional()`. + * Once the pin bumps past objectstack#8324 the spec's schema becomes reachable + * from here, and a stronger assertion replaces the hand-mirroring below: parse + * the same input through the spec and require `bridge lift === spec parse + * output`, one contract proven equal rather than copied. Do not block on it. + */ + +/** What the bridge writes to the node, through the untyped host boundary. */ +function bridgedExportOptions(exportOptions: unknown): unknown { + return new SpecBridge().transformListView({ + name: 'export_view', + columns: [{ field: 'name', label: 'Name' }], + exportOptions, + }).exportOptions; +} + +describe('SpecBridge — exportOptions lift (#4585)', () => { + it('lifts a legacy bare format array to the spec object form', () => { + expect(bridgedExportOptions(['csv', 'xlsx'])).toEqual({ formats: ['csv', 'xlsx'] }); + }); + + it('lifts an empty array to `{ formats: [] }` verbatim, neither dropped nor defaulted', () => { + // The spec's `z.array()` carries no `.min(1)`, so `[]` is a legal input and + // its transform wraps it like any other. Downstream that reads as "this + // view declares no export format": ObjectGrid's menu comes out empty and + // the export button is hidden — pinned end-to-end in plugin-grid's + // `specBridgeExportFormats.test.tsx`. + expect(bridgedExportOptions([])).toEqual({ formats: [] }); + }); + + it('passes the object form through by reference, unread and unrewritten', () => { + const authored = { formats: ['csv', 'json'], maxRecords: 5000, streaming: false }; + + // Identity, not equality: the object spelling is already the contract, so + // the bridge must not rebuild, re-key or re-order it. + expect(bridgedExportOptions(authored)).toBe(authored); + }); + + it('leaves the key absent when the view declares no exportOptions', () => { + const node = new SpecBridge().transformListView({ + name: 'no_export_view', + columns: [{ field: 'name', label: 'Name' }], + }); + + expect('exportOptions' in node).toBe(false); + }); + + it('carries a pre-retirement `pdf` into the lift instead of filtering it out', () => { + // The spec REFUSES `'pdf'` at parse with a migration prescription + // (objectstack#8010; PDF export declined as objectstack#1301 NOT_PLANNED) — + // it does not silently drop the value, so the mirror of its lift may not + // either. Stored metadata predating `os migrate meta --from 16` still + // carries it, and it dies downstream in ObjectGrid's format-agnostic menu + // filter (objectui#4535) rather than in a `'pdf'`-shaped branch here. + expect(bridgedExportOptions(['csv', 'pdf'])).toEqual({ formats: ['csv', 'pdf'] }); + }); +}); diff --git a/packages/react/src/spec-bridge/__tests__/P1SpecBridge.test.ts b/packages/react/src/spec-bridge/__tests__/P1SpecBridge.test.ts index 5bb5c375e..279ffb427 100644 --- a/packages/react/src/spec-bridge/__tests__/P1SpecBridge.test.ts +++ b/packages/react/src/spec-bridge/__tests__/P1SpecBridge.test.ts @@ -387,14 +387,24 @@ describe('P1 SpecBridge Protocol Alignment', () => { expect(node.sharing.type).toBe('collaborative'); }); - it('should pass through exportOptions string[] format', () => { + // AUTHORIZED PIN MOVE (objectui#4585). This used to pin the bare array + // reaching the node verbatim — the one shape ObjectGrid cannot read, since + // it takes `exportOptions.formats` and `.formats` on an array is + // `undefined`. The pin was about the bridge's output shape and never + // rendered it, so the bridge stayed green while the view's declared formats + // were silently replaced by the renderer's `['csv', 'json']` default. The + // bridge now applies the spec's own parse-time lift, so the legacy spelling + // arrives in the shape the renderer reads. Full coverage of the lift lives + // in `ListViewExportOptionsLift.test.ts`; the end-to-end consequence is + // pinned in plugin-grid's `specBridgeExportFormats.test.tsx`. + it('should lift a legacy exportOptions array to the spec object form', () => { const bridge = new SpecBridge(); const node = bridge.transformListView({ name: 'export_spec', exportOptions: ['csv', 'xlsx'], }); - expect(node.exportOptions).toEqual(['csv', 'xlsx']); + expect(node.exportOptions).toEqual({ formats: ['csv', 'xlsx'] }); }); it('should pass through exportOptions object format', () => { diff --git a/packages/react/src/spec-bridge/bridges/list-view.ts b/packages/react/src/spec-bridge/bridges/list-view.ts index e72e4aac3..efb1c95fc 100644 --- a/packages/react/src/spec-bridge/bridges/list-view.ts +++ b/packages/react/src/spec-bridge/bridges/list-view.ts @@ -7,6 +7,7 @@ */ import type { SchemaNode } from '@object-ui/core'; +import type { ListViewExportFormat, ListViewExportOptions } from '@object-ui/types'; import type { BridgeContext, BridgeFn } from '../types'; import type { ListView, ListColumn, RowHeight } from '@objectstack/spec/ui'; @@ -113,6 +114,54 @@ function mapDensity( return ROW_HEIGHT_TO_DENSITY[rowHeight]; } +/** + * The spec's own parse-time lift, applied where `parse` cannot reach + * (objectui#4585). + * + * `@objectstack/spec` accepts BOTH spellings of `view.exportOptions` and lifts + * the legacy bare format array to the object form at parse (objectstack#8010, + * `ui/view.zod.ts`): + * + * z.array(ListViewExportFormatSchema).transform((formats) => ({ formats })) + * + * That transform never runs on this path. The bridge's input is a TypeScript + * type, not a parsed value — there is no `parse`/`safeParse` anywhere under + * `spec-bridge/` — so a host that parses first hands over the object form while + * a host that forwards raw stored metadata hands over whatever was authored, + * and nothing here can tell them apart. The legacy array therefore used to + * reach the `object-grid` node verbatim, where `ObjectGrid` reads + * `exportOptions.formats` and only that: `.formats` on an array is `undefined`, + * the renderer's `['csv', 'json']` default won, and the view's declared formats + * were dropped with no error and no console line — the export button still + * showed, because a non-empty array is truthy. + * + * This mirrors that transform and NOTHING more: the same contract applied one + * layer out, not a second de-facto one (AGENTS.md #0.1 — the consumer-side + * `Array.isArray` fallback in the renderer is what that rule forbids). Hence: + * + * - the object form passes through by reference, unread and unrewritten; + * - an empty array lifts to `{ formats: [] }`, exactly as the spec's transform + * does — its `z.array()` carries no `.min(1)`, so `[]` is a legal input that + * wraps rather than defaults. ObjectGrid then offers no format and hides the + * export button: "declared zero formats", read literally; + * - a `'pdf'` stored before its retirement is carried, not filtered. The spec + * REFUSES `'pdf'` at parse with a migration prescription (objectstack#8010; + * PDF export itself was declined as objectstack#1301 NOT_PLANNED) — it does + * not silently drop the value, so neither may this. Such a format dies + * downstream in ObjectGrid's format-AGNOSTIC menu filter, kept deliberately + * at objectui#4535 for metadata predating `os migrate meta --from 16`. + */ +function liftExportOptions( + exportOptions: NonNullable | ListViewExportOptions, +): ListViewExportOptions { + if (!Array.isArray(exportOptions)) return exportOptions; + // The pinned `@objectstack/spec@17.0.0-rc.6` still admits `'pdf'` as an array + // member (the enum narrowing arrives with the pin bump), so the element type + // here is wider than the renderer-side `ListViewExportFormat`. Narrowing it by + // filtering would invent precisely the silent drop the spec declines to do. + return { formats: exportOptions as ListViewExportFormat[] }; +} + /** Transforms a ListView spec into a DataTable SchemaNode */ export const bridgeListView: BridgeFn = ( spec: ListViewSpec, @@ -155,7 +204,7 @@ export const bridgeListView: BridgeFn = ( if (spec.virtualScroll != null) node.virtualScroll = spec.virtualScroll; if (spec.conditionalFormatting) node.conditionalFormatting = spec.conditionalFormatting; if (spec.inlineEdit != null) node.inlineEdit = spec.inlineEdit; - if (spec.exportOptions) node.exportOptions = spec.exportOptions; + if (spec.exportOptions) node.exportOptions = liftExportOptions(spec.exportOptions); if (spec.emptyState) node.emptyState = spec.emptyState; if (spec.userActions) node.userActions = spec.userActions; if (spec.appearance) node.appearance = spec.appearance;