Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/runner-preserve-query-on-navigate.md
Original file line number Diff line number Diff line change
@@ -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=<base>` 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).
14 changes: 9 additions & 5 deletions content/docs/utilities/runner.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -134,11 +134,15 @@ browser path plus `.json`, with `/` rewritten to `/index` first.
into `null`. The Runner then shows `Page not found: <path>` 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.
Expand Down
6 changes: 4 additions & 2 deletions packages/runner/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
148 changes: 148 additions & 0 deletions packages/runner/src/App.navigation.test.tsx
Original file line number Diff line number Diff line change
@@ -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=<base>` — 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<string> {
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(<RunnerApp />);
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(<RunnerApp />);
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(<RunnerApp />);
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(<RunnerApp />);
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);
});
});
48 changes: 47 additions & 1 deletion packages/runner/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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=<base>` 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 `<SchemaRenderer>`.
Expand Down Expand Up @@ -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);
}, []);
Expand Down
7 changes: 6 additions & 1 deletion packages/runner/tsconfig.test.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
}
Loading