From cf8127f391d2b9766c30f9853b5383317cf34aba Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 14:59:47 +0000 Subject: [PATCH] =?UTF-8?q?fix(runner):=20=E7=AB=99=E5=86=85=E5=AF=BC?= =?UTF-8?q?=E8=88=AA=E4=BF=9D=E7=95=99=20query=20string,`=3Fapi=3D`=20?= =?UTF-8?q?=E4=B8=8D=E5=86=8D=E8=A2=AB=E6=8A=B9=E6=8E=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `handleNavigate` 原先 `pushState({}, '', to)` 推的是侧栏给的裸路径, 地址栏上的 query 在第一次点击后就没了。memo 住的 loader 让当前会话 看起来正常,但 F5 或把 URL 分享出去时 `params.get('api')` 读到 null, 静默退回(正常安装里为空的)LocalBundleLoader,页面变成 Page not found, 而现场没有任何东西指认「API 基址丢了」。 按 ADR-0054 C3「可寻址状态放 URL」,导航时把当前 query string 一并带走。 保留的是**整个** query,不是只挑 `api`:runner 自己只读 `api` (`App.tsx`),但它渲染的树里 `@object-ui/core` 的 `parseDebugFlags` / `@object-ui/react` 的 `useDebugMode` 同样直接读 `window.location.search` (`?__debug`、`?__debug_schema` 等),只搬 `api` 会把第二类参数留在原地 继续丢 —— 同一种静默丢失,只是爆炸半径小一点。 目标路径若自带 query 则原样保留、同名参数以它为准,其余当前参数并在其后, 绝不拼出 `path?a=1?b=2` 这种畸形 URL。读取侧无需改动:`currentPath` 初值 与 popstate 都只读 `location.pathname`,不受 query 影响。 Fixes #3578 --- .../runner-preserve-query-on-navigate.md | 5 + content/docs/utilities/runner.mdx | 14 +- packages/runner/README.md | 6 +- packages/runner/src/App.navigation.test.tsx | 148 ++++++++++++++++++ packages/runner/src/App.tsx | 48 +++++- packages/runner/tsconfig.test.json | 7 +- 6 files changed, 219 insertions(+), 9 deletions(-) create mode 100644 .changeset/runner-preserve-query-on-navigate.md create mode 100644 packages/runner/src/App.navigation.test.tsx diff --git a/.changeset/runner-preserve-query-on-navigate.md b/.changeset/runner-preserve-query-on-navigate.md new file mode 100644 index 0000000000..ee60632f95 --- /dev/null +++ b/.changeset/runner-preserve-query-on-navigate.md @@ -0,0 +1,5 @@ +--- +'@object-ui/runner': patch +--- + +Runner in-app navigation now carries the current query string across to the pushed URL instead of `pushState`-ing a bare path. Opening the Runner with `?api=` and clicking a sidebar entry no longer drops the parameter from the address bar, so reloading or sharing the resulting URL still reaches the same backend rather than silently falling back to the (normally empty) `LocalBundleLoader` and rendering `Page not found`. The whole query string is preserved, not just `api` — `@object-ui/core`'s `?__debug…` flags survive navigation for the same reason. A navigation target that spells out its own query keeps it and wins on collision, with the remaining current parameters merged in behind it (#3578). diff --git a/content/docs/utilities/runner.mdx b/content/docs/utilities/runner.mdx index 02b0aabd75..04bda3e5a1 100644 --- a/content/docs/utilities/runner.mdx +++ b/content/docs/utilities/runner.mdx @@ -134,11 +134,15 @@ browser path plus `.json`, with `/` rewritten to `/index` first. into `null`. The Runner then shows `Page not found: ` for a page, and for a failed `app.json` it drops the app chrome (header/sidebar) and renders the page on its own. The HTTP status never reaches the UI — read it from the Network tab. -- **Read once, at mount.** The parameter is captured in a `useMemo` with an empty - dependency list. In-app navigation calls `history.pushState` with the bare path, so - the address bar loses `?api=…` after the first click; the loader already created - keeps serving that session, but reloading or sharing the resulting URL falls back to - the local bundle. +- **Read once, at mount — and it survives navigation.** The parameter is captured in + a `useMemo` with an empty dependency list, so editing `?api=…` in the address bar + does nothing to the running session until you reload. In-app navigation pushes the + target path with the current query string carried over, so `?api=…` stays in the + address bar after a sidebar click, and reloading or sharing that URL reaches the + same backend. The whole query string rides along, not just `api` — that also keeps + `@object-ui/core`'s `?__debug…` flags alive across navigation. A navigation target + that spells out its own query keeps it and wins on collision; the remaining current + parameters are merged in behind it. - `NetworkLoader`'s own default base is `/api` (`constructor(baseUrl: string = '/api')`). That default only applies when the class is constructed directly in code — the Runner always passes it the query-parameter value. diff --git a/packages/runner/README.md b/packages/runner/README.md index d9be98b96c..a7f5835610 100644 --- a/packages/runner/README.md +++ b/packages/runner/README.md @@ -88,8 +88,10 @@ Without the parameter, `LocalBundleLoader` resolves `src/app-data/app.json` and git-ignored and absent from a fresh checkout, so every load returns `null` until you copy or symlink your own metadata directory into it. -Full details — route resolution order, error handling, and the caveat that in-app -navigation drops `?api=` from the address bar — are in the +In-app navigation carries the query string across, so `?api=…` stays in the address +bar and the URL you copy or reload reaches the same backend. + +Full details — route resolution order and error handling — are in the [Metadata Loading](https://www.objectui.org/docs/utilities/runner#metadata-loading) section of the docs. diff --git a/packages/runner/src/App.navigation.test.tsx b/packages/runner/src/App.navigation.test.tsx new file mode 100644 index 0000000000..4518fde701 --- /dev/null +++ b/packages/runner/src/App.navigation.test.tsx @@ -0,0 +1,148 @@ +/** + * 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. + */ + +/** + * In-app navigation must carry the URL's query string with it (objectui#3578). + * + * The Runner keeps two families of settings in the query string, and both are + * addressable state under ADR-0054 C3 (shared, and expected back after a + * reload): + * + * - `?api=` — the metadata API base, read once at mount by `App.tsx`'s + * loader `useMemo`. + * - `?__debug`, `?__debug_schema`, … — `@object-ui/core`'s debug flags, read + * from `window.location.search` by `parseDebugFlags` / `useDebugMode` in the + * tree the Runner renders. + * + * `handleNavigate` used to `pushState` a bare path, which dropped every one of + * them from the address bar on the first sidebar click. The already-memoised + * loader kept serving that session, so the loss was invisible until F5 (or a + * colleague opening the copied URL) fell back to the empty `LocalBundleLoader` + * and rendered `Page not found`. + * + * These assertions drive the real component so they are meaningful against the + * unmodified source — see the PR body for the recorded red run. + * + * The metadata loader is stubbed because `src/app-data/` is git-ignored and + * absent from a fresh checkout: `loadAppConfig` returns an app document with a + * sidebar menu (the reported repro path), and `loadPage` returns `null` so the + * main area falls to the built-in 404 block instead of the ComponentRegistry + * (which would make this a `dom-heavy` test for no added coverage). + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { act, render, screen, fireEvent } from '@testing-library/react'; + +vi.mock('./lib/MetadataLoader', () => { + const APP_CONFIG = { + name: 'runner_demo_app', + title: 'Runner Demo', + layout: 'sidebar', + menu: [ + { label: 'Customers', path: '/customers' }, + // A nav target that declares its own query. No metadata in this repo + // spells one today, but plain concatenation would turn it into the + // malformed `/orders?tab=open?api=…`, so the merge branch is pinned. + { label: 'Open orders', path: '/orders?tab=open' }, + ], + }; + class StubLoader { + async loadAppConfig() { + return APP_CONFIG; + } + async loadPage() { + return null; + } + } + return { LocalBundleLoader: StubLoader, NetworkLoader: StubLoader }; +}); + +// Static, module-scope import: `App.tsx` pulls in `@object-ui/components` and +// the two plugin packages for their registration side effects, and that cost +// must not land inside a test's timeout budget (AGENTS.md §测试纪律). +import RunnerApp from './App'; + +const API_BASE = 'https://backend.example.com/api'; + +/** Click a sidebar entry and return the URL string handed to `pushState`. */ +async function navigateVia(label: string): Promise { + const pushState = vi.spyOn(window.history, 'pushState'); + try { + const link = await screen.findByRole('link', { name: label }); + // `act` around the click so the page reload the navigation kicks off + // settles inside it, instead of updating state after the test returns. + await act(async () => { + fireEvent.click(link); + }); + expect(pushState).toHaveBeenCalledTimes(1); + return String(pushState.mock.calls[0][2]); + } finally { + pushState.mockRestore(); + } +} + +describe('RunnerApp in-app navigation', () => { + beforeEach(() => { + window.history.replaceState({}, '', '/'); + }); + + afterEach(() => { + vi.restoreAllMocks(); + window.history.replaceState({}, '', '/'); + }); + + it('carries `?api=` across a sidebar navigation so reload and share still work', async () => { + window.history.replaceState({}, '', `/?api=${API_BASE}`); + + render(); + const pushed = await navigateVia('Customers'); + + expect(pushed).toBe(`/customers?api=${API_BASE}`); + expect(window.location.pathname).toBe('/customers'); + // What a reload would read back. + expect(new URLSearchParams(window.location.search).get('api')).toBe(API_BASE); + }); + + it('carries the WHOLE query string, not just `api`', async () => { + // `__debug_schema` is `@object-ui/core`'s debug flag (parseDebugFlags), the + // Runner's second query-string consumer. + window.history.replaceState({}, '', `/?api=${API_BASE}&__debug_schema=&lang=de`); + + render(); + const pushed = await navigateVia('Customers'); + + expect(pushed).toBe(`/customers?api=${API_BASE}&__debug_schema=&lang=de`); + }); + + it('appends no stray `?` when the URL has no query string', async () => { + window.history.replaceState({}, '', '/'); + + render(); + const pushed = await navigateVia('Customers'); + + expect(pushed).toBe('/customers'); + expect(pushed).not.toContain('?'); + expect(window.location.search).toBe(''); + }); + + it('merges into a target that declares its own query instead of concatenating', async () => { + window.history.replaceState({}, '', `/?api=${API_BASE}`); + + render(); + const pushed = await navigateVia('Open orders'); + + // The target's own `tab` is kept and the preserved `api` is merged in + // behind it. This branch rebuilds the query through `URLSearchParams`, so + // the value comes out canonically percent-encoded rather than verbatim — + // a different spelling of the same parameter, not a different parameter. + expect(pushed).toBe('/orders?tab=open&api=https%3A%2F%2Fbackend.example.com%2Fapi'); + expect(new URLSearchParams(window.location.search).get('api')).toBe(API_BASE); + // Never the malformed shape a bare `to + location.search` would produce. + expect(pushed.match(/\?/g)).toHaveLength(1); + }); +}); diff --git a/packages/runner/src/App.tsx b/packages/runner/src/App.tsx index 039134064d..64050d3e34 100644 --- a/packages/runner/src/App.tsx +++ b/packages/runner/src/App.tsx @@ -18,6 +18,50 @@ import { useState, useEffect, useMemo, useCallback } from 'react'; import { LayoutRenderer } from './LayoutRenderer'; import { LocalBundleLoader, NetworkLoader, MetadataLoader } from './lib/MetadataLoader'; +/** + * Build the URL an in-app navigation to `to` should push, carrying the query + * string that is currently in the address bar along with it (objectui#3578). + * + * The Runner keeps addressable state (ADR-0054 C3) in the query string, and + * more than one kind of it: `?api=` selects the metadata loader below, + * and `@object-ui/core`'s `?__debug…` flags are read straight off + * `window.location.search` by the tree this app renders. Preserving the whole + * string rather than re-emitting a single known parameter keeps both working — + * and keeps working for whatever else starts reading the query later. + * + * `to` normally is a bare path from the app's navigation metadata, in which + * case the current query string is carried over verbatim (no re-encoding, so + * the address bar keeps exactly what the user typed). A target that declares + * its own query keeps it and wins on collision; the remaining current + * parameters are merged in behind it, so the result is never the malformed + * `path?a=1?b=2` that plain concatenation would produce. That merge rebuilds + * the query through `URLSearchParams`, so values come out canonically + * percent-encoded — the same parameters, spelled the standard way. + */ +function withPreservedQuery(to: string, currentSearch: string): string { + if (!currentSearch || currentSearch === '?') return to; + + const hashIndex = to.indexOf('#'); + const hash = hashIndex === -1 ? '' : to.slice(hashIndex); + const pathAndQuery = hashIndex === -1 ? to : to.slice(0, hashIndex); + const queryIndex = pathAndQuery.indexOf('?'); + + if (queryIndex === -1) { + const search = currentSearch.startsWith('?') ? currentSearch : `?${currentSearch}`; + return `${pathAndQuery}${search}${hash}`; + } + + const merged = new URLSearchParams(pathAndQuery.slice(queryIndex + 1)); + // Snapshot the target's own keys first: appending while testing `merged.has` + // would swallow the second value of a repeated preserved parameter. + const ownKeys = new Set(merged.keys()); + for (const [key, value] of new URLSearchParams(currentSearch)) { + if (!ownKeys.has(key)) merged.append(key, value); + } + const search = merged.toString(); + return `${pathAndQuery.slice(0, queryIndex)}${search ? `?${search}` : ''}${hash}`; +} + /** * Root component of the standalone SDUI runner: loads an app config plus the * page for the current path and hands both to ``. @@ -71,7 +115,9 @@ export default function RunnerApp() { // --- 2. Route Handling --- const handleNavigate = useCallback((to: string) => { - window.history.pushState({}, '', to); + // The route is `to`; the query string rides along so that reloading or + // sharing the resulting URL still resolves the same backend (#3578). + window.history.pushState({}, '', withPreservedQuery(to, window.location.search)); setCurrentPath(to); window.scrollTo(0, 0); }, []); diff --git a/packages/runner/tsconfig.test.json b/packages/runner/tsconfig.test.json index 1edf1cd83f..c3b5e57381 100644 --- a/packages/runner/tsconfig.test.json +++ b/packages/runner/tsconfig.test.json @@ -12,7 +12,12 @@ // Drop the root tsconfig's source-tree `paths` so `@object-ui/*` and // `@objectstack/spec` resolve through the workspace dependency's built // `.d.ts` instead of pulling sibling sources in as program inputs (TS6059). - "paths": {} + "paths": {}, + // Same ambient types as `tsconfig.json`. A test that imports `App.tsx` + // pulls `lib/MetadataLoader.ts` into this program, and that file calls + // `import.meta.glob` — declared by `vite/client`, which the root config + // this extends does not carry (it is not a Vite app). + "types": ["vite/client"] }, "include": ["src/**/*.test.ts", "src/**/*.test.tsx"] }