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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
- `codegraph sync` and the file watcher now link a lazily loaded React Router route to its page when the page appears, or gains the export the route reads, in a folder below the one the route loads, like `src/pages/Home/index.tsx` behind `import('@/pages')`. Before, such a route kept linking nothing until the folder's own `index.ts` changed or the project was re-indexed, although a fresh index linked it.
- React Router routes are now indexed when the router gets its table from a function through a variable, as in Medusa's admin dashboard, which writes `const routes = getRouteMap({ … })` and then `createBrowserRouter(routes)`. The same goes for `export const routes = buildRoutes()` or `export default buildRoutes()` imported into `useRoutes(routes)`, and for `const routes = await loadRoutes()`. Before, such a variable was read only when it held the table itself, so these apps had no routes: their pages had no route leading to them, and their `navigate('/orders/…')` calls and `<Link>` links led nowhere. Routes the function is handed by its caller, like the plugin routes Medusa spreads in, are still not read, and when two functions in one file each keep a `routes` variable, the router reads its own and never the other's, which may be a menu. Re-index React projects after upgrading.
- A React Router route written as `element: <Outlet />` or `Component: Outlet`, or whose `lazy` loader hands back `{ Component: Outlet }`, now counts as rendering nothing of its own, as a route with no element does and as `<Route element={<Outlet />}>` already did. Before, `Outlet` was taken for the route's page and for the layout of the routes inside it, which linked nothing, and a loader returning `{ Component: Outlet }` made the module it loads for its `loader` the layout of the routes inside: in Medusa's admin dashboard, a product's detail page was drawn around its variants' pages. Such a route is still a route at its own address, inside the layouts around it, and a page inside it at the same address still takes the address over. Re-index React projects after upgrading.
- React Router routes are now read from every table a route-building function returns, an early `if (!user) return [];` and each branch alike, and never from a list that a function written inside it returns, like a `.forEach` callback's `return []` or a helper's menu. A function that returns another's table, as in `return buildRoutes()`, is followed too, and so is a table behind `useMemo`, like `useMemo(() => getRoutes(user), [user])` or `useMemo(getRoutes, [])`. Before, the first `return [` anywhere in the function was taken for the table, so an empty guard or a callback's list could hide an app's routes entirely, or a menu could be read in their place. Re-index React projects after upgrading.
- In JavaScript and TypeScript, an import written `import { default as AppRoot } from './routes/app/root'`, as in bulletproof-react's router, now counts as the module's default import, the same as `import AppRoot from './routes/app/root'`. Before, it was read as an import of an export named `default`, which no module has, so a call, a route or a JSX attribute that used `AppRoot` was matched by its name alone: it could link to another file's `AppRoot`, or to nothing when the default export has a name of its own. Svelte, Vue and Astro script blocks are read the same way.
- In React, a component's JSX no longer links it to a class or component that only shares a name with one of its type arguments or with a variable of its own. A type in angle brackets, like `Document` in `<PaginatedList<Document> items={…} />`, `User` in `useState<User>()` or `Entry` in a generic `<Entry extends BaseEntity>(…) =>` component, is no longer read as a tag. A tag naming a variable or parameter the component sets itself, like `<Content>` after `const Content = isDropdown ? DropdownMenu.SubContent : ContextMenu.SubContent` or `<Widget>` in `widgets.map((Widget) => <Widget />)`, now links to nothing, unless the component declares a component of that name inside itself. Before, these linked to an unrelated class or component elsewhere in the repository: outline's document lists showed up among the callers of its `Document` model class, and its menus among the callers of the command bar's `Content`, so `codegraph_explore`, callers and impact followed renders that never happen. Re-index React projects after upgrading.
- In React, a tag a component only mentions in a comment or a string no longer links it to that component: a JSDoc example, markup commented out like `{/* <Progress /> */}`, or an error message like react-router's "You cannot render a <Router> inside another <Router>" or shadcn's "useCarousel must be used within a <Carousel />". Nothing in a TypeScript `.ts` file, which can't hold JSX, is read as a tag either, like the XML in outline's OpenSearch template. Before, these counted as renders, so react-router's hooks showed up among the callers of `Router`, `Routes` and `Route`, and `codegraph_explore`, callers and impact followed renders that never happen. A tag after an apostrophe, a URL or a backtick in a component's text, like `<p>Don't have an account? <Link to="/signup">`, still links as before. Re-index React projects after upgrading.
Expand Down
210 changes: 210 additions & 0 deletions __tests__/react-router-returned-tables.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,210 @@
/**
* What a route-building function returns, when the router is handed its
* result: every `return` of its own, never one in a function written inside
* it. Before, the first `return [` anywhere in the body was the table: the
* empty array an early guard (`if (!user) return [];`) or a `.forEach`
* callback hands back (devanshthakker15/BANKFORM1's `generateRoutes`), so
* the real table was never read; a menu a helper inside returns could be read
* instead; and only the first of two branches' tables. A function that
* returns another's result (`return buildRoutes()`) was not followed, and
* neither was a table behind `useMemo` (`useMemo(() => getRoutes(user),
* [user])`, `useMemo(getRoutes, [])`), which hooks-based routers write.
*/
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
import * as fs from 'fs';
import * as os from 'os';
import * as path from 'path';
import { CodeGraph } from '../src';

