fix: remove NonNullable to fix recursive infer in PathConfigMap

[johankasperi]

Motivation

Fixes #10876 and #9897

Test plan

This change breaks typescript in the example app because the linking config does not match how all the navigators is setup. This wasn't catched before because of this bug in the PathConfigMap type. For example we are configuring a link to the Chat screen inside NativeStack (with the path "native-stack/chat") but the Chat route isn't configured in the NativeStack navigator: https://github.com/react-navigation/react-navigation/blob/main/example/src/Screens/NativeStack.tsx#L15

I will happily fix this but wanted some feedback on the actual change to PathConfigMap first before putting in the hour(s) to fix the example project.

Also I would like some feedback on how we would like to change the example project. Should all the routes "Article", "Albums", "Chat", "Contacts", "NewsFeed" and "Dialog" exist in every navigator? Or should just the linking config match how each individual navigator is setup? This would probably mean that we cannot reduce all SCREEN_NAMES to create the config because all navigators have different param lists.

I also have a fix for the 6.x branch: https://github.com/react-navigation/react-navigation/compare/6.x...johankasperi:react-navigation:fix-pathconfig-recursive-infer?expand=1

[netlify]

✅ Deploy Preview for react-navigation-example ready!

Name	Link
🔨 Latest commit	e1249ea
🔍 Latest deploy log	https://app.netlify.com/sites/react-navigation-example/deploys/662bfd5412f50300088f6adb
😎 Deploy Preview	https://deploy-preview-11430--react-navigation-example.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[pagkt2]

I also fixed this for myself by removing NonNullable, although I still left the extends {}

[stereodenis]

Not working for me...

[osdnk]

Hi @johankasperi,

Thanks for the PR!

Can you please rebase and fix the CI? Also, can you add tests (use __typechecks__ folder).

[github-actions]

Hello 👋, this pull request has been open for more than a month with no activity on it. If you think this is still necessary with the latest version, please comment and ping a maintainer to get this reviewed, otherwise it will be closed automatically in 7 days.

[satya164]

Why did you change this? Please revert to how it was

[johankasperi]

Could you please help me with how linking options should be defined here if the Home route is NavigationScreenParams<RootDrawerParamList> | undefined. I'm getting this typecheck error when it can be undefined:

Object literal may only specify known properties, and 'screens' does not exist in type 'Omit<PathConfig<{}>, "initialRouteName" | "screens">'

on this row

react-navigation/example/src/index.tsx

Line 69 in d0abdee

screens: {

I have no experience from using NavigatorScreenParams in a union with undefined and really no idea how it should be handled by the lib. Would really appreciate some guidance. Thanks!

[johankasperi]

We are omitting screens from the type if it's not extending NavigatorScreenParams here: https://github.com/react-navigation/react-navigation/blob/d0abdee67f5db8cf39112af535846ffededfb21d/packages/core/src/types.tsx#L1012C37-L1012C46

Since this PR removes NonNullable from that line (because NonNullable broke recursive infer) I think that makes it impossible for a route to be union of NavigatorScreenParams and undefined?

[satya164]

If it makes it impossible to have undefined then it'd be a breaking change. The change needs to support | undefined to be merged

[johankasperi]

Ok, but if a route is defined as NavigatorScreenParams<{}> | undefined should you be able to config linking options for the nested navigator as in the example?

[satya164]

It wouldn't make sense to support | undefined if it only works with some of the things and not others.

[johankasperi]

I checked the main branch of the lib and from my testing the original

Hi @johankasperi,

Thanks for the PR!

Can you please rebase and fix the CI? Also, can you add tests (use __typechecks__ folder).

Thank you! I added a test in common.check.tsx. Please let me know if it was something like that you had in mind or if we should test it in some other way.

[ChrisYohann]

HI @johankasperi , did you manage to effectively resolve #9897 with your fix ? I copy pasted your changes, and it did not fix anything.
The issues says that it's not possible to type a screen that has its own parameters and a nested navigator together.

I think, as soon as you define your type like this (from the example) :

export type ParamList = {
  ScreenA: undefined
  ScreenB: NavigatorScreenParams<ParamListInnerNavigator> & {
    someparam?: number,
  }
}

Assuming PathConfigMap is defined as follows :

export type PathConfigMap<ParamList = {}> = {
  [RouteName in keyof ParamList]?: ParamList[RouteName] extends NavigatorScreenParams<
    infer T
  >
    ? string | PathConfig<T>
    : string | Omit<PathConfig<{}>, 'screens' | 'initialRouteName'>;
};

This check :

ParamList[RouteName] extends NavigatorScreenParams<
    infer T
  >
    ? string | PathConfig<T>
    : string | Omit<PathConfig<{}>, 'screens' | 'initialRouteName'>;

resolves to false for ParamList['ScreenB'] therefore the config is not valid :
I think the solution should be to enhance NavigatorScreenParams generic type to allow passing params to the screen
like :

export type NavigatorScreenParams<
  ParamList,
  State extends NavigationState = NavigationState,
  OwnScreenParams extends {} = {},
> = OwnScreenParams &
  (
    | {
        screen?: never;
        params?: never;
        initial?: never;
        path?: string;
        state: PartialState<State> | State | undefined;
      }
    | {
        [RouteName in keyof ParamList]: undefined extends ParamList[RouteName]
          ? {
              screen: RouteName;
              params?: ParamList[RouteName];
              initial?: boolean;
              path?: string;
              state?: never;
            }
          : {
              screen: RouteName;
              params: ParamList[RouteName];
              initial?: boolean;
              path?: string;
              state?: never;
            };
      }[keyof ParamList]
  );

What do you think ?

Thanks for your help !