Skip to content
Open
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: 4 additions & 1 deletion packages/ui-next/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,10 @@ function hydroPlugins(): Plugin {
}
if (!entries.length) return 'export default [];';
const imports = entries.map((e, i) => `import * as plugin${i} from '${e}';`).join('\n');
const exports = `export default [${entries.map((_, i) => `plugin${i}`).join(', ')}];`;
const exports = `export default [${entries.map((e, i) => {
const addonName = path.basename(path.resolve(e, '..', '..'));
return `{ name: '${addonName}', ...plugin${i} }`;
}).join(', ')}];`;
Comment on lines +52 to +55
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Make generated plugin name deterministic and safely escaped.

On Lines 52-55 and Lines 128-131, { name: '...', ...plugin${i} } lets plugin${i}.name overwrite the addon-derived name, and raw string interpolation can break generated JS for quoted names.

💡 Suggested patch
-const exports = `export default [${entries.map((e, i) => {
-    const addonName = path.basename(path.resolve(e, '..', '..'));
-    return `{ name: '${addonName}', ...plugin${i} }`;
-}).join(', ')}];`;
+const exports = `export default [${entries.map((e, i) => {
+    const addonName = JSON.stringify(path.basename(path.resolve(e, '..', '..')));
+    return `{ ...plugin${i}, name: ${addonName} }`;
+}).join(', ')}];`;
-`window.__hydroPlugins = [${entries.map((e, i) => {
-    const addonName = path.basename(path.resolve(e, '..', '..'));
-    return `{ name: '${addonName}', ...plugin${i} }`;
-}).join(', ')}];`,
+`window.__hydroPlugins = [${entries.map((e, i) => {
+    const addonName = JSON.stringify(path.basename(path.resolve(e, '..', '..')));
+    return `{ ...plugin${i}, name: ${addonName} }`;
+}).join(', ')}];`,

Also applies to: 128-131

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/ui-next/index.ts` around lines 52 - 55, The generated plugin entries
use raw interpolation and place name before the plugin spread so plugin${i}.name
can overwrite it and unescaped addon names can break JS; change the construction
to ensure the addon-derived name is safely JSON-escaped (e.g. use
JSON.stringify(addonName) when inserting the literal) and make the name property
come after the plugin spread so it cannot be overridden (e.g. produce an entry
like `{ ...plugin${i}, name: <escaped-addonName> }` or use Object.assign({},
plugin${i}, { name: <escaped-addonName> }) ); apply the same fix to the other
occurrence around lines 128-131.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This expected behaviour.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@renbaoshuo, understood — the placement of name before the plugin spread is intentional so that individual plugins can override the addon-derived name. I'll note this for future reviews.

( `∪。∪。) ~~


✏️ Learnings added
Learnt from: renbaoshuo
Repo: hydro-dev/Hydro PR: 1159
File: packages/ui-next/index.ts:52-55
Timestamp: 2026-05-10T15:27:48.125Z
Learning: In `packages/ui-next/index.ts`, the virtual plugin module generates entries as `{ name: '<addonName>', ...plugin${i} }` (name first, spread second) on purpose: this allows individual plugin modules to override the addon-derived `name` property by exporting their own `name`. The same pattern applies to the `buildPlugins()` esbuild bundle (`window.__hydroPlugins`). Do not flag this ordering as a bug.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

return `${imports}\n${exports}`;
}
return undefined;
Expand Down
39 changes: 29 additions & 10 deletions packages/ui-next/src/app.tsx
Original file line number Diff line number Diff line change
@@ -1,18 +1,37 @@
import { Link } from './components/link';
import { Suspense, useMemo, useSyncExternalStore } from 'react';
import { usePageData } from './context/page-data';
import { defineSlot } from './registry';
import { defineSlot, SlotName } from './registry';
import { SlotErrorBoundary } from './registry/error-boundary';
import { store } from './registry/store';

