-
-
Notifications
You must be signed in to change notification settings - Fork 10.4k
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
defineRoute #11596
defineRoute #11596
Conversation
|
I don’t really like the $ at the end of the name, users will see it and just think it’s weird. It won’t send the message “this function must be statically analyzable”. Instead just print an error from vite if the call is not statically analyzable. |
I'm actually thinking of backing out the |
Apologies if there's a discussion open for this but I couldn't find it. Is there going to be support for importing and using the component that's defined in the // widget/route.tsx
export const defineRoute$( { .... });
// another route/file
import { route.Component as MyWidget } from 'widget/route.tsx';
export default function() {
return <div><MyWidget /></div>
} This is the premise of the full-stack components using resource routes that Kent talked about: https://www.epicweb.dev/full-stack-components. It kinda works right now... but it's really clunky and requires hard-coding the action URL or manually calling fetcher.load for loader data. Being able to import and use these components anywhere in the app (instead of just as routes) would be such a huge game changer for interaction heavy apps. This has been the missing piece in remix for us. |
@willhoney7 you could access the What you are asking for is composability of data fetching and UI anywhere in your app. We had some designs for how to do this a couple years ago, but then RSC came out and changed the foundations of how this could work in React. So for the sort of composability you're asking for, the best bet is to wait for our RSC support to be stabilized. |
// loader -> data for component | ||
// loader -> serverLoader for clientLoader -> data for component | ||
// TODO: clientLoader and all the other route module export APIs (meta, handle, ErrorBoundary, etc.) | ||
export const defineRoute$ = < |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First of all, thank you for your work on improving support for typed routes! 🎉
Do you have plans to support typing for searchParams
in the future? Sorry if this was mentioned already and I missed it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not in the immediate future (RRv7.0 initial release) but the API is flexible to accommodate that later without breaking changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, for the clarification 🐱. Awesome work!
|
||
type MaybePromise<T> = T | Promise<T>; | ||
|
||
type Serializable = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How could we make this Serializable
?
type Foo = {
k: readonly string[]
}
const foo: Foo = {k: ['v']}
const bar: Serializable = foo
Type 'Foo' is not assignable to type 'Serializable'.
Type 'Foo' is not assignable to type '{ [key: string]: Serializable; [key: number]: Serializable; [key: symbol]: Serializable; }'.
Property 'k' is incompatible with index signature.
Type 'readonly string[]' is not assignable to type 'Serializable'.
Type 'readonly string[]' is not assignable to type '{ [key: string]: Serializable; [key: number]: Serializable; [key: symbol]: Serializable; }'.
Index signature for type 'string' is missing in type 'readonly string[]'.ts(2322)
Without readonly
, it works fine.
Sorry, this supposed to be about SingleFetch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea that's a bug. We'll fix it.
loader?: L; | ||
action?: A; | ||
Component?: Component<NoInfer<P>, NoInfer<L>>; | ||
}) => route; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note to self: return never
?
Would be cool if this new way of defining routes would allow for easy route duplications somehow. I'd love to be able to do something like this (current API): const customRoutes = defineRoutes((route) => {
route("/", "routes/_app.tsx"); // Go to /issues and you see all issues you have access to
route("/organization/:organizationId", "routes/_app.tsx"); // Go to /organization/:organizationId/issues and you see all issues for the organization
route("/organization/:organizationId/project/:projectId", "routes/_app.tsx"); // Go to /organization/:organizationId/project/:projectId/issues and you see all issues for the specific project of the organization
}); This currently throws an error:
If we specify unique IDs it prevents the error, but doesn't register the new routes. Anyway, it'd be great if this scenario was possible easily without having to play with redirections & duplications of real route files. |
I appreciate all the excitement around this feature! But I want to keep this PR focused on the current implementation. Any feature requests should instead be made as a discussion in this repo, thanks 🙏 |
I'm landing this feature incrementally in a bunch of smaller PRs so that we can make sure we don't break anything along the way. |
TODO
defineRoute
used elsewhere?readonly
serializable