TypeScript: Infer params from path

[itsMapleLeaf]

Closes #210

This will probably be a breaking change for TS users who manually typed their path params, I'm not sure if that warrants a major 🤔 Open to alternative approaches as well.

[cbbfcd]

LGTM

[molefrog]

I was wondering if there is any way to keep the backward compatibility for users who want to manually type their params, like:

Route<CustomParams> // params is of type `CustomParams`
Route // the type of params is inferred from path

Maybe we could move RoutePath to the second type argument and provide a default first argument like so:

export function Route<T extends DefaultParams = null, RoutePath extends Path = Path>(

But I have a little experience, so not sure if this will work.

[cbbfcd]

+1

[molefrog]

Is there any way to make the same trick for for the useRoute hook:

const [match, params] = useRoute("/app/:user") // params has the right type

[graphman65]

Hi, I created another PR (not sure how to handle such cases as it's based on the commits of this one): #217 that contains the changes that you mentioned in the comments (backward compatibility and useRoute params type inference).

@itsMapleLeaf Feel free to copy the commit and include/cherry-pick it in your PR, you did most of the work, don't want to take credit for it.

[itsMapleLeaf]

Hi, I created another PR (not sure how to handle such cases as it's based on the commits of this one): #217 that contains the changes that you mentioned in the comments (backward compatibility and useRoute params type inference).

@itsMapleLeaf Feel free to copy the commit and include/cherry-pick it in your PR, you did most of the work, don't want to take credit for it.

Thanks for that, I didn't have the time to come back to this 😅 Went ahead and cherry-picked it

[graphman65]

@itsMapleLeaf Tested it and it works fine!

Let's add this test case and we should be good to go.

diff --git a/types/type-specs.tsx b/types/type-specs.tsx
index 81dae1e..2e2dd1e 100644
--- a/types/type-specs.tsx
+++ b/types/type-specs.tsx
@@ -90,7 +90,13 @@ const invalidParamsWithGeneric: Params<{ id: number }> = { id: 13 }; // $ExpectE
 
 // for pathToRegexp matcher
 <Route path="/:user([a-z]i+)/profile/:tab/:first+/:second*">
-  {({ user, tab, first, second }) => `${user}, ${tab}, ${first}, ${second}`}
+  {({ user, tab, first, second }) => {
+    user; // $ExpectType string
+    tab; // $ExpectType string
+    first; // $ExpectType string
+    second; // $ExpectType string | undefined
+    return `${user}, ${tab}, ${first}, ${second}`;
+  }}
 </Route>;
 
 /*

[itsMapleLeaf]

@graphman65 Done 👍

For future reference, reviews have a "suggestion" feature, which makes it a lot easier to suggest changes, where I can add them in with a button click

[cbbfcd]

+1

[molefrog]

Great work @itsMapleLeaf @graphman65 !

[codecov]

Codecov Report

Merging #211 (c9c2997) into master (fed3426) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #211   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            7         7           
  Lines          250       250           
  Branches        50        50           
=========================================
  Hits           250       250           

Impacted Files	Coverage Δ
test/test-utils.js	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fed3426...c9c2997. Read the comment docs.

[molefrog]

Just released this change as an alpha version 2.8.0-alpha.1, could you check if the types are working?

Tried making a simple sandbox, but it doesn't seem to be working, https://codesandbox.io/s/little-fire-7bi1w?file=/src/App.tsx:410-415

[graphman65]

I just created a sample project using 2.8.0-alpha.1 and it's working fine using typescript@4.4.2
The vscode client in codesanbox (used by the language server and linting) is using 4.1.2 so it should be working too.
I will investigate.

[graphman65]

Ok so I investigated and this is an issue in codesanbox, not wouter.

The page is trying to load this file: https://prod-packager-packages.codesandbox.io/v1/typings/wouter/2.8.0-alpha.1.json but it fails with a 502 error.

But when I set the version of wouter to 2.7.4 it's working just fine and loading this file successfully. Looks like they are caching type info and it's not yet available for the new release.

I found a lot of issues mentioning this error and looks like it got fixed at some point but some users started to face the issue again 5 days ago. (codesandbox/codesandbox-client#5502).

[molefrog]

One thing I've realised is that we still need proper types for the Preact version, which is distributed as a separate npm package wouter-preact. The types for Preact are identical expect for the index.d.ts (see types/preact folder). Since ExtractRouteParams type is going to be probably identical to React's version, I think it makes sense to extract these utility types into a standalone type definition file, like types/route-params.d.ts for example. Let me know if you guys would like to work on that feature as this is something we will need for the upcoming release, otherwise I can start digging.

P.S. Seems like supporting type definitions is becoming a bit PITA, so I'm really thinking about converting the source code into TypeScript in v3 of the library 🤔