const AppInner = defineSlot('page:app', () => {
const data = usePageData();
const App = defineSlot('app:root', () => {
const { name } = usePageData();
const slotName: SlotName = `page:${name}`;

return (
<>
<div>app, page:{data.name}</div>
const [subscribe, getSnapshot] = useMemo(() => [
(cb: () => void) => store.subscribe(slotName, cb),
() => store.getVersion(slotName),
], [slotName]);

useSyncExternalStore(subscribe, getSnapshot);

const Comp = store.getDefault(slotName);

if (!Comp) {
return (
<div>
<Link to="homepage">homepage</Link> <Link to="problem_main">problem_main</Link>
Page not found: <code>{name}</code>
</div>
</>
);
}

return (
<SlotErrorBoundary slotName={slotName} label="renderer">
<Suspense fallback={<div>Loading...</div>}>
<Comp />
</Suspense>
</SlotErrorBoundary>
);
});

export default AppInner;
export default App;
5 changes: 5 additions & 0 deletions packages/ui-next/src/components/layout.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { defineSlot } from '../registry';

const Layout = defineSlot('app:layout', ({ children }: React.PropsWithChildren) => <>{children}</>);

export default Layout;
1 change: 0 additions & 1 deletion packages/ui-next/src/globals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ export const isInjected: boolean = !!injectionData.HYDRO_INJECTED;
export const hydroDomains: string[] = injectionData.hydro_domains ?? [];
export const pluginsUrl: string | undefined = injectionData.plugins_url;

// routeMap as an external store for useSyncExternalStore, with HMR state preservation
interface RouteMapStore {
_routeMap: Record<string, string>;
_listeners: Set<() => void>;
Expand Down
2 changes: 2 additions & 0 deletions packages/ui-next/src/main.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import './pages';

import { StrictMode } from 'react';
import { createRoot } from 'react-dom/client';
import * as api from './api';
Expand Down
19 changes: 19 additions & 0 deletions packages/ui-next/src/pages/homepage.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { Link } from '../components/link';

export default function Homepage() {
return (
<div>
<div>homepage</div>
<Link to="problem_main">problem_main</Link>
</div>
);
}

export function Layout({ children }: React.PropsWithChildren) {
return (
<div>
{children}
<div>layout test</div>
</div>
);
}
4 changes: 4 additions & 0 deletions packages/ui-next/src/pages/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { registerPage } from '../registry/page';

registerPage('homepage', () => import('./homepage'));
registerPage('problem_main', () => import('./problem_main'));
10 changes: 10 additions & 0 deletions packages/ui-next/src/pages/problem_main.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { Link } from '../components/link';

export default function ProblemMain() {
return (
<div>
<div>problem_main</div>
<Link to="homepage">homepage</Link>
</div>
);
}
1 change: 1 addition & 0 deletions packages/ui-next/src/registry/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
export { SlotErrorBoundary } from './error-boundary';
export { after, before, intercept, patch, replace, wrap } from './interceptors';
export { registerPage } from './page';
export { installPlugin } from './plugin';
export type { PluginAPI, PluginDefinition } from './plugin';
export { defineSlot } from './slot';
Expand Down
28 changes: 28 additions & 0 deletions packages/ui-next/src/registry/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import React, { lazy } from 'react';
import Layout from '../components/layout';
import { store } from './store';

interface PageModule<P = any> {
default: React.ComponentType<P>;
Layout?: React.ComponentType<React.PropsWithChildren>;
}

type PageLoader<P = any> = () => Promise<PageModule<P>>;

export function registerPage<P = any>(name: string, loader: PageLoader<P>) {
store.setDefault(`page:${name}`, lazy(() =>
loader().then((mod) => {
const PageComp = mod.default;
const LayoutComp = mod.Layout || Layout;
return {
default(props: React.PropsWithChildren<P>) {
return (
<LayoutComp>
<PageComp {...props} />
</LayoutComp>
);
},
};
}),
));
}
4 changes: 3 additions & 1 deletion packages/ui-next/src/registry/plugin.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { after, before, intercept, patch, replace, wrap } from './interceptors';
import { registerPage } from './page';

export interface PluginAPI {
intercept: typeof intercept;
Expand All @@ -7,10 +8,11 @@ export interface PluginAPI {
patch: typeof patch;
replace: typeof replace;
wrap: typeof wrap;
registerPage: typeof registerPage;
}

export function createPluginAPI(): PluginAPI {
return { intercept, before, after, patch, replace, wrap };
return { intercept, before, after, patch, replace, wrap, registerPage };
}

export interface PluginDefinition {
Expand Down
1 change: 1 addition & 0 deletions packages/ui-next/src/registry/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ function createRegistryStore() {

function setDefault(name: SlotName, comp: React.FC<any>) {
state.defaults[name] = comp;
bumpVersion(name);
}

function getDefault(name: SlotName): React.FC<any> | undefined {
Expand Down
2 changes: 1 addition & 1 deletion packages/ui-next/src/registry/types.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
type Scope = 'page' | 'component' | 'layout' | string;
type Scope = 'app' | 'page' | 'component' | string;
export type SlotName = `${Scope}:${string}`;

export type Interceptor<P = any> = (
Expand Down
Loading