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(typescript): Fix incorrect type generated for glob route parameters | Add type tests #6364

Merged
merged 26 commits into from
Sep 16, 2022
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
3330c73
Move route parser to inside rwjsRouter
dac09 Sep 6, 2022
09e462e
Simple tsdlite setup with jest
dac09 Sep 7, 2022
6c86058
Add tsd tests for routeParamsTypes
dac09 Sep 7, 2022
73d5906
Don't use caret
dac09 Sep 8, 2022
b98bba6
Dedupe yarn
dac09 Sep 8, 2022
db82751
Merge branch 'main' of github.com:redwoodjs/redwood into try/tsd-tests
dac09 Sep 9, 2022
7ca4f78
Break up RouteParser, add aditional tests
dac09 Sep 9, 2022
fbd69a0
Fix glob type |
dac09 Sep 9, 2022
e671ab2
Comment
dac09 Sep 9, 2022
d870521
Merge branch 'main' into try/tsd-tests
dac09 Sep 9, 2022
684f7c0
Rename one of the types
dac09 Sep 9, 2022
806d120
Merge main
dac09 Sep 12, 2022
50a7671
Use a ts file for types instead,
dac09 Sep 12, 2022
1845e62
Update packages/router/src/routeParamsTypes.ts
dac09 Sep 12, 2022
6fdeae5
Add extra tsconfig to typetests |
dac09 Sep 12, 2022
88edc39
Update packages/router/src/__typetests__/routeParamsTypes.test.ts
dac09 Sep 12, 2022
d860055
Merge branch 'main' into try/tsd-tests
dac09 Sep 13, 2022
061bcc8
Add a couple of more router matchPath tests
Tobbe Sep 15, 2022
8706a05
Merge branch 'main' into try/tsd-tests
dac09 Sep 15, 2022
e68fb7f
Add more tests
dac09 Sep 15, 2022
64758f6
Fix the strange edge cases
dac09 Sep 15, 2022
c1d424f
Use expectType in a different way for more accurate tests
dac09 Sep 15, 2022
9e3f586
Add FAQ to tests
dac09 Sep 15, 2022
37e1c79
tweak messages
dac09 Sep 15, 2022
7359f8c
Merge branch 'main' into try/tsd-tests
Tobbe Sep 16, 2022
b80f2d2
Merge branch 'main' into try/tsd-tests
dac09 Sep 16, 2022
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
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
"@testing-library/react": "12.1.5",
"@testing-library/react-hooks": "8.0.1",
"@testing-library/user-event": "14.4.3",
"@tsd/typescript": "4.8.2",
"@types/babel__generator": "7.6.4",
"@types/fs-extra": "9.0.13",
"@types/jest": "29.0.1",
Expand All @@ -81,6 +82,7 @@
"fs-extra": "10.1.0",
"is-port-reachable": "3.1.0",
"jest": "29.0.3",
"jest-runner-tsd": "4.0.0",
"jscodeshift": "0.13.1",
"lerna": "5.4.3",
"lodash.template": "4.5.0",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,7 @@
import '@redwoodjs/router'
import { RouteParams, QueryParams } from '@redwoodjs/router'
import { A } from 'ts-toolbelt'



type RouteParams<Route> = ${"Route extends `${string}/${infer Rest}`"}
? A.Compute<ParsedParams<Rest>>
: {}

type QueryParams = Record<string | number, string | number | boolean>

