From 4e140370a84aec3f3a2faaed18519db4d6fb3e86 Mon Sep 17 00:00:00 2001 From: Hendrik Liebau Date: Tue, 17 Sep 2024 13:37:26 +0200 Subject: [PATCH] Fix regression in TS plugin: allow `reset` prop in error files (#69777) The TypeScript error `Props must be serializable for components in the "use client" entry file, "reset" is invalid.` for the `reset` prop in `error.tsx` and `global-error.tsx` was fixed in #46898 and #48756, respectively. However, a regression of the fix was accidentally introduced in #67211. This PR restores the previous behavior and adds fixtures for manual testing. (Since the Next.js TS plugin only applies to VSCode, manual testing in VSCode is required. See [`test/development/typescript-plugin/README.md`](https://github.com/vercel/next.js/blob/a5f30e68f8d1fe614965abc5df0234a869c6db8a/test/development/typescript-plugin/README.md) for details.) --- .../typescript/rules/client-boundary.ts | 40 +++++++++++-------- test/development/typescript-plugin/README.md | 3 ++ .../typescript-plugin/app/client.tsx | 12 +++++- .../typescript-plugin/app/error.tsx | 23 +++++++++++ .../typescript-plugin/app/global-error.tsx | 19 +++++++++ 5 files changed, 78 insertions(+), 19 deletions(-) create mode 100644 test/development/typescript-plugin/app/error.tsx create mode 100644 test/development/typescript-plugin/app/global-error.tsx diff --git a/packages/next/src/server/typescript/rules/client-boundary.ts b/packages/next/src/server/typescript/rules/client-boundary.ts index 9ed1616a88ac9..5a6c2a3773e58 100644 --- a/packages/next/src/server/typescript/rules/client-boundary.ts +++ b/packages/next/src/server/typescript/rules/client-boundary.ts @@ -52,12 +52,23 @@ const clientBoundary = { if (typeDeclarationNode) { if (ts.isFunctionTypeNode(typeDeclarationNode)) { - // By convention, props named "action" can accept functions since we assume these are Server Actions. - // Structurally, there's no difference between a Server Action and a normal function until TypeScript exposes directives in the type of a function. - // This will miss accidentally passing normal functions but a false negative is better than a false positive given how frequent the false-positive would be. + // By convention, props named "action" can accept functions since we + // assume these are Server Actions. Structurally, there's no + // difference between a Server Action and a normal function until + // TypeScript exposes directives in the type of a function. This + // will miss accidentally passing normal functions but a false + // negative is better than a false positive given how frequent the + // false-positive would be. const maybeServerAction = propName === 'action' || /.+Action$/.test(propName) - if (!maybeServerAction) { + + // There's a special case for the error file that the `reset` prop + // is allowed to be a function: + // https://github.com/vercel/next.js/issues/46573 + const isErrorReset = + (isErrorFile || isGlobalErrorFile) && propName === 'reset' + + if (!maybeServerAction && !isErrorReset) { diagnostics.push({ file: source, category: ts.DiagnosticCategory.Warning, @@ -75,19 +86,14 @@ const clientBoundary = { ts.isConstructorTypeNode(typeDeclarationNode) || ts.isClassDeclaration(typeDeclarationNode) ) { - // There's a special case for the error file that the `reset` prop is allowed - // to be a function: - // https://github.com/vercel/next.js/issues/46573 - if (!(isErrorFile || isGlobalErrorFile) || propName !== 'reset') { - diagnostics.push({ - file: source, - category: ts.DiagnosticCategory.Warning, - code: NEXT_TS_ERRORS.INVALID_CLIENT_ENTRY_PROP, - messageText: `Props must be serializable for components in the "use client" entry file, "${propName}" is invalid.`, - start: prop.getStart(), - length: prop.getWidth(), - }) - } + diagnostics.push({ + file: source, + category: ts.DiagnosticCategory.Warning, + code: NEXT_TS_ERRORS.INVALID_CLIENT_ENTRY_PROP, + messageText: `Props must be serializable for components in the "use client" entry file, "${propName}" is invalid.`, + start: prop.getStart(), + length: prop.getWidth(), + }) } } } diff --git a/test/development/typescript-plugin/README.md b/test/development/typescript-plugin/README.md index 022a0bf3709e8..36a2b4ace2389 100644 --- a/test/development/typescript-plugin/README.md +++ b/test/development/typescript-plugin/README.md @@ -15,3 +15,6 @@ The plugin only applies to VSCode so manual testing in VSCode is required. `app/client.tsx#ClientComponent` has props that can and can't be serialized. Ensure the current comments still describe the observed behavior. + +`app/error.tsx#Error` and `app/global-error.tsx#GlobalError` have a `reset` prop +that should be excluded from the serialization check. diff --git a/test/development/typescript-plugin/app/client.tsx b/test/development/typescript-plugin/app/client.tsx index 13b5f542ac9d6..b4969313507e6 100644 --- a/test/development/typescript-plugin/app/client.tsx +++ b/test/development/typescript-plugin/app/client.tsx @@ -1,14 +1,22 @@ 'use client' +class MyClass {} + export function ClientComponent({ unknownAction, //^^^^^^^^^^^ fine because it looks like an action unknown, - //^^^^^ "Error(TS71007): Props must be serializable for components in the "use client" entry file. "unknown" is a function that's not a Server Action. Rename "unknown" either to "action" or have its name end with "Action" e.g. "unknownAction" to indicate it is a Server Action.ts(71007) + //^^^^^ "Error(TS71007): Props must be serializable for components in the "use client" entry file. "unknown" is a function that's not a Server Action. Rename "unknown" either to "action" or have its name end with "Action" e.g. "unknownAction" to indicate it is a Server Action. ts(71007) + foo, + //^ "Error(TS71007): Props must be serializable for components in the "use client" entry file, "foo" is invalid.ts(71007) + bar, + //^ "Error(TS71007): Props must be serializable for components in the "use client" entry file, "bar" is invalid.ts(71007) }: { unknownAction: () => void unknown: () => void + foo: new () => Error + bar: MyClass }) { - console.log({ unknown, unknownAction }) + console.log({ unknown, unknownAction, foo, bar }) return null } diff --git a/test/development/typescript-plugin/app/error.tsx b/test/development/typescript-plugin/app/error.tsx new file mode 100644 index 0000000000000..536b1161e79e6 --- /dev/null +++ b/test/development/typescript-plugin/app/error.tsx @@ -0,0 +1,23 @@ +'use client' + +import { useEffect } from 'react' + +export default function Error({ + error, + reset, + //^^^ fine because it's the special reset prop in an error file +}: { + error: Error & { digest?: string } + reset: () => void +}) { + useEffect(() => { + console.error(error) + }, [error]) + + return ( +
+

Something went wrong!

+ +
+ ) +} diff --git a/test/development/typescript-plugin/app/global-error.tsx b/test/development/typescript-plugin/app/global-error.tsx new file mode 100644 index 0000000000000..2852c1f02047d --- /dev/null +++ b/test/development/typescript-plugin/app/global-error.tsx @@ -0,0 +1,19 @@ +'use client' + +export default function GlobalError({ + error, + reset, + //^^^ fine because it's the special reset prop in a global-error file +}: { + error: Error & { digest?: string } + reset: () => void +}) { + return ( + + +

Something went wrong!

+ + + + ) +}