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] @refinedev/nextjs-router and others should allow configuration #6308

Closed
datner opened this issue Sep 8, 2024 · 11 comments
Closed

[FEAT] @refinedev/nextjs-router and others should allow configuration #6308

datner opened this issue Sep 8, 2024 · 11 comments
Labels
enhancement New feature or request

Comments

@datner
Copy link

datner commented Sep 8, 2024

Is your feature request related to a problem? Please describe.

Trying to use refine without material ui or antd or anything of the sort for personal taste reasons (and a big reason why I adopted refine).
But I found it quite difficult to actually develop a proper customized solution on the most surface level.. For example it is impossible to sufficiently customize the AuthPage without swizzling, but doing the same for the Link of the routerProvider is plain impossible in conventional methods 🤷🏻‍♂️

Describe alternatives you've considered

import routerProvider from "@refinedev/nextjs-router";
import {MyLink} from "@components/my-link";

routerProvider.Link = MyLink as any

Additional context

No response

Describe the thing to improve

makeProvider({...})
@datner datner added the enhancement New feature or request label Sep 8, 2024
@aliemir
Copy link
Member

aliemir commented Sep 16, 2024

Hey @datner, Refine expects provider props to be objects with certain properties. Simply by overriding the Link property, you can configure routerProvider to use your <Link /> component. In its type definition, we expect Link to forward the ref and allow a to prop for href. If your component complies with those requirements then you should not need marking it as any to use.

I think having something like makeProvider is worth discussing but I'm more curious about why you needed marking it as any and what errors did you had before that, can you give us a small reproducible example or some code to share? 🙏

@datner
Copy link
Author

datner commented Sep 22, 2024

CleanShot 2024-09-23 at 02 26 33

import { Url } from "next/dist/shared/lib/router/router";
import NextLink, { type LinkProps } from "next/link";
import React, { forwardRef } from "react";

type Destination = { to: string; href: never } | { href: Url; to?: string };

export const Link = forwardRef(function Link(
  props: Omit<LinkProps, "href"> & Destination & React.ComponentPropsWithoutRef<"a">,
  ref: React.ForwardedRef<HTMLAnchorElement>,
) {
  return (
      <NextLink {...props} href={props.to ?? props.href} ref={ref} />
  );
});

This obviously is not a 1:1 to my component, which has other stuff included. But the point is that the routerProvider requires a less strictly-typed variant of Link than I am comfortable using, even after I loosened the type around to. Because I am not consuming it, I don't mind casting it as any. Though casting or not, needing to import a module and then mutate it at runtime unsafely feels really off Like last time I did this and it was considered the expected practice was back when I used MooTools. You call it 'overriding' the component, but more accurately it's overwriting the object. Honestly I'm shocked that this is what is expected to be done 🫨

@alicanerdurmaz
Copy link
Member

Hello @datner, We released a new <Link /> component on the last release. Since it can take all the props of the <a> element, I think it will meet all your needs.

Please let us know if you encounter any other problems.

@datner
Copy link
Author

datner commented Oct 31, 2024

@alicanerdurmaz Hey, the issue was not solved. It persists in the same manner and can be recreated as described above

CleanShot 2024-10-31 at 16 56 09@2x

The error has changed, but the narrowing issue persists. I don't think it's possible without a makeProvider-like method since you need to use inference

@BatuhanW
Copy link
Member

Hey @datner I think it looks like a type issue, not an issue with Refine. You are providing Validator<string | null | undefined> into a Validator<string> type.

@datner
Copy link
Author

datner commented Nov 1, 2024

@BatuhanW Thank you. But I am well aware of the error. Please read the rest of the issue 🥲

Solving the type error is not a problem. I'm saying that it shouldn't exist because the error is incorrect. Or, more specifically, the type is too strict. But I don't want to widen the type. I suggested a way to configure the router without hacking it in, which would solve the issue.

To quote:

Refine expects provider props to be objects with certain properties. By overriding the Link property, you can configure routerProvider to use your component. In its type definition, we expect Link to forward the ref and allow a to prop for href. If your component complies with those requirements, then you should not need marking it as any to use.

My component allows a to prop for the href, as you can see in my previous comment #6308

Unless you feel like my use-case of using the href prop of next/routers link with its Url interface instead of just using the string type of to for @refinedev/nextjs-router is an invalid use-case?

@alicanerdurmaz
Copy link
Member

Hello @datner,

In the November release, we improved the types of the <Link /> component. Here’s an example of how it can be used:

import { Link } from "@refinedev/core";
import React, {
  type CSSProperties,
  forwardRef,
  type PropsWithChildren,
  type Ref,
} from "react";

type MyCustomLinkProps = PropsWithChildren<{
  href: string;
  style?: CSSProperties;
}>;

const MyCustomLink = forwardRef(
  (
    { children, style, ...props }: MyCustomLinkProps,
    ref: Ref<HTMLAnchorElement>,
  ) => {
    return (
      <Link<MyCustomLinkProps>
        {...props}
        ref={ref}
        style={{ color: "red", ...style }}
        to={props.href}
      >
        {children}
      </Link>
    );
  },
);

export const PostList: React.FC = () => {
  return <MyCustomLink href="/posts">Go to posts</MyCustomLink>;
};

I didn’t fully understand what you want to do or the problem you’re facing. Sorry about that. If you’re still having trouble, could you please explain it in more detail?

@datner
Copy link
Author

datner commented Nov 12, 2024

Hi @alicanerdurmaz ,

