Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(file-router): allow routes with the same path (#2971) (CP: 24.6) #2979

Merged
merged 1 commit into from
Dec 10, 2024
Merged

fix(file-router): allow routes with the same path (#2971) (CP: 24.6) #2979

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
132 changes: 101 additions & 31 deletions packages/ts/file-router/src/runtime/RouterConfigurationBuilder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,14 @@ function isReactRouteModule(module?: Module): module is RouteModule<ComponentTyp
export type RouteList = readonly RouteObject[];
export type WritableRouteList = RouteObject[];

export type RouteTransformer<T> = (
original: RouteObject | undefined,
overriding: T | undefined,
children?: RouteList,
) => RouteObject | undefined;
export type RouteTransformerOptions<T> = Readonly<{
children?: RouteList;
original?: RouteObject;
overriding?: T;
dupe: boolean;
}>;

export type RouteListSplittingRule = (route: RouteObject) => boolean;
export type RouteTransformer<T> = (opts: RouteTransformerOptions<T>) => RouteObject | undefined;

type RoutesModifier = (routes: RouteList | undefined) => RouteList | undefined;

Expand Down Expand Up @@ -90,7 +91,7 @@ export class RouterConfigurationBuilder {
* @param routes - A list of routes to add to the current list.
*/
withFileRoutes(routes: readonly AgnosticRoute[]): this {
return this.update(routes, (original, added, children) => {
return this.update(routes, ({ original, overriding: added, children }) => {
if (added) {
const { module, path, flowLayout } = added;
if (!isReactRouteModule(module)) {
Expand Down Expand Up @@ -146,8 +147,8 @@ export class RouterConfigurationBuilder {
{ index: true, element: createElement(component), handle: config },
];

this.update(fallbackRoutes, (original, added, children) => {
if (original && !getRouteHandleFlag(original, RouteHandleFlags.IGNORE_FALLBACK)) {
this.update(fallbackRoutes, ({ original, overriding: added, children, dupe }) => {
if (original && !getRouteHandleFlag(original, RouteHandleFlags.IGNORE_FALLBACK) && !dupe) {
if (!children) {
return original;
}
Expand Down Expand Up @@ -282,7 +283,7 @@ export class RouterConfigurationBuilder {
* and the user is not authenticated.
*/
protect(redirectPath?: string): this {
this.update(undefined, (route, _, children) => {
this.update(undefined, ({ original: route, children }) => {
const finalRoute = protectRoute(route!, redirectPath);
finalRoute.children = children as RouteObject[] | undefined;
return finalRoute;
Expand All @@ -291,54 +292,123 @@ export class RouterConfigurationBuilder {
return this;
}

/**
* Deeply updates the current list of routes with the given routes merging
* them in process.
*
* @param routes - A list of routes to merge with the current list.
* @param callback - A callback to transform the routes during the merge.
*/
update<T extends RouteBase>(routes: undefined, callback: RouteTransformer<undefined>): this;
update<T extends RouteBase>(routes: readonly T[], callback?: RouteTransformer<T>): this;
update<T extends RouteBase>(
routes: readonly T[] | undefined,
callback: RouteTransformer<T | undefined> = (original, overriding, children) =>
callback: RouteTransformer<T | undefined> = ({ original, overriding, children }) =>
({
...original,
...overriding,
children,
}) as RouteObject,
): this {
this.#modifiers.push((existingRoutes) =>
transformTree<[RouteList | undefined, readonly T[] | undefined], RouteList | undefined>(
// Going through the existing and added list of routes.
transformTree<readonly [RouteList | undefined, readonly T[] | undefined], RouteList | undefined>(
[existingRoutes, routes],
null,
([original, added], next) => {
if (original && added) {
const originalMap = new Map(original.map((route) => createRouteEntry(route)));
const addedMap = new Map(added.map((route) => createRouteEntry(route)));

const paths = new Set([...originalMap.keys(), ...addedMap.keys()]);
// If we have both original and added routes, we have to merge them.
const final: Array<RouteObject | undefined> = [];
const paths = new Set([...original.map(({ path }) => path), ...added.map(({ path }) => path)]);

for (const path of paths) {
const originalRoute = originalMap.get(path);
const addedRoute = addedMap.get(path);

let route: RouteObject | undefined;
if (originalRoute && addedRoute) {
route = callback(originalRoute, addedRoute, next([originalRoute.children, addedRoute.children]));
} else if (originalRoute) {
route = callback(originalRoute, undefined, next([originalRoute.children, undefined]));
} else {
route = callback(undefined, addedRoute, next([undefined, addedRoute!.children]));
// We can have multiple routes with the same path, so we have to
// consider all of them.
const originalRoutes = original.filter((r) => r.path === path);
// We can have only one route with the same path in the added list.
const addedRoutes = added.filter((r) => r.path === path);

if (addedRoutes.length > 1) {
throw new Error('Adding multiple routes with the same path is not allowed');
}

if (route) {
originalMap.set(path, route);
const addedRoute = addedRoutes[0] as T | undefined;

if (originalRoutes.length > 0 && addedRoute) {
// In case we have both original and added routes, we run
// the callback for each original route in pair with the added
// route. To make the difference, we flag all the routes except
// the last one as `dupe`.
//
// Why the last one is not `dupe`? According to the
// `react-router` logic, the last route is the fallback for all
// routes with the same path. So, if we apply callback to it,
// we implicitly apply it to all other routes with the same
// path.
//
// In case this logic doesn't work, the user can apply the
// callback without considering the `dupe` flag.
for (let i = 0; i < originalRoutes.length; i++) {
final.push(
callback({
original: originalRoutes[i],
overriding: addedRoute,
children: next([originalRoutes[i].children, addedRoute.children]),
dupe: i < originalRoutes.length - 1,
}) ?? originalRoutes[i],
);
}
} else if (originalRoutes.length > 0) {
// In case we don't have the added route with the path being
// processed, we run the callback for each original route.
for (let i = 0; i < originalRoutes.length; i++) {
final.push(
callback({
original: originalRoutes[i],
children: next([originalRoutes[i].children, undefined]),
dupe: i < originalRoutes.length - 1,
}) ?? originalRoutes[i],
);
}
} else {
// In case we don't have the original route with the path being
// processed, we run the callback for only the added route.
const result = callback({
overriding: addedRoute,
children: next([undefined, addedRoute!.children]),
dupe: false,
});

if (result) {
final.push(result);
}
}
}

return [...originalMap.values()];
return final.filter((r) => r != null);
} else if (original) {
// If we have only original routes, we run the callback for each
// original route.
return original
.map((route) => callback(route, undefined, next([route.children, undefined])))
.map((route) =>
callback({
original: route,
children: next([route.children, undefined]),
dupe: false,
}),
)
.filter((r) => r != null);
} else if (added) {
// If we have only added routes, we run the callback for each added
// route.
return added
.map((route) => callback(undefined, route, next([undefined, route.children])))
.map((route) =>
callback({
overriding: route,
children: next([undefined, route.children]),
dupe: false,
}),
)
.filter((r) => r != null);
}

Expand Down
86 changes: 83 additions & 3 deletions packages/ts/file-router/test/runtime/RouterConfigurationBuilder.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,17 @@ describe('RouterBuilder', () => {
},
]);
reset = mockDocumentBaseURI('https://example.com/foo');
globalThis.window = {
// @ts-expect-error Fake just enough so tests pass
history: {
replaceState: () => {},
},
// @ts-expect-error Fake just enough so tests pass
location: '',
addEventListener: () => {},
};
// @ts-expect-error Fake just enough so tests pass
globalThis.document.defaultView = globalThis.window;
});

afterEach(() => {
Expand Down Expand Up @@ -168,9 +179,6 @@ describe('RouterBuilder', () => {
{
path: '/test',
element: <div>Test</div>,
},
{
path: '/test',
children: [
{
path: '/child-test',
Expand Down Expand Up @@ -830,4 +838,76 @@ describe('RouterBuilder', () => {
reset();
});
});
describe('issues', () => {
it('#2954', () => {
const { routes } = new RouterConfigurationBuilder()
.withFileRoutes([
{
path: '',
children: [
{
path: '',
module: {
config: {
menu: { order: 0 },
title: 'Public view',
},
default: NextTest,
},
},
{
path: 'login',
module: {
config: {
menu: { exclude: true },
flowLayout: false,
},
},
},
],
},
])
.withFallback(Server)
.protect()
.build();

expect(routes).to.be.like([
{
path: '',
children: [
{
path: 'login',
handle: {
menu: {
exclude: true,
},
flowLayout: false,
title: 'undefined',
},
},
],
handle: {
title: 'undefined',
},
},
{
path: '',
children: [
{
element: <NextTest />,
handle: {
menu: { order: 0 },
title: 'Public view',
},
index: true,
},
{ path: '*', element: <Server /> },
],
handle: { title: 'undefined' },
},
{ path: '*', element: <Server /> },
{ index: true, element: <Server /> },
]);
});
});
});
Loading