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.

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: Repair catch-all route params couldn't be mixed with other params #25

Merged
merged 4 commits into from
Aug 17, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
7 changes: 4 additions & 3 deletions src/create-navigation-config.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,11 +149,12 @@ describe('createNavigationConfig', () => {
describe('when path has multiple route params', () => {
const { routes } = createNavigationConfig((defineRoute) => ({
organizationUser: defineRoute(
'/organizations/[orgId]/users/[userId]',
'/organizations/[orgId]/users/[userId]/[[...catch_all]]',
{
params: z.object({
orgId: z.string(),
userId: z.string(),
catch_all: z.array(z.string()).default([])
}),
},
),
Expand All @@ -168,8 +169,8 @@ describe('createNavigationConfig', () => {
});

expect(
routes.organizationUser({ orgId: 'org_123', userId: 'user_123' }),
).toBe('/organizations/org_123/users/user_123');
routes.organizationUser({ orgId: 'org_123', userId: 'user_123', catch_all: ['channel_123'] }),
).toBe('/organizations/org_123/users/user_123/channel_123');
});

it('exposes method to validate only params', () => {
Expand Down
52 changes: 52 additions & 0 deletions src/make-route-builder.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,58 @@ describe('makeRouteBuilder', () => {
});
});
});

describe('when path has normal and catch-all params', () => {
it('creates a builder that replaces the path params with their value', () => {
const builder = makeRouteBuilder(
'/organization/[orgId]/c/[...catch_all]',
{
params: z.object({
orgId: z.string(),
catch_all: z.array(z.string()),
}),
},
);

// @ts-expect-error no searchParams validation was defined
builder({ catch_all: ['channels'], search: {} });

expect(
builder({ catch_all: ['channels', 'channel_123'], orgId: 'org_123' }),
).toBe('/organization/org_123/c/channels/channel_123');

expect(builder.getSchemas()).toEqual({
params: expect.any(Object),
search: undefined,
});
});
});
});

describe('when path has normal and optional catch-all params', () => {
it('creates a builder that replaces the path params with their value', () => {
const builder = makeRouteBuilder(
'/organization/[orgId]/c/[[...catch_all]]',
{
params: z.object({
orgId: z.string(),
catch_all: z.array(z.string()).default([]),
}),
},
);

// @ts-expect-error no searchParams validation was defined
builder({ catch_all: ['channels'], search: {} });

expect(
builder({ catch_all: ['channels', 'channel_123'], orgId: 'org_123' }),
).toBe('/organization/org_123/c/channels/channel_123');

expect(builder.getSchemas()).toEqual({
params: expect.any(Object),
search: undefined,
});
});
});

describe('for a path with route params and searchParams', () => {
Expand Down
4 changes: 2 additions & 2 deletions src/make-route-builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ type Suffix = `?${string}`;
type SafePath<Path extends string> = string extends Route ? Path : Route<Path>;

type ExtractPathParams<T extends string> =
T extends `${string}[[...${infer Param}]]${infer Rest}` ?
T extends `${infer Rest}[[...${infer Param}]]` ?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Catch-all params are supposed to be the last one, so we 'parse' them from right to left.

Copy link
Owner

Choose a reason for hiding this comment

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

Good catch @Katona

Param | ExtractPathParams<Rest>
: T extends `${string}[...${infer Param}]${infer Rest}` ?
: T extends `${infer Rest}[...${infer Param}]` ?
Param | ExtractPathParams<Rest>
: T extends `${string}[${infer Param}]${infer Rest}` ?
Param | ExtractPathParams<Rest>
Expand Down
Loading