Skip to content

Commit 1c31dce

Browse files
posvaposva
committed
fix: mixed params, query and hash
1 parent 9be2b7c commit 1c31dce

File tree

5 files changed

+313
-31
lines changed

5 files changed

+313
-31
lines changed

‎packages/router/src/experimental/route-resolver/matchers/matcher-pattern.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,8 @@ export interface MatcherPatternQuery<
254254
export interface MatcherPatternHash<
255255
TParams extends MatcherParamsFormatted = MatcherParamsFormatted,
256256
> extends MatcherPattern<string, TParams> {}
257+
// TODO: is this worth? It doesn't look like it is as it makes typing stricter but annoying
258+
// > extends MatcherPattern<`#${string}` | '', TParams> {}
257259

258260
/**
259261
* Generic object of params that can be passed to a matcher.

‎packages/router/src/experimental/route-resolver/resolver-fixed.spec.ts‎

Lines changed: 242 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
import { describe, expect, it } from 'vitest'
22
import { createFixedResolver } from './resolver-fixed'
33
import { NO_MATCH_LOCATION } from './resolver-abstract'
4-
import { MatcherQueryParams } from './matchers/matcher-pattern'
4+
import {
5+
EmptyParams,
6+
MatcherPatternHash,
7+
MatcherQueryParams,
8+
} from './matchers/matcher-pattern'
59
import {
610
MatcherPatternQuery,
711
MatcherPatternPathStatic,
@@ -13,7 +17,7 @@ import {
1317
ANY_HASH_PATTERN_MATCHER,
1418
PAGE_QUERY_PATTERN_MATCHER,
1519
} from './matchers/test-utils'
16-
import { miss } from './matchers/errors'
20+
import { MatchMiss, miss } from './matchers/errors'
1721
import { MatcherPatternPath } from './matchers/matcher-pattern'
1822

1923
// Additional pattern matchers for testing advanced scenarios
@@ -67,6 +71,35 @@ const REPEATABLE_PARAM_MATCHER: MatcherPatternPath<{ p: string | string[] }> = {
6771
},
6872
}
6973

74+
const OPTIONAL_NUMBER_HASH_MATCHER: MatcherPatternHash<{
75+
hash: number | null
76+
}> = {
77+
match(hash) {
78+
if (!hash || hash === '#') return { hash: null }
79+
const num = Number(hash.slice(1))
80+
if (Number.isNaN(num)) throw miss('Hash must be a number')
81+
return { hash: num }
82+
},
83+
build({ hash }) {
84+
return hash != null ? `#${hash}` : ''
85+
},
86+
}
87+
88+
const OPTIONAL_NUMBER_QUERY_MATCHER: MatcherPatternQuery<{
89+
count: number | null
90+
}> = {
91+
match(query) {
92+
if (!query.count) return { count: null }
93+
const count = Number(query.count)
94+
if (Number.isNaN(count)) throw miss('Count must be a number')
95+
return { count }
96+
},
97+
build({ count }) {
98+
// return { count: count != null ? String(count) : null }
99+
return count != null ? { count: String(count) } : ({} as EmptyParams)
100+
},
101+
}
102+
70103
describe('fixed resolver', () => {
71104
describe('new matchers', () => {
72105
it('static path', () => {
@@ -602,7 +635,7 @@ describe('fixed resolver', () => {
602635
})
603636
})
604637

605-
it('preserves empty string hash from matcher over to.hash', () => {
638+
it('preserves hash from param even if empty', () => {
606639
const resolver = createFixedResolver([
607640
{
608641
name: 'document',
@@ -620,8 +653,8 @@ describe('fixed resolver', () => {
620653
).toMatchObject({
621654
name: 'document',
622655
path: '/',
623-
params: { hash: '' },
624-
hash: '', // empty string from matcher is preserved
656+
params: { hash: null },
657+
hash: '',
625658
fullPath: '/',
626659
})
627660
})
@@ -654,6 +687,210 @@ describe('fixed resolver', () => {
654687
})
655688
})
656689

690+
describe('manual values breaking re-resolution', () => {
691+
it('throws when manual hash cannot be parsed by hash matcher', () => {
692+
const resolver = createFixedResolver([
693+
{
694+
name: 'page',
695+
path: EMPTY_PATH_PATTERN_MATCHER,
696+
hash: OPTIONAL_NUMBER_HASH_MATCHER,
697+
},
698+
])
699+
700+
expect(
701+
resolver.resolve({
702+
name: 'page',
703+
params: {},
704+
hash: '#invalid-text',
705+
})
706+
).toMatchObject({
707+
name: 'page',
708+
params: { hash: null },
709+
fullPath: '/',
710+
hash: '',
711+
})
712+
})
713+
714+
it('throws when manual query cannot be parsed by query matcher', () => {
715+
const resolver = createFixedResolver([
716+
{
717+
name: 'search',
718+
path: EMPTY_PATH_PATTERN_MATCHER,
719+
// this query returns {} if no count is provided as a param
720+
// that {} gets merged with the invalid query and throws
721+
query: [OPTIONAL_NUMBER_QUERY_MATCHER],
722+
},
723+
])
724+
725+
expect(() =>
726+
resolver.resolve({
727+
name: 'search',
728+
params: {},
729+
query: { count: 'invalid', other: 'value' }, // Not a number
730+
})
731+
).toThrow(MatchMiss)
732+
})
733+
734+
it('ignores the hash if a parser is provided', () => {
735+
const resolver = createFixedResolver([
736+
{
737+
name: 'page',
738+
path: EMPTY_PATH_PATTERN_MATCHER,
739+
hash: OPTIONAL_NUMBER_HASH_MATCHER,
740+
},
741+
])
742+
743+
expect(
744+
resolver.resolve({
745+
name: 'page',
746+
params: {},
747+
hash: '#42',
748+
})
749+
).toEqual({
750+
name: 'page',
751+
params: { hash: null },
752+
fullPath: '/',
753+
path: '/',
754+
query: {},
755+
hash: '',
756+
matched: expect.any(Array),
757+
})
758+
})
759+
760+
it('succeeds and parses when manual query is valid for matcher', () => {
761+
const resolver = createFixedResolver([
762+
{
763+
name: 'search',
764+
path: EMPTY_PATH_PATTERN_MATCHER,
765+
query: [OPTIONAL_NUMBER_QUERY_MATCHER],
766+
},
767+
])
768+
769+
expect(
770+
resolver.resolve({
771+
name: 'search',
772+
params: {},
773+
query: { count: '10', other: 'value' }, // Valid number
774+
})
775+
).toEqual({
776+
name: 'search',
777+
path: '/',
778+
params: { count: 10 },
779+
query: { count: '10', other: 'value' },
780+
hash: '',
781+
fullPath: '/?count=10&other=value',
782+
matched: expect.any(Array),
783+
})
784+
})
785+
786+
it('keeps other query values that are not params', () => {
787+
const resolver = createFixedResolver([
788+
{
789+
name: 'page',
790+
path: EMPTY_PATH_PATTERN_MATCHER,
791+
query: [OPTIONAL_NUMBER_QUERY_MATCHER],
792+
hash: OPTIONAL_NUMBER_HASH_MATCHER,
793+
},
794+
])
795+
796+
expect(
797+
resolver.resolve({
798+
name: 'page',
799+
params: { hash: 42 },
800+
query: { count: '10', other: 'value' },
801+
})
802+
).toEqual({
803+
name: 'page',
804+
path: '/',
805+
params: { count: 10, hash: 42 },
806+
query: { count: '10', other: 'value' },
807+
hash: '#42',
808+
fullPath: '/?count=10&other=value#42',
809+
matched: expect.any(Array),
810+
})
811+
})
812+
813+
it('ignores manual hash if defined as param', () => {
814+
const resolver = createFixedResolver([
815+
{
816+
name: 'page',
817+
path: EMPTY_PATH_PATTERN_MATCHER,
818+
hash: OPTIONAL_NUMBER_HASH_MATCHER,
819+
},
820+
])
821+
822+
expect(
823+
resolver.resolve({
824+
name: 'page',
825+
params: { hash: 100 },
826+
hash: '#invalid',
827+
})
828+
).toMatchObject({
829+
name: 'page',
830+
params: { hash: 100 },
831+
hash: '#100',
832+
})
833+
})
834+
835+
it('preserves currentLocation hash fallback when no manual values', () => {
836+
const resolver = createFixedResolver([
837+
{
838+
name: 'page',
839+
path: EMPTY_PATH_PATTERN_MATCHER,
840+
hash: OPTIONAL_NUMBER_HASH_MATCHER,
841+
},
842+
])
843+
844+
const currentLocation = resolver.resolve({
845+
name: 'page',
846+
params: { hash: 50 },
847+
})
848+
849+
// No manual values, should preserve currentLocation
850+
expect(resolver.resolve({}, currentLocation)).toEqual({
851+
name: 'page',
852+
path: '/',
853+
params: { hash: 50 },
854+
hash: '#50',
855+
fullPath: '/#50',
856+
query: {},
857+
matched: expect.any(Array),
858+
})
859+
})
860+
861+
it('preserves currentLocation query fallback when no manual values', () => {
862+
const resolver = createFixedResolver([
863+
{
864+
name: 'search',
865+
path: EMPTY_PATH_PATTERN_MATCHER,
866+
query: [OPTIONAL_NUMBER_QUERY_MATCHER],
867+
},
868+
])
869+
870+
const currentLocation = resolver.resolve({
871+
name: 'search',
872+
params: { count: 20 },
873+
})
874+
875+
expect(
876+
resolver.resolve(
877+
{
878+
query: {
879+
other: 'value',
880+
},
881+
},
882+
currentLocation
883+
)
884+
).toMatchObject({
885+
name: 'search',
886+
path: '/',
887+
params: { count: 20 },
888+
query: { count: '20', other: 'value' },
889+
fullPath: '/?count=20&other=value',
890+
})
891+
})
892+
})
893+
657894
describe('encoding', () => {
658895
const resolver = createFixedResolver([
659896
{ name: 'any-path', path: ANY_PATH_PATTERN_MATCHER },

0 commit comments

Comments
 (0)