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

[Bug]: type errors with exactOptionalPropertyTypes #11991

Closed
Tracked by #1622
marekdedic opened this issue Sep 12, 2024 · 4 comments
Closed
Tracked by #1622

[Bug]: type errors with exactOptionalPropertyTypes #11991

marekdedic opened this issue Sep 12, 2024 · 4 comments
Labels

Comments

@marekdedic
Copy link

What version of React Router are you using?

6.26.2

Steps to Reproduce

  1. Install react-router
  2. Enable exactOptionalPropertyTypes in your tsconfig
  3. Compile

Expected Behavior

No errors

Actual Behavior

node_modules/react-router/dist/index.d.ts:17:116 - error TS2344: Type 'RouteMatch<string, RouteObject>' does not satisfy the constraint 'AgnosticRouteMatch<string, AgnosticRouteObject>'.
  Types of property 'route' are incompatible.
    Type 'RouteObject' is not assignable to type 'AgnosticRouteObject' with 'exactOptionalPropertyTypes: true'. Consider adding 'undefined' to the types of the target's properties.

17 export interface unstable_PatchRoutesOnNavigationFunction extends unstable_AgnosticPatchRoutesOnNavigationFunction<RouteMatch> {
                                                                                                                      ~~~~~~~~~~

node_modules/react-router/dist/lib/components.d.ts:60:30 - error TS2344: Type 'NonIndexRouteObject' does not satisfy the constraint 'AgnosticRouteObject'.
  Type 'NonIndexRouteObject' is not assignable to type 'AgnosticNonIndexRouteObject' with 'exactOptionalPropertyTypes: true'. Consider adding 'undefined' to the types of the target's properties.
    Type 'NonIndexRouteObject' is not assignable to type 'AgnosticBaseRouteObject' with 'exactOptionalPropertyTypes: true'. Consider adding 'undefined' to the types of the target's properties.
      Types of property 'caseSensitive' are incompatible.
        Type 'boolean | undefined' is not assignable to type 'boolean'.

60     lazy?: LazyRouteFunction<NonIndexRouteObject>;
                                ~~~~~~~~~~~~~~~~~~~

node_modules/react-router/dist/lib/components.d.ts:81:30 - error TS2344: Type 'IndexRouteObject' does not satisfy the constraint 'AgnosticRouteObject'.

81     lazy?: LazyRouteFunction<IndexRouteObject>;
                                ~~~~~~~~~~~~~~~~

node_modules/react-router/dist/lib/context.d.ts:20:30 - error TS2344: Type 'RouteObject' does not satisfy the constraint 'AgnosticRouteObject'.
  Type 'IndexRouteObject' is not assignable to type 'AgnosticRouteObject' with 'exactOptionalPropertyTypes: true'. Consider adding 'undefined' to the types of the target's properties.
    Type 'IndexRouteObject' is not assignable to type 'AgnosticIndexRouteObject' with 'exactOptionalPropertyTypes: true'. Consider adding 'undefined' to the types of the target's properties.
      Type 'IndexRouteObject' is not assignable to type 'AgnosticBaseRouteObject' with 'exactOptionalPropertyTypes: true'. Consider adding 'undefined' to the types of the target's properties.
        Types of property 'caseSensitive' are incompatible.
          Type 'boolean | undefined' is not assignable to type 'boolean'.
            Type 'undefined' is not assignable to type 'boolean'.

20     lazy?: LazyRouteFunction<RouteObject>;
                                ~~~~~~~~~~~

node_modules/react-router/dist/lib/context.d.ts:39:30 - error TS2344: Type 'RouteObject' does not satisfy the constraint 'AgnosticRouteObject'.

39     lazy?: LazyRouteFunction<RouteObject>;
                                ~~~~~~~~~~~

node_modules/react-router/dist/lib/context.d.ts:46:151 - error TS2344: Type 'RouteObjectType' does not satisfy the constraint 'AgnosticRouteObject'.
  Type 'RouteObject' is not assignable to type 'AgnosticRouteObject' with 'exactOptionalPropertyTypes: true'. Consider adding 'undefined' to the types of the target's properties.
    Type 'RouteObjectType' is not assignable to type 'AgnosticNonIndexRouteObject' with 'exactOptionalPropertyTypes: true'. Consider adding 'undefined' to the types of the target's properties.
      Type 'RouteObject' is not assignable to type 'AgnosticNonIndexRouteObject' with 'exactOptionalPropertyTypes: true'. Consider adding 'undefined' to the types of the target's properties.
        Type 'IndexRouteObject' is not assignable to type 'AgnosticNonIndexRouteObject' with 'exactOptionalPropertyTypes: true'. Consider adding 'undefined' to the types of the target's properties.
          Type 'IndexRouteObject' is not assignable to type 'AgnosticBaseRouteObject' with 'exactOptionalPropertyTypes: true'. Consider adding 'undefined' to the types of the target's properties.
            Type 'RouteObjectType' is not assignable to type 'AgnosticBaseRouteObject' with 'exactOptionalPropertyTypes: true'. Consider adding 'undefined' to the types of the target's properties.
              Type 'RouteObject' is not assignable to type 'AgnosticBaseRouteObject' with 'exactOptionalPropertyTypes: true'. Consider adding 'undefined' to the types of the target's properties.
                Type 'IndexRouteObject' is not assignable to type 'AgnosticBaseRouteObject' with 'exactOptionalPropertyTypes: true'. Consider adding 'undefined' to the types of the target's properties.

46 export interface RouteMatch<ParamKey extends string = string, RouteObjectType extends RouteObject = RouteObject> extends AgnosticRouteMatch<ParamKey, RouteObjectType> {
                                                                                                                                                         ~~~~~~~~~~~~~~~

@timdorr
Copy link
Member

timdorr commented Sep 12, 2024

This isn't on us to implement. We are fine with undefined as a value for optional properties.

You should be using skipLibCheck on your own project, as you shouldn't be applying a compiler config our code isn't written for.

@timdorr timdorr closed this as not planned Won't fix, can't repro, duplicate, stale Sep 12, 2024
@marekdedic
Copy link
Author

marekdedic commented Sep 13, 2024

Hi,
thanks for the quick response. Let me push back on it somewhat :)

You should be using skipLibCheck on your own project, as you shouldn't be applying a compiler config our code isn't written for.

Your code is part of the source for my project, AFAIK, skipLibCheck is an escape hatch for incorrect and/or incompatible typings. I could very well use this escape hatch, but I'm trying to fix the typings instead :)

This isn't on us to implement. We are fine with undefined as a value for optional properties.

I think there is a misunderstanding between us. I don't want you to make the properties required, I would like to see the typings reformulated in such a way that they allow undefined explicitly. I tried looking into what this would take and it turns out the whole thing would consist of just modifying the following type to add | undefined to some of its properties:

type AgnosticBaseRouteObject = {
    caseSensitive?: boolean | undefined;
    path?: string | undefined;
    id?: string | undefined;
    loader?: LoaderFunction | boolean | undefined;
    action?: ActionFunction | boolean | undefined;
    hasErrorBoundary?: boolean | undefined;
    shouldRevalidate?: ShouldRevalidateFunction | undefined;
    handle?: any;
    lazy?: LazyRouteFunction<AgnosticBaseRouteObject>;
};

Happy to send in a PR if you agree.

@timdorr
Copy link
Member

timdorr commented Sep 13, 2024

You should be importing our built type definition from npm, not including the full source in your project. That's not something we support. We have validated those according to our own compiler config, so you don't have to validate them again. That is the main purpose of skipLibCheck, so you're not trying to validate types against an incompatible config.

And we would prefer not to have to be explicit about undefined. We are fine with the default behavior without exactOptionalPropertyTypes enabled.

@marekdedic
Copy link
Author

You should be importing our built type definition from npm, not including the full source in your project.

I am using the NPM package and the issue is present nonetheless.

We have validated those according to our own compiler config, so you don't have to validate them again. That is the main purpose of skipLibCheck, so you're not trying to validate types against an incompatible config.

The way I understand it and read it in the available resources, compiled lib type definitions are to be considered as part of the compilation. skipLibCheck exists s a hack around incorrect upstream type definitions.

And we would prefer not to have to be explicit about undefined. We are fine with the default behavior without exactOptionalPropertyTypes enabled.

Unfortunately, this means you are forcing all your downstream users to either forgo more precise optional property typing (which I get not everybody wants, but this way it's not possible for me), or to have to have the compatibility with libraries disabled.

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

No branches or pull requests

2 participants