const pages = ['Home', 'Login', 'Orders', 'OrderDetail', 'Admin', 'Account', 'Reports', 'Help', 'Faq', 'Settings', 'Dashboard', 'Landing', 'NotFound'];

const FILES: Record<string, string> = {
'package.json': JSON.stringify({ name: 'apps', dependencies: { react: '^18', 'react-router-dom': '^6' } }),
'src/pages.jsx': pages.map((p) => `export function ${p}() {\n return <div>${p}</div>;\n}\n`).join(''),

// BANKFORM1: dynamic routes built in a callback whose `return []` comes first.
'src/permissions/routes.jsx': `import { Home, Login, NotFound, Orders, OrderDetail } from '../pages';

export const generateRoutes = (permissions) => {
const dynamicRoutes = [];
permissions.forEach((currentPerm) => {
if (!currentPerm.module) return [];
dynamicRoutes.push({ path: \`/\${currentPerm.module}\`, element: <Orders /> });
});
return [
{ path: '/login', element: <Login /> },
{
path: '/',
children: [{ path: '/', element: <Home /> }, { path: '/orders/:id', element: <OrderDetail /> }, ...dynamicRoutes],
},
{ path: '*', element: <NotFound /> },
];
};
`,
'src/permissions/App.jsx': `import { createBrowserRouter, RouterProvider } from 'react-router-dom';
import { generateRoutes } from './routes';

export default function App({ permissions }) {
const dynamicRoutes = generateRoutes(permissions);
const router = createBrowserRouter(dynamicRoutes);
return <RouterProvider router={router} />;
}
`,

// An early guard, and two branches that each return a table.
'src/guarded/routes.jsx': `import { Account, Admin, Dashboard, Landing } from '../pages';

export function getRoutes(user) {
if (!user) return [];
if (user.isAdmin) {
return [{ path: '/admin', element: <Admin /> }];
}
return [
{ path: '/dashboard', element: <Dashboard /> },
{ path: '/account', element: <Account /> },
];
}

export const publicRoutes = (ready) => (ready ? [] : [{ path: '/welcome', element: <Landing /> }]);

// Parameters with calls and arrows of their own in them.
export const meRoutes = (user = currentUser(), onLeave = () => null) => [{ path: '/me', element: <Account /> }];

function currentUser() {
return null;
}
`,
'src/guarded/App.jsx': `import { useRoutes } from 'react-router-dom';
import { getRoutes } from './routes';

export default function App({ user }) {
return useRoutes(getRoutes(user));
}
`,

'src/guarded/Me.jsx': `import { useRoutes } from 'react-router-dom';
import { meRoutes } from './routes';

export default function Me() {
return useRoutes(meRoutes());
}
`,

// A table behind useMemo, written both ways.
'src/memo/routes.jsx': `import { Reports, Settings } from '../pages';

export const getRoutes = (user) => [{ path: '/reports', element: <Reports /> }];

export default function settingsRoutes() {
return [{ path: '/settings', element: <Settings /> }];
}
`,
'src/memo/App.jsx': `import { useMemo } from 'react';
import { useRoutes } from 'react-router-dom';
import { getRoutes } from './routes';

export default function App({ user }) {
const routes = useMemo(() => getRoutes(user), [user]);
return useRoutes(routes);
}
`,
'src/memo/Settings.jsx': `import { useMemo } from 'react';
import { useRoutes } from 'react-router-dom';
import settingsRoutes from './routes';

export default function SettingsApp() {
const routes = useMemo(settingsRoutes, []);
return useRoutes(routes);
}
`,

// A function that returns another's table; functions written inside a
// builder return their own lists, which are no routes.
'src/nested/build.jsx': `import { Faq, Help, Home } from '../pages';

export function buildRoutes() {
const note = <p>Don't panic</p>;
const menu = () => {
return [{ path: '/help', element: <Help /> }];
};
function faqRoutes() {
return [{ path: '/faq', element: <Faq /> }];
}
return [{ path: '/', element: <Home />, handle: { note, links: menu() }, loader() { return [faqRoutes()]; } }];
}
`,
'src/nested/routes.jsx': `import { buildRoutes } from './build';

export function appRoutes() {
return buildRoutes();
}
`,
// Read straight from the builder too: before, the menu inside was the table.
'src/nested/Direct.jsx': `import { useRoutes } from 'react-router-dom';
import { buildRoutes } from './build';

export default function Direct() {
return useRoutes(buildRoutes());
}
`,
'src/nested/App.jsx': `import { useRoutes } from 'react-router-dom';
import { appRoutes } from './routes';

export default function App() {
return useRoutes(appRoutes());
}
`,
};