declare module '@redwoodjs/router' {
interface AvailableRoutes {
// Only "<Route />" components with a "name" and "path" prop will be populated here.
Expand All @@ -20,39 +13,3 @@ ${routes.map(
}
}

type ParamType<match> = match extends 'Int'
? number
: match extends 'Boolean'
? boolean
: match extends 'Float'
? number
: string

// Path string parser for Redwood Routes
type ParsedParams<PartialRoute> =
// {a:Int}/[...moar]
${"PartialRoute extends `{${infer Param}:${infer Match}}/${infer Rest}`"}
? // check for greedy match e.g. {b}/{c:Int}
// Param = b}/{c, Rest2 = {c, Match = Int so we reconstruct the old one {c + : + Int + }
${"Param extends `${infer Param2}}/${infer Rest2}`"}
? { [ParamName in Param2]: string } &
${"ParsedParams<`${Rest2}:${Match}}`> &"}
${"ParsedParams<`${Rest}`>"}
${": { [Entry in Param]: ParamType<Match> } & ParsedParams<`${Rest}`>"}
: // has type, but at the end e.g. {d:Int}
${"PartialRoute extends `{${infer Param}:${infer Match}}`"}
? // Greedy match order 2
${"Param extends `${infer Param2}}/${infer Rest2}`"}
? { [ParamName in Param2]: string } &
${"ParsedParams<`${Rest2}:${Match}}`>"}
: { [Entry in Param]: ParamType<Match> }
: // no type, but has stuff after it, e.g. {c}/{d}
${"PartialRoute extends `{${infer Param}}/${infer Rest}`"}
${"? { [ParamName in Param]: string } & ParsedParams<`${Rest}`>"}
: // last one with no type e.g. {d}
${"PartialRoute extends `{${infer Param}}`"}
? { [ParamName in Param]: string }
: // if theres a non param
${"PartialRoute extends `${string}/${infer Rest}`"}
${"? ParsedParams<`${Rest}`>"}
: {}
18 changes: 16 additions & 2 deletions packages/router/jest.config.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,19 @@
/** @type {import('@jest/types').Config.InitialOptions} */
module.exports = {
setupFilesAfterEnv: ['./jest.setup.js'],
testEnvironment: 'jest-environment-jsdom',
projects: [
{
displayName: 'code',
setupFilesAfterEnv: ['./jest.setup.js'],
testEnvironment: 'jest-environment-jsdom',
testMatch: ['**/*.test.+(ts|tsx|js)', '!**/__typetests__/*.ts'],
},
{
displayName: {
color: 'blue',
name: 'types',
},
runner: 'jest-runner-tsd',
testMatch: ['**/__typetests__/*.test.ts'],
},
],
}
2 changes: 1 addition & 1 deletion packages/router/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"build:types": "tsc --build --verbose",
"build:watch": "nodemon --watch src --ext \"js,ts,tsx\" --ignore dist --exec \"yarn build\"",
"prepublishOnly": "NODE_ENV=production yarn build",
"test": "jest src",
"test": "jest",
"test:watch": "yarn test --watch"
},
"dependencies": {
Expand Down
105 changes: 105 additions & 0 deletions packages/router/src/__typetests__/routeParamsTypes.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
import { expectAssignable } from 'tsd-lite'

// @WARN!: I'm importing this from the built package.
// So you will need to build, before running test again!
// See https://github.com/jest-community/jest-runner-tsd/issues/111
dac09 marked this conversation as resolved.
Show resolved Hide resolved
import type { RouteParams, ParamType } from '../routeParamsTypes'

describe('RouteParams<>', () => {
test('Single parameters', () => {
expectAssignable<RouteParams<'bazinga/{id:Int}'>>({
id: 1,
})
})

test('Route string with no types defaults to string', () => {
expectAssignable<RouteParams<'/blog/{year}/{month}/{day}/{slug}'>>({
year: '2020',
month: '01',
day: '01',
slug: 'hello-world',
})
})

test('Custom param types', () => {
const customParams = {
name: 'hello-world-slug',
}

expectAssignable<RouteParams<'/post/{name:slug}'>>(customParams)
})

test('Multiple Glob route params', () => {
const globRoutes = {
fromDate: '2021/11/03',
toDate: '2021/11/17',
}

expectAssignable<RouteParams<'/from/{fromDate...}/to/{toDate...}'>>(
globRoutes
)
})

test('Single Glob route params', () => {
const globRoutes = {
fromDate: '2021/11/03',
}

expectAssignable<RouteParams<'/from/{fromDate...}'>>(globRoutes)
})

test('Glob params in the middle', () => {
test('Multiple Glob route params', () => {
const middleGlob = {
folders: 'src/lib/auth.js',
}

expectAssignable<RouteParams<'/repo/{folders...}/edit'>>(middleGlob)
})
})

test('Mixed typed and untyped params', () => {
const untypedFirst = {
b: 'bazinga',
c: true,
}

const typedFirst = {
b: 1245,
c: 'stringy-string',
}

expectAssignable<RouteParams<'/mixed/{b}/{c:Boolean}'>>(untypedFirst)
expectAssignable<RouteParams<'/mixed/{b:Float}/{c}'>>(typedFirst)
})

test('Params in the middle', () => {
const paramsInTheMiddle = {
authorId: 'id:author',
id: 10,
}

expectAssignable<RouteParams<'/posts/{authorId:string}/{id:Int}/edit'>>(
paramsInTheMiddle
)
})
})

describe('ParamType<>', () => {
test('Float', () => {
expectAssignable<ParamType<'Float'>>(1.02)
})

test('Boolean', () => {
expectAssignable<ParamType<'Boolean'>>(true)
expectAssignable<ParamType<'Boolean'>>(false)
})

test('Int', () => {
expectAssignable<ParamType<'Int'>>(51)
})

test('String', () => {
expectAssignable<ParamType<'String'>>('bazinga')
})
})
10 changes: 10 additions & 0 deletions packages/router/src/__typetests__/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"extends": "../../tsconfig.json",
"compilerOptions": {
"composite": false,
"incremental": true,
"declaration": false,
"declarationMap": false,
"emitDeclarationOnly": false
},
}
3 changes: 3 additions & 0 deletions packages/router/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,6 @@ export interface AvailableRoutes {
}

export { SkipNavLink, SkipNavContent } from '@reach/skip-nav'

// Used by packages/internal/src/generate/templates/web-routerRoutes.d.ts.template
export * from './routeParamsTypes'
114 changes: 114 additions & 0 deletions packages/router/src/routeParamsTypes.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
import { A } from 'ts-toolbelt'

type GenericParams = Record<string | number, string | number | boolean>

export type QueryParams = GenericParams

export type RouteParams<Route> = Route extends `${string}/${infer Rest}`
? A.Compute<ParsedParams<Rest>>
: GenericParams

export type ParamType<match> = match extends 'Int'
? number
: match extends 'Boolean'
? boolean
: match extends 'Float'
? number
: string

// This is used for a specific case where the first param
// doesnt have a type, but second one does
// See comment above it's usage
type ParamsFromGreedyMatch<
TParam extends string,
TMatch extends string,
TRest extends string
> = {
[ParamName in TParam as RemoveGlobDots<ParamName>]: string
} & ParsedParams<`${TRest}:${TMatch}}`> &
ParsedParams<`${TRest}`>

// Note that this has to be first in the list, because it does greedy param checks
type TypedParamInFront<
TParam extends string,
TMatch extends string,
TRest extends string
> = TParam extends `${infer Param2}}/${infer Rest2}`
? // check for greedy match (basically if the param contains a slash in it)
// e.g. in {b}/{c:Int} it matches b}/{c as the param
// Rest2 = {c, Match = Int so we reconstruct the old one {c + : + Int + }
ParamsFromGreedyMatch<Param2, TMatch, Rest2>
: // Otherwise its a regular match
{
[ParamName in TParam]: ParamType<TMatch>
} & ParsedParams<`${TRest}`>

// This is the second part of greedy match
// has type, but at the end e.g. {d:Int} or d:Int} <-- no opening brace
// Needs to be right after TypedParamInFront
type TypedParamAtEnd<
TParam extends string,
TMatch extends string
> = TParam extends `${infer Param2}}/${infer Rest2}`
? {
[ParamName in Param2]: string
} & ParsedParams<`${Rest2}:${TMatch}}`>
: { [ParamName in TParam]: ParamType<TMatch> }

// This mapper takes a param name and will remove dots if its a glob
// e.g. fromDate... -> fromDate
// Only used when the param doesn't have a type, because glob params dont have types
type RemoveGlobDots<Param> = Param extends `${infer GlobParamName}...`
? GlobParamName
: Param

// no type, but has stuff after it, e.g. {c}/{d} or {c}/bazinga
type MultiParamsWithoutType<TParam extends string, TRest extends string> = {
[ParamName in TParam as RemoveGlobDots<ParamName>]: string
} & ParsedParams<`${TRest}`>

type JustParamNoType<TParam extends string> = {
[ParamName in TParam as RemoveGlobDots<ParamName>]: string
}

// Path string parser for Redwood Routes
type ParsedParams<PartialRoute> =
// PartialRoute extends `{${infer GlobParam}...}/${infer Rest}}`
// ? ParsedParams<GlobParam> & ParsedParams<Rest>
// : // {a:Int}/[...moar]
PartialRoute extends `{${infer Param}:${infer Match}}/${infer Rest}`
? TypedParamInFront<Param, Match, Rest>
: // has type, but at the end e.g. {d:Int}
PartialRoute extends `{${infer Param}:${infer Match}}`
? // Greedy match order 2
TypedParamAtEnd<Param, Match>
: // no type, but has stuff after it, e.g. {c}/{d} or {c}/bazinga
PartialRoute extends `{${infer Param}}/${infer Rest}`
? MultiParamsWithoutType<Param, Rest>
: // last one with no type e.g. {d} - just a param
PartialRoute extends `{${infer Param}}`
? JustParamNoType<Param>
: // if theres a non param
PartialRoute extends `${string}/${infer Rest}`
? ParsedParams<`${Rest}`>
: // Fallback when doesn't match any of these
Record<string | number, any>

/**
* Translation in pseudocode without ternaries
*
if ('{c:Int}/...rest') {
checkForGreedyMatch()
} else if ('{c:Int}') {
typedParamAtEnd()
} else if ('{c}/...rest') {
multipleParamsNoTypes()
} else if('{d}') {
justParamNoType()
} else if ('bazinga/..rest') {
// Call itself
parseParamsRecursiveCall(rest)
}


**/
Loading