Repository: tighten/ziggy
Pull request #697 (2023-12-18 11:34:50)

Feat: Improve generated types to include optional types
Improve the generated typescript file to include optional params on `KnownRouteParams` object, instead of marking everything as optional

![CleanShot 2023-12-19 at 16 30 11](https://github.com/tighten/ziggy/assets/2529110/d40c8cc9-d61d-47d3-806b-3bf2bfe932cc)
![CleanShot 2023-12-19 at 16 29 54](https://github.com/tighten/ziggy/assets/2529110/3f5ccb2b-79ae-4139-8ed2-02763147b1c1)



Patch:
diff --git a/src/Output/Types.php b/src/Output/Types.php
index 5b7a8fd..6269c58 100644
--- a/src/Output/Types.php
+++ b/src/Output/Types.php
@@ -3,6 +3,7 @@
 namespace Tighten\Ziggy\Output;
 
 use Illuminate\Support\Arr;
+use Illuminate\Support\Str;
 use Stringable;
 use Tighten\Ziggy\Ziggy;
 
@@ -20,8 +21,8 @@ class Types implements Stringable
         $routes = collect($this->ziggy->toArray()['routes'])->map(function ($route) {
             return collect($route['parameters'] ?? [])->map(function ($param) use ($route) {
                 return Arr::has($route, "bindings.{$param}")
-                    ? ['name' => $param, 'binding' => $route['bindings'][$param]]
-                    : ['name' => $param];
+                    ? ['name' => $param, 'required' => ! Str::contains($route['uri'], "{$param}?"), 'binding' => $route['bindings'][$param]]
+                    : ['name' => $param, 'required' => ! Str::contains($route['uri'], "{$param}?")];
             });
         });
 
diff --git a/src/js/index.d.ts b/src/js/index.d.ts
index 5de3735..bf0c5dc 100644
--- a/src/js/index.d.ts
+++ b/src/js/index.d.ts
@@ -21,7 +21,7 @@ type RouteName = KnownRouteName | (string & {});
 /**
  * Information about a single route parameter.
  */
-type ParameterInfo = { name: string; binding?: string };
+type ParameterInfo = { name: string; required: boolean; binding?: string };
 
 /**
  * A primitive route parameter value, as it would appear in a URL.
@@ -43,15 +43,19 @@ type ParameterValue = RawParameterValue | DefaultRoutable;
  * A parseable route parameter, either plain or nested inside an object under its binding key.
  */
 type Routable<I extends ParameterInfo> = I extends { binding: string }
-    ? { [K in I['binding']]: RawParameterValue } | RawParameterValue
+    ? ({ [K in I['binding']]: RawParameterValue } & Record<keyof any, unknown>) | RawParameterValue
     : ParameterValue;
 
 // Uncomment to test:
-// type A = Routable<{ name: 'foo', binding: 'bar' }>;
+// type A = Routable<{ name: 'foo', required: true, binding: 'bar' }>;
 // = RawParameterValue | { bar: RawParameterValue }
-// type B = Routable<{ name: 'foo' }>;
+// type B = Routable<{ name: 'foo', required: true, }>;
 // = RawParameterValue | DefaultRoutable
 
+// Utility types for KnownRouteParamsObject
+type RequiredParams<I extends readonly ParameterInfo[]> = Extract<I[number], { required: true }>;
+type OptionalParams<I extends readonly ParameterInfo[]> = Extract<I[number], { required: false }>;
+
 /**
  * An object containing a special '_query' key to target the query string of a URL.
  */
@@ -64,13 +68,19 @@ type GenericRouteParamsObject = Record<keyof any, unknown> & HasQueryParam;
 /**
  * An object of parameters for a specific named route.
  */
-// TODO: The keys here could be non-optional (or more detailed) if we can determine which params are required/not.
 type KnownRouteParamsObject<I extends readonly ParameterInfo[]> = {
-    [T in I[number] as T['name']]?: Routable<T>;
+    [T in RequiredParams<I> as T['name']]: Routable<T>;
+} & {
+    [T in OptionalParams<I> as T['name']]?: Routable<T>;
 } & GenericRouteParamsObject;
 // `readonly` allows TypeScript to determine the actual values of all the
 // parameter names inside the array, instead of just seeing `string`.
 // See https://github.com/tighten/ziggy/pull/664#discussion_r1329978447.
+
+// Uncomment to test:
+// type A = KnownRouteParamsObject<[{ name: 'foo'; required: true }, { name: 'bar'; required: false }]>;
+// = { foo: ... } & { bar?: ... }
+
 /**
  * An object of route parameters.
  */
@@ -98,7 +108,7 @@ type KnownRouteParamsArray<I extends readonly ParameterInfo[]> = [
 // See https://github.com/tighten/ziggy/pull/664#discussion_r1330002370.
 
 // Uncomment to test:
-// type B = KnownRouteParamsArray<[{ name: 'post', binding: 'uuid' }]>;
+// type B = KnownRouteParamsArray<[{ name: 'post'; required: true; binding: 'uuid' }]>;
 // = [RawParameterValue | { uuid: RawParameterValue }, ...unknown[]]
 
 /**
@@ -111,7 +121,7 @@ type RouteParamsArray<N extends RouteName> = N extends KnownRouteName
 /**
  * All possible parameter argument shapes for a route.
  */
-type RouteParams<N extends RouteName> = ParameterValue | RouteParamsObject<N> | RouteParamsArray<N>;
+type RouteParams<N extends RouteName> = RouteParamsObject<N> | RouteParamsArray<N>;
 
 /**
  * A route.
@@ -146,7 +156,7 @@ interface Config {
  */
 interface Router {
     current(): RouteName | undefined;
-    current<T extends RouteName>(name: T, params?: RouteParams<T>): boolean;
+    current<T extends RouteName>(name: T, params?: ParameterValue | RouteParams<T>): boolean;
     get params(): Record<string, unknown>;
     has<T extends RouteName>(name: T): boolean;
 }
@@ -163,6 +173,12 @@ export function route<T extends RouteName>(
     absolute?: boolean,
     config?: Config,
 ): string;
+export function route<T extends RouteName>(
+    name: T,
+    params?: ParameterValue | undefined,
+    absolute?: boolean,
+    config?: Config,
+): string;
 // Called with configuration arguments only - returns a configured Router instance
 export function route(
     name: undefined,


Test patch:
diff --git a/tests/Unit/CommandRouteGeneratorTest.php b/tests/Unit/CommandRouteGeneratorTest.php
index 659c770..b5ee1d2 100644
--- a/tests/Unit/CommandRouteGeneratorTest.php
+++ b/tests/Unit/CommandRouteGeneratorTest.php
@@ -147,6 +147,7 @@ class CommandRouteGeneratorTest extends TestCase
     {
         app('router')->get('posts', $this->noop())->name('posts.index');
         app('router')->post('posts/{post}/comments', PostCommentController::class)->name('postComments.store');
+        app('router')->post('posts/{post}/comments/{comment?}', PostCommentController::class)->name('postComments.storeComment');
         app('router')->getRoutes()->refreshNameLookups();
 
         Artisan::call('ziggy:generate',  ['--types' => true]);
diff --git a/tests/fixtures/ziggy-7.d.ts b/tests/fixtures/ziggy-7.d.ts
index c71ee83..063e49b 100644
--- a/tests/fixtures/ziggy-7.d.ts
+++ b/tests/fixtures/ziggy-7.d.ts
@@ -4,16 +4,19 @@ declare module 'ziggy-js' {
     "posts.index": [],
     "postComments.show": [
         {
-            "name": "post"
+            "name": "post",
+            "required": true
         },
         {
             "name": "comment",
+            "required": true,
             "binding": "uuid"
         }
     ],
     "postComments.store": [
         {
-            "name": "post"
+            "name": "post",
+            "required": true
         }
     ]
 }
diff --git a/tests/fixtures/ziggy.d.ts b/tests/fixtures/ziggy.d.ts
index 90ea8c8..1fc1691 100644
--- a/tests/fixtures/ziggy.d.ts
+++ b/tests/fixtures/ziggy.d.ts
@@ -4,7 +4,18 @@ declare module 'ziggy-js' {
     "posts.index": [],
     "postComments.store": [
         {
-            "name": "post"
+            "name": "post",
+            "required": true
+        }
+    ],
+    "postComments.storeComment": [
+        {
+            "name": "post",
+            "required": true
+        },
+        {
+            "name": "comment",
+            "required": false
         }
     ]
 }
diff --git a/tests/js/route.test-d.ts b/tests/js/route.test-d.ts
index 102c2e4..7d6ca79 100644
--- a/tests/js/route.test-d.ts
+++ b/tests/js/route.test-d.ts
@@ -6,24 +6,24 @@ import { route } from '../../src/js';
 declare module '../../src/js' {
     interface RouteList {
         'posts.index': [];
-        'posts.comments.store': [{ name: 'x' }];
-        'posts.comments.show': [{ name: 'post' }, { name: 'comment'; binding: 'uuid' }];
-        optional: [{ name: 'maybe' }];
+        'posts.comments.store': [{ name: 'post'; required: true }];
+        'posts.comments.show': [
+            { name: 'post'; required: true },
+            { name: 'comment'; required: false; binding: 'uuid' },
+        ];
+        optional: [{ name: 'maybe'; required: false }];
     }
 }
 
-// Test route name autocompletion
+// Test route name autocompletion by adding quotes inside `route()` - should suggest route names above
 assertType(route());
 
-// Test route parameter name autocompletion
-assertType(route('posts.comments.store', {}));
+// Test route parameter name autocompletion by adding more keys to the parameter object - should show info about params, e.g. for the 'comment' param if you type `c`
+assertType(route('posts.comments.show', { post: 1 }));
 
-// TODO once we can detect whether params are required/optional: @ts-expect-error missing required 'post' parameter
+// @ts-expect-error missing required 'post' parameter
 assertType(route('posts.comments.show', { comment: 2 }));
 
-// TODO once we can detect whether params are required/optional: @ts-expect-error missing required 'comment' parameter
-assertType(route('posts.comments.show', { post: 2 }));
-
 // Simple object example
 assertType(route('posts.comments.show', { post: 2, comment: 9 }));
 // Allows extra object properties
@@ -39,9 +39,9 @@ assertType(route('posts.comments.show', { post: { id: 2, foo: 'bar' } }));
 assertType(route('posts.comments.show', { post: { foo: 'bar' } }));
 
 // Binding/'routable' object example with custom 'uuid' binding
-assertType(route('posts.comments.show', { comment: { uuid: 1 } }));
+assertType(route('posts.comments.show', { comment: { uuid: 1 }, post: '1' }));
 // Allows extra nested object properties
-assertType(route('posts.comments.show', { comment: { uuid: 1, foo: 'bar' } }));
+assertType(route('posts.comments.show', { comment: { uuid: 1, foo: 'bar' }, post: '1' }));
 // @ts-expect-error missing 'uuid' key in comment parameter object
 assertType(route('posts.comments.show', { comment: { foo: 'bar' } }));
 // @ts-expect-error missing 'uuid' key in comment parameter object
@@ -49,10 +49,20 @@ assertType(route('posts.comments.show', { comment: { foo: 'bar' } }));
 // parameter has an explicit 'uuid' binding, so that's required :)
 assertType(route('posts.comments.show', { comment: { id: 2 } }));
 
+// Plain values
+assertType(route('posts.comments.show', 2));
+assertType(route('posts.comments.show', 'foo'));
+
+// TODO @ts-expect-error parameter argument itself is required
+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, 3]));
+// assertType(route('posts.comments.show', ['foo'])); // TODO shouldn't error, only one required param
 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 }, 3]));
 assertType(route('posts.comments.show', ['2', { uuid: 3 }]));
 assertType(route('posts.comments.show', [{ id: 2 }, { uuid: '3' }]));
@@ -78,3 +88,12 @@ assertType(route().has(''));
 
 // Test router getter autocompletion
 assertType(route().params);
+
+assertType(route().current('missing', { foo: 1 }));
+
+// @ts-expect-error missing required 'post' parameter
+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', 'foo'));