Merge pull request #878 from pataar/fix/optional-params-array-types

Fix TypeError when optional params omitted from array (fixes #736)

Author: bakerkretzmar

src/js/index.d.ts

@@ -114,16 +114,28 @@ type RouteParamsObject<N extends RouteName> = N extends KnownRouteName
 type GenericRouteParamsArray = unknown[];
 /**
  * An array of parameters for a specific named route.
+ *
+ * Required parameters come first as required tuple elements, then optional
+ * parameters become optional tuple elements, followed by arbitrary extras.
  */
 type KnownRouteParamsArray<I extends readonly ParameterInfo[]> = [
-    ...{ [K in keyof I]: Routable<I[K]> },
-    ...unknown[],
+    ...RequiredParamsTuple<I>,
+    ...OptionalParamsTuple<I>,
 ];
-// Because `K in keyof I` for a `readonly` array is always a number, even though this
-// looks like `{ 0: T, 1: U, 2: V }` TypeScript generates `[T, U, V]`. The nested
-// array destructing lets us type the first n items in the array, which are known
-// route parameters, and then allow arbitrary additional items.
-// See https://github.com/tighten/ziggy/pull/664#discussion_r1330002370.
+
+type RequiredParamsTuple<I extends readonly ParameterInfo[]> =
+    I extends readonly [infer F extends ParameterInfo, ...infer R extends ParameterInfo[]]
+        ? F extends { required: true }
+            ? [Routable<F>, ...RequiredParamsTuple<R>]
+            : []
+        : [];
+
+type OptionalParamsTuple<I extends readonly ParameterInfo[]> =
+    I extends readonly [infer F extends ParameterInfo, ...infer R extends ParameterInfo[]]
+        ? F extends { required: false }
+            ? [Routable<F>?, ...OptionalParamsTuple<R>]
+            : OptionalParamsTuple<R>
+        : [...unknown[]];
 
 // Uncomment to test:
 // type B = KnownRouteParamsArray<[{ name: 'post'; required: true; binding: 'uuid' }]>;

tests/js/route.test-d.ts

@@ -65,12 +65,12 @@ assertType(route('posts.comments.show', 'foo'));
 assertType(route('posts.comments.show'));
 
 // Simple array examples
-// assertType(route('posts.comments.show', [2])); // TODO shouldn't error, only one required param
+assertType(route('posts.comments.show', [2]));
 assertType(route('posts.comments.show', [2, 3]));
-// assertType(route('posts.comments.show', ['foo'])); // TODO shouldn't error, only one required param
+assertType(route('posts.comments.show', ['foo']));
 assertType(route('posts.comments.show', ['foo', 'bar']));
 // Allows mix of plain values and parameter objects
-// assertType(route('posts.comments.show', [{ id: 2 }])); // TODO shouldn't error, only one required param
+assertType(route('posts.comments.show', [{ id: 2 }]));
 assertType(route('posts.comments.show', [{ id: 2 }, 3]));
 assertType(route('posts.comments.show', ['2', { uuid: 3 }]));
 assertType(route('posts.comments.show', [{ id: 2 }, { uuid: '3' }]));
@@ -105,9 +105,13 @@ assertType(route().current('missing', { foo: 1 }));
 assertType(route().current('posts.comments.show', { comment: 2 }));
 assertType(route().current('posts.comments.show', { post: 2 }));
 assertType(route().current('posts.comments.show', 2));
-// assertType(route().current('posts.comments.show', [2])); // TODO shouldn't error, only one required param
+assertType(route().current('posts.comments.show', [2]));
 assertType(route().current('posts.comments.show', 'foo'));
 
+// All-optional route with array params
+assertType(route('optional', []));
+assertType(route('optional', ['foo']));
+
 // Test route function return types
 assertType<string>(route('optional', { maybe: 'foo' }));
 assertType<string>(route('optional', 'foo'));