/** `path -> Component` for every route whose node sits under `dir`. */
function routeBindings(cg: CodeGraph, dir: string): string[] {
return cg.getNodesByKind('route').filter((r) => r.filePath.startsWith(dir)).flatMap((r) => cg.getOutgoingEdges(r.id)
.filter((e) => e.kind === 'references')
.map((e) => `${r.name} ${(e.metadata as Record<string, unknown> | undefined)?.layout ? '~>' : '->'} ${cg.getNode(e.target)?.name}`))
.sort();
}

describe('react-router: the tables a route-building function returns', () => {
let root: string;
let cg: CodeGraph;
beforeAll(async () => {
root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-rr-returns-'));
for (const [rel, content] of Object.entries(FILES)) {
fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true });
fs.writeFileSync(path.join(root, rel), content);
}
cg = await CodeGraph.init(root, { index: true });
}, 60_000);
afterAll(() => {
cg?.close();
if (root) fs.rmSync(root, { recursive: true, force: true });
});

it("reads the function's own table, past the `return []` of a callback inside it", () => {
expect(routeBindings(cg, 'src/permissions/')).toEqual([
'/ -> Home',
'/login -> Login',
'/orders/:id -> OrderDetail',
'/* -> NotFound',
].sort());
});

it('reads every table the function returns, past an early `return []`', () => {
expect(routeBindings(cg, 'src/guarded/')).toEqual([
'/account -> Account',
'/admin -> Admin',
'/dashboard -> Dashboard',
'/me -> Account',
]);
});

it('reads the table behind useMemo, a callback or a function handed over', () => {
expect(routeBindings(cg, 'src/memo/')).toEqual(['/reports -> Reports', '/settings -> Settings']);
});

it('follows a function that returns another function\'s table, and reads no list a function inside it returns', () => {
expect(routeBindings(cg, 'src/nested/')).toEqual(['/ -> Home']);
const all = cg.getNodesByKind('route').map((r) => r.name);
for (const p of ['/help', '/faq', '/welcome']) expect(all).not.toContain(p);
});
});
Loading