[Router] Fix TS error when passing WrapperProps to <PrivateSet>s

[Philzen]

Redwood v6.4 deprecated the unauthorized prop on <Set>s so during that upgrade we migrated to <PrivateSet> as per the release notes.

Since then any props forwarded to a wrap={MyWrapper} component show up as TypeScript errors. While hovering over the wrap prop of a <Set> correctly infers WrapperType<MyWrapper>, hovering over the wrap prop of a <PrivateSet> gives WrapperType<unknown>

I'm a bit surprised to see that TypeScript's Omit drops the props derived via the generic from Set here 🤯 … could not find anything explaining that behavior in the docs, so would be glad to learn why that happens (otherwise feels a bit like a typescript bug?).

Simply adding them again as an intersection types does the job and gets rid of the typescript error (e738350) 🚀

[Philzen]

70f4450 enables the pretty helpful TSDOC info on the wrap prop while also reducing duplication.

[Philzen]

Would be cool to have this in v8.4.2 🙏

[Philzen]

BTW, the same result as above two commits could also be achieved a little simpler:

export declare function Set<WrapperProps>(props: SetProps<WrapperProps>): React.JSX.Element;
type PrivateSetProps = {
    /** The page name where a user will be redirected when not authenticated */
    unauthenticated: string;
};
/** @deprecated Please use `<PrivateSet>` instead */
export declare function Private<WrapperProps>(props: SetProps<WrapperProps> | PrivateSetProps): React.JSX.Element;
export declare function PrivateSet<WrapperProps>(props: SetProps<WrapperProps> | PrivateSetProps): React.JSX.Element;

… but this may not communicate the intent of the authors as clearly.

[Tobbe]

I've confirmed the issue, and verified the fix.

Fixing this seems to have brought up another issue though

Types of parameters '__0' and 'props' are incompatible.
        Type 'Omit<{ children: Element[]; unauthenticated: string; wrap: ({ title, titleTo, buttonLabel, buttonTo, children, }: LayoutProps) => Element; title: string; titleTo: string; buttonLabel: string; buttonTo: string; }, "children" | "wrap"> & { ...; }' is not assignable to type 'LayoutProps'.
          Types of property 'titleTo' are incompatible.
            Type 'string' is not assignable to type 'keyof AvailableRoutes'.

And if you look inside <ScaffoldLayout> it has this:

type LayoutProps = {
  title: string
  titleTo: keyof typeof routes
  buttonLabel: string
  buttonTo: keyof typeof routes
  children: React.ReactNode
}

const ScaffoldLayout = ({
  title,
  titleTo,
  buttonLabel,
  buttonTo,
  children,
}: LayoutProps) => {

Where keyof typeof routes === keyof AvailableRoutes.

To fix that I have to either declare a separate variable with the expected type, or do a type cast. Like this:

  const userExamplesName: RouteName = 'userExamples'

  return (
    <Router>
      <PrivateSet unauthenticated="home" wrap={ScaffoldLayout} title="UserExamples" titleTo={userExamplesName} buttonLabel="New UserExample" buttonTo={'newUserExample' as RouteName}>
        <Route path="/user-examples/new" page={UserExampleNewUserExamplePage} name="newUserExample" />

@Philzen Are you also seeing this issue? Or is it just something on my side?

[Philzen]

@Philzen Are you also seeing this issue? Or is it just something on my side?

I'm not sure i have that kind of complex layout file to test at hand atm. However, just as a double check that the root cause lies in this PR, does this also happen when you use a Set instead of a PrivateSet?

[Tobbe]

does this also happen when you use a Set instead of a PrivateSet?

Yes, it does. So let's ignore this issue for this PR.

(Honestly I'm pretty sure it was a misstake to add the ability to spread props onto <Set> like this. Together with the ability to have multiple wrapper components there's no way to make this fully typesafe 😞)

[Tobbe]

I'm not sure i have that kind of complex layout file to test at hand atm.

Not expecting you to do this, but if you wanted to test all you have to do is use the scaffold generator for any model and you should see the problem yarn rw g scaffold modelName

[Philzen]

Thx for the hint, i could reproduce it.

One really peculiar finding: Try Set instead and add an unauthenticated="login" prop – the same TS error will appear. 🤯 🤯 🤯
Vice versa remove the unauthenticated prop from your PrivateSet example, and the TS error on wrap disappears. So that narrows down the culprit … but why that happens frankly goes over the top of my head here.

[Tobbe]

Try Set instead and add an unauthenticated="login" prop – the same TS error will appear.

"same TS error" as what? When I add `unauthenticated I get this:

And if I do PrivateSet without unauthenticated I get this

[Philzen]

Try Set instead and add an unauthenticated="login" prop – the same TS error will appear.

"same TS error" as what?

The same TS error that you were reporting, reproducible on a <Set /> as well:

if I do PrivateSet without unauthenticated I get this

Yes of course, as unauthenticated is required here. But as i can see on your screenshot, the TS error on wrap disappears.

[Tobbe]

Gotcha. On PrivateSet I think TS just gives up when it doesn't find the unauthenticated prop 🤷

[Philzen]

Actually, the TS error you're seeing is triggered by any prop that is added. Try roles instead of unauthenticated on a Set:

So at least that proves that this PR has not introduced a new bug. 🤷

[Tobbe]

I'm seeing it without anything extra added at all

So at least that proves that this PR has not introduced a new bug. 🤷

I'd say so, yeah