Skip to content

Commit

Permalink
chore: use map for conflicts, make sure things are in order recieved,…
Browse files Browse the repository at this point in the history
… update test infra

Signed-off-by: Logan McAnsh <logan@mcan.sh>
  • Loading branch information
mcansh committed Feb 17, 2023
1 parent f1e43fd commit 92f9669
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 36 deletions.
48 changes: 19 additions & 29 deletions packages/remix-dev/__tests__/flat-routes-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -627,8 +627,8 @@ describe("flatRoutes", () => {
test("index files", () => {
// we'll add file manually before running the tests
let testFiles = [
"routes/_landing._index.tsx",
"routes/_dashboard._index.tsx",
"routes/_landing._index.tsx",
"routes/_index.tsx",
];

Expand All @@ -644,20 +644,13 @@ describe("flatRoutes", () => {
// we had a collision as /route and /index are the same
expect(routes).toHaveLength(1);
expect(consoleError).toHaveBeenCalledWith(
trimAllLines(`⚠️ Route Path Collision: "/"
The following routes all define the same URL, only the first one will be used
🟢 routes${path.sep}_landing._index.tsx
⭕️️ routes${path.sep}_dashboard._index.tsx
⭕️️ routes${path.sep}_index.tsx
`)
getErrorMessage("/", testFiles)
);
});

test("folder/route.tsx matching folder.tsx", () => {
// we'll add file manually before running the tests
let testFiles = ["routes/dashboard/route.tsx", "routes/dashboard.tsx"];
let testFiles = ["routes/dashboard.tsx", "routes/dashboard/route.tsx"];

let routeManifest = flatRoutesUniversal(
APP_DIR,
Expand All @@ -671,13 +664,7 @@ describe("flatRoutes", () => {
// we had a collision as /route and /index are the same
expect(routes).toHaveLength(1);
expect(consoleError).toHaveBeenCalledWith(
trimAllLines(`⚠️ Route Path Collision: "/dashboard"
The following routes all define the same URL, only the first one will be used
🟢 routes${path.sep}dashboard${path.sep}route.tsx
⭕️️ routes${path.sep}dashboard.tsx
`)
getErrorMessage("/dashboard", testFiles)
);
});

Expand All @@ -700,21 +687,24 @@ describe("flatRoutes", () => {
// we had a collision as /route and /index are the same
expect(routes).toHaveLength(1);
expect(consoleError).toHaveBeenCalledWith(
trimAllLines(`⚠️ Route Path Collision: "/products/:productId"
The following routes all define the same URL, only the first one will be used
🟢 routes${path.sep}products.$productId.tsx
⭕️️ routes${path.sep}products.$pid.tsx
`)
getErrorMessage("/products/:pid", testFiles)
);
});
});
});

function trimAllLines(str: string) {
return str
.split("\n")
.map((line) => line.trim())
.join("\n");
function normalizePath(filePath: string) {
return filePath.split("/").join(path.sep);
}

function getErrorMessage(pathname: string, routes: string[]) {
let [taken, ...others] = routes;

return (
`⚠️ Route Path Collision: "${pathname}"\n\n` +
`The following routes all define the same URL, only the first one will be used\n\n` +
`🟢 ${normalizePath(taken)}\n` +
others.map((route) => `⭕️️ ${normalizePath(route)}`).join("\n") +
"\n"
);
}
25 changes: 18 additions & 7 deletions packages/remix-dev/config/flat-routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ function getRouteMap(
let routeMap = new Map<string, RouteInfo>();
let nameMap = new Map<string, RouteInfo>();
let uniqueRoutes = new Map<string, RouteInfo>();
let conflicts: { [path: string]: RouteInfo[] } = {};
let conflicts = new Map<string, RouteInfo[]>();

for (let routePath of routePaths) {
let routesDirectory = path.join(appDirectory, prefix);
Expand All @@ -338,8 +338,14 @@ function getRouteMap(
let conflict = uniqueRoutes.get(uniqueRouteId);
// collect conflicts for later reporting
if (conflict) {
conflicts[routeInfo.path || "/"] ||= [conflict];
conflicts[routeInfo.path || "/"].push(routeInfo);
let currentConflicts = conflicts.get(routeInfo.path || "/");
if (!currentConflicts) {
conflicts.set(routeInfo.path || "/", [conflict, routeInfo]);
} else {
currentConflicts.push(routeInfo);
conflicts.set(routeInfo.path || "/", currentConflicts);
}

continue;
}
uniqueRoutes.set(uniqueRouteId, routeInfo);
Expand Down Expand Up @@ -377,8 +383,13 @@ function getRouteMap(
segment.startsWith(":") &&
nextSegment.startsWith(":")
) {
conflicts[nextRoute.path || "/"] ||= [nextRoute];
conflicts[nextRoute.path || "/"].push(routeInfo);
let currentConflicts = conflicts.get(routeInfo.path || "/");
if (!currentConflicts) {
conflicts.set(routeInfo.path || "/", [routeInfo, nextRoute]);
} else {
currentConflicts.push(nextRoute);
conflicts.set(routeInfo.path || "/", currentConflicts);
}
return false;
}
}
Expand All @@ -387,8 +398,8 @@ function getRouteMap(
});

// report conflicts
if (Object.keys(conflicts).length > 0) {
for (let [path, routes] of Object.entries(conflicts)) {
if (conflicts.size > 0) {
for (let [path, routes] of conflicts.entries()) {
let [taken, ...rest] = routes;

console.error(
Expand Down

0 comments on commit 92f9669

Please sign in to comment.