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

Clean up types within consolidated package #12177

Merged
merged 6 commits into from
Oct 23, 2024
Merged

Conversation

brophdawg11
Copy link
Contributor

No description provided.

Copy link

changeset-bot bot commented Oct 22, 2024

🦋 Changeset detected

Latest commit: 6c6c57f

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

This PR includes changesets to release 10 packages
Name Type
@react-router/dev Major
react-router Major
@react-router/fs-routes Major
@react-router/remix-config-routes-adapter Major
@react-router/architect Major
@react-router/cloudflare Major
react-router-dom Major
@react-router/express Major
@react-router/node Major
@react-router/serve Major

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

}
routes[parentId].push(route);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • Remix RouteManifest was Record<string, Route>
  • React Router's internal RouteManifest was more accurately Record<string, Route | undefined>

Switching to the stricter type means we have to introduce some defensiveness in existing Remix code - which feels more appropriate given the new fog of war behavior

/**
* Data for a route that was returned from a `loader()`.
*/
export type AppData = 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.

Got rid of this - just used unknown in the few spots this was used

ActionFunction,
ActionFunctionArgs,
LoaderFunction,
LoaderFunctionArgs,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can just use these directly now since there are no "Remix" versions anymore

headers: Headers;
};
}
) => ReturnType<RRLoaderFunction>;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

🫡

| undefined;
params: Params;
location: Location;
matches: MetaMatches<MatchLoaders>;
error?: 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.

Copied this comment from the duped server runtime type

Comment on lines +340 to +343
export type RouteManifest<R = AgnosticDataRouteObject> = Record<
string,
R | undefined
>;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a generic here so the remix layers could pass in their route type

/**
* @deprecated Use `LoaderFunctionArgs`/`ActionFunctionArgs` instead
*/
export type DataFunctionArgs = RRActionFunctionArgs<AppLoadContext> &
Copy link
Contributor Author

Choose a reason for hiding this comment

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

All these can go away and we just use the RR types - and users will add context: appLoadContext via module augmentation: https://reactrouter.com/dev/guides/upgrading/remix#step-7---update-types-for-apploadcontext

* A function that handles data mutations for a route on the client
* @private Public API is exported from @react-router/react
*/
type ClientActionFunction = (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These client types were duped here and in the @remix-run/react package so we just keep the other ones.

* A function that defines `<link>` tags to be inserted into the `<head>` of
* the document on route transitions.
*/
export interface LinksFunction {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Kill link/meta types in favor of the ones from @remix-run/react

@brophdawg11 brophdawg11 merged commit 5a363e5 into dev Oct 23, 2024
8 checks passed
@brophdawg11 brophdawg11 deleted the brophdawg11/clean-up-types branch October 23, 2024 19:13
Copy link
Contributor

🤖 Hello there,

We just published version 7.0.0-pre.2 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
Contributor

🤖 Hello there,

We just published version 7.0.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.

1 participant