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

Fix regression in TS plugin: allow reset prop in error files #69777

Merged
merged 1 commit into from
Sep 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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') {
unstubbable marked this conversation as resolved.
Show resolved Hide resolved
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>
)
}
Loading