You're "looking" in the wrong end here. You're using the existing Link that will use some default Link implementation, or one provided by the RouterContext.
So your example: routerProvider.Link -> refinedev.Link -> MyCustomLink(refinedev.Link)
Everything 'builtin' in the app, will not use your custom link unless manually overridden if accessible, or manually re-implement alongside the rest of the component/page if not accessible.
What this issue is about: MyCustomLink(routerProvider.Link) -> redefinedev.Link, meaning that everything builtin that is using Link from @refinedev/core will use MyCustomLink just like it would use routerProvider.Link.
How? Currently, like this:

import routerProvider from "@refinedev/nextjs-router";
routerProvider.Link = MyCustomLink as any

function RootLayout() {
  return <Refine routerProvider={routerProvider} ...></Refine>
}

What am I suggesting?

import makeProvider from "@refinedev/nextjs-router";

const routerProvider = makeProvider({ Link: MyCustomLink }) // no `any` casting required

function RootLayout() {
  return <Refine routerProvider={routerProvider} ...></Refine>
}

There are many many reasons why you would want this:

  • analytics
  • styling
  • custom behavior
  • nav blocking
  • expressing permissions
  • adding better a11y
  • type safety

And most importantly, leveraging what refinedev has and provides.. This difficulty in customization is prevalent throughout the project, and since I've opened this issue I had to drop refinedev from my project because I found myself just reimplementing the lib in its entirety, benefitting only from the hooks. (which are great!, but they complete the refine suite, they don't make it)

I want to use refine, I see the value and I believe that it's fit and ready and miles ahead of the ecosystem.
Please tell me if I'm the sole eccentric desiring these things, I can't know if I am crazy, but I kept rubbing against these little splinters

@alicanerdurmaz
Copy link
Member

Hello again @datner,

This is my implementation; please let me know if I'm missing anything.

1. Custom Router Provider

Instead of manipulating the routerProvider provider, we can create a new dataProvider. I don’t quite understand why we need makeProvider right now. Since routerProvider is a JavaScript object, we can easily override it.

"use client";

import baseRouterProvider from "@refinedev/nextjs-router";
import { forwardRef, type PropsWithChildren } from "react";

export type MyCustomLinkProps = PropsWithChildren<{
  to: string;
  color: "red" | "green";
  [prop: string]: any;
}>;

const MyCustomLink = forwardRef<HTMLAnchorElement, MyCustomLinkProps>(
  ({ to, color, children, ...props }, ref) => {
    return (
      <a
        {...props}
        href={to}
        style={{
          color,
          ...props.style,
        }}
        ref={ref}
      >
        {children}
      </a>
    );
  },
);

export const routerProvider = {
  ...baseRouterProvider,
  Link: MyCustomLink,
};

2. Usage of custom routerProvider.Link

We can use it by importing from "@refinedev/core".

"use client";

import { Link } from "@refinedev/core";

export default function IndexPage() {
  return (
    <div
      style={{
        display: "flex",
        flexDirection: "column",
        alignItems: "center",
        justifyContent: "center",
        height: "100vh",
        gap: "24px",
      }}
    >
      <Link to="/page-1" color="red">
        To Page 1
      </Link>
      <Link to="/page-2" color="green">
        To Page 2
      </Link>
    </div>
  );
}

3. TypeScript support.

Since we're using the routerProvider.Link component by importing it from @refinedev/core, we currently don't have TypeScript support. We need to give type to the <Link />

"use client";

import { Link } from "@refinedev/core";
import type { MyCustomLinkProps } from "./router-provider";

export default function IndexPage() {
  return (
    <Link<MyCustomLinkProps> to="/page-2" color="pink">
      To Page 2
    </Link>
  );
}

Adding a generic type to every <Link /> component will take much work. Instead, we can re-export a generic version of the <Link /> component.

@BatuhanW
Copy link
Member

Hey @datner your desire is acceptable, using your custom link for analytics and all other features. What we can't clarify is, it's already possible currently. What we call router provider is just a simple object. That returns some methods inside it. So you can already implement makeProvider method yourself. It doesn't need to be built inside Refine.

import routerProvider from "@refinedev/nextjs-router";

const makeProvider = (props) => {
  return { ...routerProvider, ...props }
}

const routerProvider = makeProvider({ Link: MyCustomLink }) // no `any` casting required

function RootLayout() {
  return <Refine routerProvider={routerProvider} ...></Refine>
}

Refine is already designed in a way, that it will use router provider's Link method when possible internally. You don't need to change anything else.

Closing the issue, feel free to re-open it if you see fit.

@BatuhanW BatuhanW closed this as not planned Won't fix, can't repro, duplicate, stale Nov 20, 2024
@aliemir
Copy link
Member

aliemir commented Nov 20, 2024

Not sure if the error was related to an earlier bug or not. I've just tested an implementation with a custom Link component which will be used throughout the project for all <Link /> usages in Refine components and didn't had any type issues when typed accordingly with the ref and the props.

Implementation can be something like below:

"use client";

import React from "react";
import Link from "next/link";
import routerProviderBase from "@refinedev/nextjs-router";

import type { RouterProvider } from "@refinedev/core";
import type { Url } from "next/dist/shared/lib/router/router";

type Destination = { to: string; href: never } | { href: Url; to?: string };

type LinkProps = React.ComponentProps<NonNullable<RouterProvider["Link"]>> &
  Destination;

const MyLink: RouterProvider["Link"] = React.forwardRef<
  HTMLAnchorElement,
  LinkProps
>(({ to, children, href, ...props }, ref) => {
  // any custom requirements can be handled here and will be shared with all `<Link />` usages.
  return <Link href={href ?? to} {...props} ref={ref} />;
});

export const routerProvider: RouterProvider = {
  ...routerProviderBase,
  Link: MyLink,
};

Then for the usage in custom made pages, <Link /> component from @refinedev/core can be used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants