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

Feat: Improve generated types to include optional types #697

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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'));
Loading