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/big-spoons-grab.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@remix-run/dev": minor
---

allow defining multiple routes for the same route module file
1 change: 1 addition & 0 deletions contributors.yml
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@
- lpsinger
- lswest
- lucasdibz
- lucasferreira
- luispagarcia
- luisrivas
- luistak
Expand Down
76 changes: 76 additions & 0 deletions packages/remix-dev/__tests__/defineRoutes-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,4 +87,80 @@ describe("defineRoutes", () => {
}
`);
});

it("allows multiple routes with the same route module", () => {
let routes = defineRoutes((route) => {
route("/user/:id", "routes/index.tsx", { id: "user-by-id" });
route("/user", "routes/index.tsx", { id: "user" });
route("/other", "routes/other-route.tsx");
});

expect(routes).toMatchInlineSnapshot(`
Object {
"routes/other-route": Object {
"caseSensitive": undefined,
"file": "routes/other-route.tsx",
"id": "routes/other-route",
"index": undefined,
"parentId": undefined,
"path": "/other",
},
"user": Object {
"caseSensitive": undefined,
"file": "routes/index.tsx",
"id": "user",
"index": undefined,
"parentId": undefined,
"path": "/user",
},
"user-by-id": Object {
"caseSensitive": undefined,
"file": "routes/index.tsx",
"id": "user-by-id",
"index": undefined,
"parentId": undefined,
"path": "/user/:id",
},
}
`);
});

it("throws an error on route id collisions", () => {
// Two conflicting custom id's
let defineNonUniqueRoutes = () => {
defineRoutes((route) => {
route("/user/:id", "routes/user.tsx", { id: "user" });
route("/user", "routes/user.tsx", { id: "user" });
route("/other", "routes/other-route.tsx");
});
};

expect(defineNonUniqueRoutes).toThrowErrorMatchingInlineSnapshot(
`"Unable to define routes with duplicate route id: \\"user\\""`
);

// Custom id conflicting with a later-defined auto-generated id
defineNonUniqueRoutes = () => {
defineRoutes((route) => {
route("/user/:id", "routes/user.tsx", { id: "routes/user" });
route("/user", "routes/user.tsx");
});
};

expect(defineNonUniqueRoutes).toThrowErrorMatchingInlineSnapshot(
`"Unable to define routes with duplicate route id: \\"routes/user\\""`
);

// Custom id conflicting with an earlier-defined auto-generated id
defineNonUniqueRoutes = () => {
defineRoutes((route) => {
route("/user", "routes/user.tsx");
route("/user/:id", "routes/user.tsx", { id: "routes/user" });
});
};

expect(defineNonUniqueRoutes).toThrowErrorMatchingInlineSnapshot(
`"Unable to define routes with duplicate route id: \\"routes/user\\""`
);
});
});
44 changes: 26 additions & 18 deletions packages/remix-dev/compiler/assets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,13 @@ export async function createAssetsManifest(
config.appDirectory,
config.entryClientFile
);
let routesByFile: Map<string, Route> = Object.keys(config.routes).reduce(
let routesByFile: Map<string, Route[]> = Object.keys(config.routes).reduce(
(map, key) => {
let route = config.routes[key];
map.set(route.file, route);
map.set(
route.file,
map.has(route.file) ? [...map.get(route.file), route] : [route]
);
return map;
},
new Map()
Expand All @@ -83,22 +86,27 @@ export async function createAssetsManifest(
/(^browser-route-module:|\?browser$)/g,
""
);
let route = routesByFile.get(entryPointFile);
invariant(route, `Cannot get route for entry point ${output.entryPoint}`);
let sourceExports = await getRouteModuleExports(config, route.id);
routes[route.id] = {
id: route.id,
parentId: route.parentId,
path: route.path,
index: route.index,
caseSensitive: route.caseSensitive,
module: resolveUrl(key),
imports: resolveImports(output.imports),
hasAction: sourceExports.includes("action"),
hasLoader: sourceExports.includes("loader"),
hasCatchBoundary: sourceExports.includes("CatchBoundary"),
hasErrorBoundary: sourceExports.includes("ErrorBoundary"),
};
let groupedRoute = routesByFile.get(entryPointFile);
invariant(
groupedRoute,
`Cannot get route(s) for entry point ${output.entryPoint}`
);
for (let route of groupedRoute) {
let sourceExports = await getRouteModuleExports(config, route.id);
routes[route.id] = {
id: route.id,
parentId: route.parentId,
path: route.path,
index: route.index,
caseSensitive: route.caseSensitive,
module: resolveUrl(key),
imports: resolveImports(output.imports),
hasAction: sourceExports.includes("action"),
hasLoader: sourceExports.includes("loader"),
hasCatchBoundary: sourceExports.includes("CatchBoundary"),
hasErrorBoundary: sourceExports.includes("ErrorBoundary"),
};
}
}
}

Expand Down
14 changes: 13 additions & 1 deletion packages/remix-dev/config/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,12 @@ export interface DefineRouteOptions {
* Should be `true` if this is an index route that does not allow child routes.
*/
index?: boolean;

/**
* An optional unique id string for this route. Use this if you need to aggregate
* two or more routes with the same route file.
*/
id?: string;
}

interface DefineRouteChildren {
Expand Down Expand Up @@ -141,14 +147,20 @@ export function defineRoutes(
path: path ? path : undefined,
index: options.index ? true : undefined,
caseSensitive: options.caseSensitive ? true : undefined,
id: createRouteId(file),
id: options.id || createRouteId(file),
parentId:
parentRoutes.length > 0
? parentRoutes[parentRoutes.length - 1].id
: undefined,
file,
};

if (route.id in routes) {
throw new Error(
`Unable to define routes with duplicate route id: "${route.id}"`
Copy link
Contributor

Choose a reason for hiding this comment

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

I adjusted this to account for the (very edge-case) scenario that an auto-generated route id collies with a previously-defined custom id and expanded the tests above to assert the new behavior

);
}

routes[route.id] = route;

if (children) {
Expand Down