Aug 29 '24 03:08 kshramt

https://www.typescriptlang.org/tsconfig/#exactOptionalPropertyTypes

Aug 29 '24 03:08 SeanCassiere

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 7d892ac8ef96a46995b4fff31481f1cc2b6e927a. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

✅ Successfully ran 2 targets

Sent with 💌 from NxCloud.

Aug 29 '24 03:08 nx-cloud[bot]

Open in Stackblitz

More templates

@tanstack/create-router

pnpm add https://pkg.pr.new/@tanstack/create-router@2213

@tanstack/eslint-plugin-router

pnpm add https://pkg.pr.new/@tanstack/eslint-plugin-router@2213

@tanstack/history

pnpm add https://pkg.pr.new/@tanstack/history@2213

@tanstack/react-cross-context

pnpm add https://pkg.pr.new/@tanstack/react-cross-context@2213

@tanstack/react-router-with-query

pnpm add https://pkg.pr.new/@tanstack/react-router-with-query@2213

@tanstack/react-router

pnpm add https://pkg.pr.new/@tanstack/react-router@2213

@tanstack/router-cli

pnpm add https://pkg.pr.new/@tanstack/router-cli@2213

@tanstack/router-arktype-adapter

pnpm add https://pkg.pr.new/@tanstack/router-arktype-adapter@2213

@tanstack/router-generator

pnpm add https://pkg.pr.new/@tanstack/router-generator@2213

@tanstack/router-devtools

pnpm add https://pkg.pr.new/@tanstack/router-devtools@2213

@tanstack/router-plugin

pnpm add https://pkg.pr.new/@tanstack/router-plugin@2213

@tanstack/router-valibot-adapter

pnpm add https://pkg.pr.new/@tanstack/router-valibot-adapter@2213

@tanstack/router-vite-plugin

pnpm add https://pkg.pr.new/@tanstack/router-vite-plugin@2213

@tanstack/router-zod-adapter

pnpm add https://pkg.pr.new/@tanstack/router-zod-adapter@2213

@tanstack/start

pnpm add https://pkg.pr.new/@tanstack/start@2213

@tanstack/start-vite-plugin

pnpm add https://pkg.pr.new/@tanstack/start-vite-plugin@2213

@tanstack/virtual-file-routes

pnpm add https://pkg.pr.new/@tanstack/virtual-file-routes@2213

commit: 7d892ac

Aug 29 '24 03:08 pkg-pr-new[bot]

CI failures addressed and changes ( https://github.com/TanStack/router/compare/fca21d38300ebdc467fe0a2a218c467314ce1522..3dab3613c557c09f5a76447a01e41af568a2b201 ) force pushed.

Aug 29 '24 03:08 kshramt

do we have runtime tests and type tests for all those situations where you can pass in an explicit undefined?

Sep 20 '24 19:09 schiller-manuel

To clarify, this PR doesn't alter the type definition, but rather makes the existing behavior more explicit (see https://www.typescriptlang.org/tsconfig/#exactOptionalPropertyTypes ).

Sep 21 '24 07:09 kshramt

thanks for clarification. can you please add the exactOptionalPropertyTypesflag to the root tsconfig.json so we catch this with our type tests?

Sep 21 '24 08:09 schiller-manuel

Setting "exactOptionalPropertyTypes": true in the root tsconfig.json will result in many type-check errors like this:

src/CatchBoundary.tsx:14:6 - error TS2769: No overload matches this call.
  Overload 1 of 2, '(props: { getResetKey: () => string | number; children: (props: { error: Error | null; reset: () => void; }) => ReactNode; onCatch?: (error: Error, errorInfo: ErrorInfo) => void; }): CatchBoundaryImpl', gave the following error.
    Type '{ getResetKey: () => string | number; onCatch: ((error: Error, errorInfo: ErrorInfo) => void) | undefined; children: ({ error, reset }: { error: Error | null; reset: () => void; }) => string | ... 6 more ... | undefined; }' is not assignable to type 'Readonly<{ getResetKey: () => string | number; children: (props: { error: Error | null; reset: () => void; }) => ReactNode; onCatch?: (error: Error, errorInfo: ErrorInfo) => void; }>' with 'exactOptionalPropertyTypes: true'. Consider adding 'undefined' to the types of the target's properties.
      Types of property 'onCatch' are incompatible.
        Type '((error: Error, errorInfo: ErrorInfo) => void) | undefined' is not assignable to type '(error: Error, errorInfo: ErrorInfo) => void'.
          Type 'undefined' is not assignable to type '(error: Error, errorInfo: ErrorInfo) => void'.
  Overload 2 of 2, '(props: { getResetKey: () => string | number; children: (props: { error: Error | null; reset: () => void; }) => ReactNode; onCatch?: (error: Error, errorInfo: ErrorInfo) => void; }, context: any): CatchBoundaryImpl', gave the following error.
    Type '{ getResetKey: () => string | number; onCatch: ((error: Error, errorInfo: ErrorInfo) => void) | undefined; children: ({ error, reset }: { error: Error | null; reset: () => void; }) => string | ... 6 more ... | undefined; }' is not assignable to type 'Readonly<{ getResetKey: () => string | number; children: (props: { error: Error | null; reset: () => void; }) => ReactNode; onCatch?: (error: Error, errorInfo: ErrorInfo) => void; }>' with 'exactOptionalPropertyTypes: true'. Consider adding 'undefined' to the types of the target's properties.
      Types of property 'onCatch' are incompatible.
        Type '((error: Error, errorInfo: ErrorInfo) => void) | undefined' is not assignable to type '(error: Error, errorInfo: ErrorInfo) => void'.
          Type 'undefined' is not assignable to type '(error: Error, errorInfo: ErrorInfo) => void'.

14     <CatchBoundaryImpl

There are many places outside of link.tsx that implicitly assume x?: T allows undefined, making them incompatible with this flag.

Sep 21 '24 11:09 kshramt

would it make sense to fix all of those issues?

Sep 21 '24 11:09 schiller-manuel

WIP: I am trying to add the flag to packages/react-router/tsconfig.json (changing the root tsconfig.json seems a bit involved). It seems that https://github.com/TanStack/router/blob/f36f92b459bf0aabbc4d49c1e72c8b2ad6894622/packages/react-router/src/router.ts#L707-L710 may not work well with explicit undefineds, as {...{transformer: 1}, ...{transformer: undefined}} results in {transformer: undefined}.

Sep 21 '24 16:09 kshramt

Sorry, as enabling exactOptionalPropertyTypes turned out to be more complex than I expected, I have rolled back the branch to https://github.com/TanStack/router/pull/2213/commits/7d892ac8ef96a46995b4fff31481f1cc2b6e927a . Since I've already implemented a workaround without this PR, please feel free to close it, although I believe the changes, while not comprehensive, could still be useful in some cases.

Sep 22 '24 06:09 kshramt

Closing this as there's no movement in the requirement for exactOptionalPropertyTypes.

Oct 18 '24 05:10 SeanCassiere

refactor(react-router): support the `exactOptionalPropertyTypes` ts option in `<Link>`

☁️ Nx Cloud Report