Skip to content

Commit

Permalink
Fix regression in TS plugin: allow reset prop in error files (#69777)
Browse files Browse the repository at this point in the history
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.)
  • Loading branch information
unstubbable authored Sep 17, 2024
1 parent 3127e86 commit 4e14037
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 19 deletions.
40 changes: 23 additions & 17 deletions packages/next/src/server/typescript/rules/client-boundary.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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(),
})
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions test/development/typescript-plugin/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
12 changes: 10 additions & 2 deletions test/development/typescript-plugin/app/client.tsx
Original file line number Diff line number Diff line change
@@ -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
}
23 changes: 23 additions & 0 deletions test/development/typescript-plugin/app/error.tsx
Original file line number Diff line number Diff line change
@@ -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 (
<div>
<h2>Something went wrong!</h2>
<button onClick={() => reset()}>Try again</button>
</div>
)
}
19 changes: 19 additions & 0 deletions test/development/typescript-plugin/app/global-error.tsx
Original file line number Diff line number Diff line change
@@ -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 (
<html>
<body>
<h2>Something went wrong!</h2>
<button onClick={() => reset()}>Try again</button>
</body>
</html>
)
}

0 comments on commit 4e14037

Please sign in to comment.