Skip to content

Commit

Permalink
fix(remix-dev): allow defining multiple routes for the same route mod…
Browse files Browse the repository at this point in the history
…ule file (#3970)

* Allow multiple routes for same route module

* Update packages/remix-dev/config/routes.ts

Co-authored-by: Andrew Leedham <AndrewLeedham@outlook.com>

* Update routes.ts

- Better name for automated ID variable;
- Small adjust in `id` option comment;

* - Removing redundant IF

* Update routes.ts

Revert complex custom ID in routes

* Non unique custom routes ID error and test

* Update assets.ts

Trying to solve a conflict

* Revert "Update assets.ts"

This reverts commit 2064c57.

* Error on collisions with non-custom routeIds

* Create big-spoons-grab.md

Co-authored-by: Andrew Leedham <AndrewLeedham@outlook.com>
Co-authored-by: Matt Brophy <matt@brophy.org>
  • Loading branch information
3 people authored Dec 1, 2022
1 parent 95b4b28 commit da0c278
Show file tree
Hide file tree
Showing 5 changed files with 121 additions and 19 deletions.
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}"`
);
}

routes[route.id] = route;

if (children) {
Expand Down

0 comments on commit da0c278

Please sign in to comment.