Skip to content

Commit

Permalink
Merge pull request #697 from alexmccabe/feat-improve-optional-paramet…
Browse files Browse the repository at this point in the history
…er-types

Feat: Improve generated types to include optional types
  • Loading branch information
bakerkretzmar committed Mar 25, 2024
2 parents 3b71873 + bc1ce2b commit 48f137e
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 26 deletions.
5 changes: 3 additions & 2 deletions src/Output/Types.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace Tighten\Ziggy\Output;

use Illuminate\Support\Arr;
use Illuminate\Support\Str;
use Stringable;
use Tighten\Ziggy\Ziggy;

Expand All @@ -20,8 +21,8 @@ public function __toString(): string
$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}?")];
});
});

Expand Down
34 changes: 25 additions & 9 deletions src/js/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
*/
Expand All @@ -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.
*/
Expand Down Expand Up @@ -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[]]

/**
Expand All @@ -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.
Expand Down Expand Up @@ -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;
}
Expand All @@ -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,
Expand Down
1 change: 1 addition & 0 deletions tests/Unit/CommandRouteGeneratorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ public function can_generate_dts_file()
{
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]);
Expand Down
7 changes: 5 additions & 2 deletions tests/fixtures/ziggy-7.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
]
}
Expand Down
13 changes: 12 additions & 1 deletion tests/fixtures/ziggy.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
]
}
Expand Down
43 changes: 31 additions & 12 deletions tests/js/route.test-d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -39,20 +39,30 @@ 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
// 'id' doesn't fix it because 'id' is the default/fallback but this
// 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' }]));
Expand All @@ -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'));

0 comments on commit 48f137e

Please sign in to comment.