Unverified 提交 1c8ccdb4 authored 作者: keppo-bot[bot]'s avatar keppo-bot[bot] 提交者: GitHub

Fix preview route discovery states (#3158)

## Summary - stop the preview route dropdown from showing "Loading routes..." after route discovery has already finished - expand React Router discovery to scan common route files like `src/routes/*`, `*routes.tsx`, and `router.tsx` instead of only `src/App.tsx` - add parser tests covering modular route files and merged route discovery ## Test plan - `npm run fmt && npm run lint:fix && npm run ts` - `npm test` 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- devin-review-badge-begin --> --- <a href="https://app.devin.ai/review/dyad-sh/dyad/pull/3158" target="_blank"> <picture> <source media="(prefers-color-scheme: dark)" srcset="https://static.devin.ai/assets/gh-open-in-devin-review-dark.svg?v=1"> <img src="https://static.devin.ai/assets/gh-open-in-devin-review-light.svg?v=1" alt="Open with Devin"> </picture> </a> <!-- devin-review-badge-end --> --------- Co-authored-by: 's avatarWill Chen <7344640+wwwillchen@users.noreply.github.com>
上级 139b3ffe
...@@ -24,6 +24,10 @@ gh pr create --head <owner>:<branch> ... ...@@ -24,6 +24,10 @@ gh pr create --head <owner>:<branch> ...
This can happen when remotes are configured in a non-fork layout and `gh` fails to infer the branch mapping. This can happen when remotes are configured in a non-fork layout and `gh` fails to infer the branch mapping.
## GH auth allowlist and git push
If `gh auth status` succeeds but `git push` fails with `Repo <owner>/<repo> is not allowlisted` followed by `fatal: could not read Username for 'https://github.com/...': Device not configured`, run `gh auth setup-git` first and then push to an allowlisted remote. In some bot workspaces, fork remotes are not allowlisted even when `upstream` is, so retry the push against `upstream` if project policy permits it.
## Empty branches cannot produce PRs ## Empty branches cannot produce PRs
Before creating a PR for a freshly pushed branch, check whether it is actually ahead of the base branch: Before creating a PR for a freshly pushed branch, check whether it is actually ahead of the base branch:
......
import { describe, it, expect } from "vitest"; import { describe, it, expect } from "vitest";
import { import {
buildRouteLabel, buildRouteLabel,
getReactRouterCandidateFiles,
parseRoutesFromRouterFile, parseRoutesFromRouterFile,
parseRoutesFromRouterFiles,
parseRoutesFromNextFiles, parseRoutesFromNextFiles,
} from "@/hooks/useParseRouter"; } from "@/hooks/useParseRouter";
...@@ -131,6 +133,66 @@ describe("parseRoutesFromRouterFile", () => { ...@@ -131,6 +133,66 @@ describe("parseRoutesFromRouterFile", () => {
}); });
}); });
describe("getReactRouterCandidateFiles", () => {
it("prioritizes src/App.tsx and includes modular route files", () => {
const files = [
"src/main.tsx",
"src/App.tsx",
"src/routes/publicRoutes.tsx",
"src/routes/protectedRoutes.tsx",
"src/features/orders/orderRoutes.tsx",
"src/router.tsx",
"src/pages/Home.tsx",
];
expect(getReactRouterCandidateFiles(files)).toEqual([
"src/App.tsx",
"src/routes/publicRoutes.tsx",
"src/routes/protectedRoutes.tsx",
"src/features/orders/orderRoutes.tsx",
"src/router.tsx",
]);
});
it("falls back to root App.tsx when src/App.tsx is absent", () => {
const files = ["App.tsx", "routes/publicRoutes.tsx"];
expect(getReactRouterCandidateFiles(files)).toEqual([
"App.tsx",
"routes/publicRoutes.tsx",
]);
});
});
describe("parseRoutesFromRouterFiles", () => {
it("merges routes across multiple files without duplicates", () => {
const routes = parseRoutesFromRouterFiles([
`
<Routes>
<Route path="/" element={<Home />} />
<Route path="/about" element={<About />} />
</Routes>
`,
`
export function protectedRoutes() {
return (
<>
<Route path="/dashboard" element={<Dashboard />} />
<Route path="/about" element={<AboutAgain />} />
</>
);
}
`,
]);
expect(routes.map((route) => route.path)).toEqual([
"/",
"/about",
"/dashboard",
]);
});
});
describe("parseRoutesFromNextFiles", () => { describe("parseRoutesFromNextFiles", () => {
describe("pages router", () => { describe("pages router", () => {
it("should parse routes from pages directory", () => { it("should parse routes from pages directory", () => {
......
...@@ -183,7 +183,11 @@ export const PreviewIframe = ({ loading }: { loading: boolean }) => { ...@@ -183,7 +183,11 @@ export const PreviewIframe = ({ loading }: { loading: boolean }) => {
const [errorMessage, setErrorMessage] = useAtom(previewErrorMessageAtom); const [errorMessage, setErrorMessage] = useAtom(previewErrorMessageAtom);
const selectedChatId = useAtomValue(selectedChatIdAtom); const selectedChatId = useAtomValue(selectedChatIdAtom);
const { streamMessage } = useStreamChat(); const { streamMessage } = useStreamChat();
const { routes: availableRoutes } = useParseRouter(selectedAppId); const {
routes: availableRoutes,
loading: routesLoading,
error: routesError,
} = useParseRouter(selectedAppId);
const { restartApp } = useRunApp(); const { restartApp } = useRunApp();
const { settings, updateSettings } = useSettings(); const { settings, updateSettings } = useSettings();
const { userBudget } = useUserBudgetInfo(); const { userBudget } = useUserBudgetInfo();
...@@ -1243,7 +1247,15 @@ export const PreviewIframe = ({ loading }: { loading: boolean }) => { ...@@ -1243,7 +1247,15 @@ export const PreviewIframe = ({ loading }: { loading: boolean }) => {
<ChevronDown size={14} className="flex-shrink-0" /> <ChevronDown size={14} className="flex-shrink-0" />
</DropdownMenuTrigger> </DropdownMenuTrigger>
<DropdownMenuContent className="w-full"> <DropdownMenuContent className="w-full">
{availableRoutes.length > 0 ? ( {routesLoading ? (
<DropdownMenuItem disabled>
Loading routes...
</DropdownMenuItem>
) : routesError ? (
<DropdownMenuItem disabled>
Unable to load routes
</DropdownMenuItem>
) : availableRoutes.length > 0 ? (
availableRoutes.map((route) => ( availableRoutes.map((route) => (
<DropdownMenuItem <DropdownMenuItem
key={route.path} key={route.path}
...@@ -1258,7 +1270,7 @@ export const PreviewIframe = ({ loading }: { loading: boolean }) => { ...@@ -1258,7 +1270,7 @@ export const PreviewIframe = ({ loading }: { loading: boolean }) => {
)) ))
) : ( ) : (
<DropdownMenuItem disabled> <DropdownMenuItem disabled>
Loading routes... No routes detected
</DropdownMenuItem> </DropdownMenuItem>
)} )}
</DropdownMenuContent> </DropdownMenuContent>
......
import { useEffect, useMemo, useState } from "react"; import { useMemo } from "react";
import { useLoadAppFile } from "@/hooks/useLoadAppFile"; import { useQueries } from "@tanstack/react-query";
import { useLoadApp } from "@/hooks/useLoadApp"; import { useLoadApp } from "@/hooks/useLoadApp";
import { ipc } from "@/ipc/types";
import { queryKeys } from "@/lib/queryKeys";
export interface ParsedRoute { export interface ParsedRoute {
path: string; path: string;
label: string; label: string;
} }
const ROUTE_FILE_EXTENSIONS = new Set(["js", "jsx", "ts", "tsx"]);
function hasRouteFileExtension(filePath: string): boolean {
const extension = filePath.split(".").pop()?.toLowerCase();
return extension !== undefined && ROUTE_FILE_EXTENSIONS.has(extension);
}
function isAppEntryFile(filePath: string): boolean {
return /(?:^|\/)App\.(?:js|jsx|ts|tsx)$/i.test(filePath);
}
function isRouteModuleFile(filePath: string): boolean {
if (!hasRouteFileExtension(filePath)) {
return false;
}
return (
/(?:^|\/)routes\/.+\.(?:js|jsx|ts|tsx)$/i.test(filePath) ||
/(?:^|\/)[^/]*routes?\.(?:js|jsx|ts|tsx)$/i.test(filePath) ||
/(?:^|\/)router\.(?:js|jsx|ts|tsx)$/i.test(filePath)
);
}
export function getReactRouterCandidateFiles(files: string[]): string[] {
const candidates = new Set<string>();
for (const file of files) {
if (!hasRouteFileExtension(file)) {
continue;
}
if (isAppEntryFile(file) || isRouteModuleFile(file)) {
candidates.add(file);
}
}
if (files.includes("src/App.tsx")) {
candidates.delete("src/App.tsx");
return ["src/App.tsx", ...Array.from(candidates)];
}
if (files.includes("App.tsx")) {
candidates.delete("App.tsx");
return ["App.tsx", ...Array.from(candidates)];
}
return Array.from(candidates);
}
/** /**
* Builds a human-readable label from a route path. * Builds a human-readable label from a route path.
*/ */
...@@ -54,6 +105,24 @@ export function parseRoutesFromRouterFile( ...@@ -54,6 +105,24 @@ export function parseRoutesFromRouterFile(
} }
} }
export function parseRoutesFromRouterFiles(
contents: Array<string | null | undefined>,
): ParsedRoute[] {
const parsedRoutes: ParsedRoute[] = [];
for (const content of contents) {
for (const route of parseRoutesFromRouterFile(content ?? null)) {
if (
!parsedRoutes.some((existingRoute) => existingRoute.path === route.path)
) {
parsedRoutes.push(route);
}
}
}
return parsedRoutes;
}
/** /**
* Parses routes from Next.js file-based routing (pages/ or app/ directories). * Parses routes from Next.js file-based routing (pages/ or app/ directories).
*/ */
...@@ -132,8 +201,6 @@ export function parseRoutesFromNextFiles(files: string[]): ParsedRoute[] { ...@@ -132,8 +201,6 @@ export function parseRoutesFromNextFiles(files: string[]): ParsedRoute[] {
* Loads the app router file and parses available routes for quick navigation. * Loads the app router file and parses available routes for quick navigation.
*/ */
export function useParseRouter(appId: number | null) { export function useParseRouter(appId: number | null) {
const [routes, setRoutes] = useState<ParsedRoute[]>([]);
// Load app to access the file list // Load app to access the file list
const { const {
app, app,
...@@ -142,33 +209,53 @@ export function useParseRouter(appId: number | null) { ...@@ -142,33 +209,53 @@ export function useParseRouter(appId: number | null) {
refreshApp, refreshApp,
} = useLoadApp(appId); } = useLoadApp(appId);
// Load router related file to extract routes for non-Next apps
const {
content: routerContent,
loading: routerFileLoading,
error: routerFileError,
refreshFile,
} = useLoadAppFile(appId, "src/App.tsx");
// Detect Next.js app by presence of next.config.* in file list // Detect Next.js app by presence of next.config.* in file list
const isNextApp = useMemo(() => { const isNextApp = useMemo(() => {
if (!app?.files) return false; if (!app?.files) return false;
return app.files.some((f) => f.toLowerCase().includes("next.config")); return app.files.some((f) => f.toLowerCase().includes("next.config"));
}, [app?.files]); }, [app?.files]);
// Parse routes either from Next.js file-based routing or from router file const candidateRouterFiles = useMemo(() => {
useEffect(() => { if (!app?.files || isNextApp) {
if (isNextApp && app?.files) { return [];
setRoutes(parseRoutesFromNextFiles(app.files));
} else {
setRoutes(parseRoutesFromRouterFile(routerContent ?? null));
} }
}, [isNextApp, app?.files, routerContent]);
return getReactRouterCandidateFiles(app.files);
}, [app?.files, isNextApp]);
const routerFileQueries = useQueries({
queries: candidateRouterFiles.map((filePath) => ({
queryKey: queryKeys.appFiles.content({ appId, filePath }),
queryFn: async () => {
return ipc.app.readAppFile({ appId: appId!, filePath });
},
enabled: appId !== null,
})),
});
const routes =
isNextApp && app?.files
? parseRoutesFromNextFiles(app.files)
: parseRoutesFromRouterFiles(
routerFileQueries.map((query) => query.data ?? null),
);
const routerFileLoading =
!isNextApp &&
candidateRouterFiles.length > 0 &&
routerFileQueries.some((query) => query.isLoading);
const routerFileError = !isNextApp
? (routerFileQueries.find((query) => query.error)?.error ?? null)
: null;
const combinedLoading = appLoading || routerFileLoading; const combinedLoading = appLoading || routerFileLoading;
const combinedError = appError || routerFileError || null; const combinedError = appError || routerFileError || null;
const refresh = async () => { const refresh = async () => {
await Promise.allSettled([refreshApp(), refreshFile()]); await Promise.allSettled([
refreshApp(),
...routerFileQueries.map(async (query) => {
await query.refetch();
}),
]);
}; };
return { return {
......
Markdown 格式
0%
您添加了 0 到此讨论。请谨慎行事。
请先完成此评论的编辑!
注册 或者 后发表评论