feat(router): Better autocomplete for <Set>s

[Tobbe]

This is a continuation of #11756 to provide autocomplete for the wrapper props

There's a limit to 10 wrapper components. After that additional props will not be included. To work around this you can either make sure to put wrapper components that doesn't require any props last, or just split your wrappers between multiple <Set>s

[Philzen]

I'm trying this in VSCode … still does not solve 3. and 9. in our text case matrix for me :(

But i see that 9. is actually working for the very first element in the array (in current main), which is presumably why you came up with this current solution of yours...

[Tobbe]

Thanks for testing this @Philzen. Now, can you please try three more things for me?

Try these one at a time

1. Add another property to TitleLayoutProps with a unique name (like numProp: number)
2. Enable strict mode ("strict": true) in web/tsconfig.json
3. Help TS out a bit with the types by doing <Set<[typeof TitleLayout, typeof ScaffoldLayout]> ...>

Did any of the above changes fix the types for you?

[Philzen]

But i see that 9. is actually working for the very first element in the array (in current main), which is presumably why you came up with this current solution of yours...

Oh boy, i just found a really important detail. It does matter whether the props are typed with type or interface 😟
It will only issue a TS error if at least the 2nd, 3rd, … component are typed via interfaces, not with type. (still only for the first wrapper's props though) .

That is something to keep in mind while debugging … interfaces can be extended in-place, while types need to extend via intersection: https://www.typescriptlang.org/docs/handbook/2/everyday-types.html#differences-between-type-aliases-and-interfaces

Note that the generated ScaffoldLayout uses a type while the components in your tests (such as TitleLayout) use interfaces ❕

Now, can you please try three more things for me?

On it.

[Philzen]

1. Add another property to `TitleLayoutProps` with a unique name (like `numProp: number`)

Interesting 🤔 … i have wrap={[ScaffoldLayout, TitleLayout]}, before it gave me a warning about the missing required props of ScaffoldLayout, now when i add a prop it will give me a warning about the missing TitleLayout props.

However in both cases it stops complaining as soon as i have filled in one components' required props, if won't care which.

2. Enable strict mode (`"strict": true`) in `web/tsconfig.json`

Cannot see any difference.

3. Help TS out a bit with the types by doing `<Set<[typeof TitleLayout, typeof ScaffoldLayout]> ...>`

No help.

<Set<[typeof TitleLayout, typeof ScaffoldLayout]>… gives squiggles on the wrap components.
Both <Set<typeof TitleLayout & typeof ScaffoldLayout> and <Set<typeof TitleLayout | typeof ScaffoldLayout> give squiggles on the wrap-prop.

[Tobbe]

That's different compared to what I'm seeing.

This is what I'm working with

import { Router, Route, Set } from '@redwoodjs/router'

import ScaffoldLayout from 'src/layouts/ScaffoldLayout'

interface TitleLayoutProps {
  children: React.ReactNode
  title: string
}

const TitleLayout = ({ children }: TitleLayoutProps) => {
  return <>{children}</>
}

const Routes = () => {
  return (
    <Router>
      <Set wrap={[TitleLayout, ScaffoldLayout]} title="UserExamples" titleTo="home" buttonLabel="New UserExample" buttonTo="home">
        <Route path="/user-examples/new" page={UserExampleNewUserExamplePage} name="newUserExample" />
        <Route path="/user-examples/{id:Int}/edit" page={UserExampleEditUserExamplePage} name="editUserExample" />
        <Route path="/user-examples/{id:Int}" page={UserExampleUserExamplePage} name="userExample" />
        <Route path="/user-examples" page={UserExampleUserExamplesPage} name="userExamples" />
      </Set>
      <Route notfound page={NotFoundPage} />
    </Router>
  )
}

export default Routes

And by default I get this

1. Unique Prop on TitleLayout

2. TypeScript Strict Mode

(Had to restart the TS server for this to work)

3. Explicit Generics

(Switched back to non-strict TS before running this experiment)

As you can see all those things fix the issue for me.
And as you can maybe also see, I did my experiments in VSCode this time, just to rule out IDE differences.

[Tobbe]

And, as a bonus, with explicit generics you also get type errors if you don't include required props, like numProp in this case

[Philzen]

After a restart i cannot reproduce what went wrong there on my side. I can confirm what you are seeing now. Looks good.

[Philzen]

@Tobbe did you test that this doesn't introduce a regression for #11739?

If i try out this PR, i get TS errors and no autocompletion for any props on <PrivateSet /, so it's back to square one.

https://raw.githubusercontent.com/redwoodjs/redwood/6cfc992c35a06307bc703c402296b86ae58873f4/packages/router/src/Set.tsx currently is still the version that works best for me, but maybe you can fix PrivateSet for this PR?

[Tobbe]

maybe you can fix PrivateSet for this PR?

I just made a totally separate type for private sets (for now). We can come back and iterate later.

6cfc992/packages/router/src/Set.tsx (raw) currently is still the version that works best for me

Does this still hold true now when <PrivateSet> is fixed?

[Philzen]

Does this still hold true now when <PrivateSet> is fixed?

No, this version works for me now 😺

Check out my review comment – extracting that complex type may save some duplication and makes the TS evaluation faster.

Otherwise i'm completely OK with this PR, cheers.

[Tobbe]

Check out my review comment – extracting that complex type may save some duplication and makes the TS evaluation faster.

Did you forget to publish it? I don't see any review comments

[Philzen]

As a side-note: it is recommended to extract this into a separate type as per https://github.com/microsoft/TypeScript/wiki/Performance#naming-complex-types

[Tobbe]

I tried extracting this. But when I do TS just always seems to resolve it to unknown.

But I did some other changes, so now at least all of the Set type isn't duplicated.

[Philzen]

Did you forget to publish it? I don't see any review comments

Apparently 🤦 😆

[Tobbe]

@Philzen Feel free to have a swing at extracting that big complicated type if you want