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

Add generics for Remix type enhancements #10843

Merged
merged 2 commits into from
Sep 5, 2023

Conversation

brophdawg11
Copy link
Contributor

Adding TS generics to support remix-run/remix#7319

@changeset-bot
Copy link

changeset-bot bot commented Sep 5, 2023

🦋 Changeset detected

Latest commit: 25622d5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages
Name Type
react-router-dom Patch
react-router Patch
@remix-run/router Patch
react-router-dom-v5-compat Patch
react-router-native Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

params: AgnosticRouteMatch["params"];
data: unknown;
handle: unknown;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is now exported from utils.ts as UIMatch

@brophdawg11 brophdawg11 merged commit b6614b0 into release-next Sep 5, 2023
3 checks passed
@brophdawg11 brophdawg11 deleted the brophdawg11/align-types branch September 5, 2023 19:54
@github-actions
Copy link
Contributor

github-actions bot commented Sep 5, 2023

🤖 Hello there,

We just published version 6.16.0-pre.1 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

Copy link
Member

@MichaelDeBoey MichaelDeBoey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the light of named tuple elements, I've seen something passing by to do the same for generic elements as well, so I think it would be better to have long names instead

https://devblogs.microsoft.com/typescript/announcing-typescript-5-2/#named-and-anonymous-tuple-elements

/**
* An entry in a history stack. A location contains information about the
* URL path, as well as possibly some arbitrary state and a key.
*/
export interface Location extends Path {
export interface Location<S = any> extends Path {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
export interface Location<S = any> extends Path {
export interface Location<State = any> extends Path {

/**
* Arguments passed to loader functions
*/
export interface LoaderFunctionArgs extends DataFunctionArgs {}
export interface LoaderFunctionArgs<C = any> extends DataFunctionArgs<C> {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
export interface LoaderFunctionArgs<C = any> extends DataFunctionArgs<C> {}
export interface LoaderFunctionArgs<Context = any> extends DataFunctionArgs<Context> {}


/**
* Arguments passed to action functions
*/
export interface ActionFunctionArgs extends DataFunctionArgs {}
export interface ActionFunctionArgs<C = any> extends DataFunctionArgs<C> {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
export interface ActionFunctionArgs<C = any> extends DataFunctionArgs<C> {}
export interface ActionFunctionArgs<Context = any> extends DataFunctionArgs<Context> {}

Comment on lines +169 to +170
export interface LoaderFunction<C = any> {
(args: LoaderFunctionArgs<C>): Promise<DataFunctionValue> | DataFunctionValue;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
export interface LoaderFunction<C = any> {
(args: LoaderFunctionArgs<C>): Promise<DataFunctionValue> | DataFunctionValue;
export interface LoaderFunction<Context = any> {
(args: LoaderFunctionArgs<Context>): Promise<DataFunctionValue> | DataFunctionValue;

Comment on lines +176 to +177
export interface ActionFunction<C = any> {
(args: ActionFunctionArgs<C>): Promise<DataFunctionValue> | DataFunctionValue;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
export interface ActionFunction<C = any> {
(args: ActionFunctionArgs<C>): Promise<DataFunctionValue> | DataFunctionValue;
export interface ActionFunction<Context = any> {
(args: ActionFunctionArgs<Context>): Promise<DataFunctionValue> | DataFunctionValue;

@@ -490,6 +493,28 @@ export function matchRoutes<
return matches;
}

export interface UIMatch<D = unknown, H = unknown> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
export interface UIMatch<D = unknown, H = unknown> {
export interface UIMatch<Data = unknown, Handle = unknown> {

Comment on lines +500 to +501
data: D;
handle: H;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
data: D;
handle: H;
data: Data;
handle: Handle;

@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version 6.16.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants