Improve RSC plugin to provide better errors (vercel#42435)

This PR improves the RSC plugin for SWC to throw an error when the `"use
client"` directive is in the top level, but not before other statements
/ expressions. For example:

Code:

```js
import 'react'

'use client'
```

Error:

```
The "use client" directive must be placed before other expressions. Move it to the top of the file to resolve this issue.

   ,----
 3 | 'use client'
   : ^^^^^^^^^^^^
   `----
```

## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have a helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the
feature request has been accepted for implementation before opening a
PR.
- [ ] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have a helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `pnpm build && pnpm lint`
- [ ] The "examples guidelines" are followed from [our contributing
doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md)

Co-authored-by: JJ Kasper <jj@jjsweb.site>

.eslintignore

@@ -29,6 +29,7 @@ test/integration/eslint/**
 test/integration/script-loader/**/*
 test/development/basic/legacy-decorators/**/*
 test/production/emit-decorator-metadata/**/*.js
+test/e2e/app-dir/rsc-errors/app/swc/use-client/page.js
 test-timings.json
 packages/next-swc/crates/**
 bench/nested-deps/pages/**

packages/next-swc/crates/core/src/react_server_components.rs

@@ -84,33 +84,60 @@ impl<C: Comments> ReactServerComponents<C> {
         let _ = &module.body.retain(|item| {
             match item {
                 ModuleItem::Stmt(stmt) => {
-                    if !finished_directives {
-                        if !stmt.is_expr() {
-                            // Not an expression.
-                            finished_directives = true;
-                        }
+                    if !stmt.is_expr() {
+                        // Not an expression.
+                        finished_directives = true;
+                    }
 
-                        match stmt.as_expr() {
-                            Some(expr_stmt) => {
-                                match &*expr_stmt.expr {
-                                    Expr::Lit(Lit::Str(Str { value, .. })) => {
-                                        if &**value == "use client" {
+                    match stmt.as_expr() {
+                        Some(expr_stmt) => {
+                            match &*expr_stmt.expr {
+                                Expr::Lit(Lit::Str(Str { value, .. })) => {
+                                    if &**value == "use client" {
+                                        if !finished_directives {
                                             is_client_entry = true;
-
-                                            // Remove the directive.
-                                            return false;
+                                        } else {
+                                            HANDLER.with(|handler| {
+                                                handler
+                                                    .struct_span_err(
+                                                        expr_stmt.span,
+                                                        "NEXT_RSC_ERR_CLIENT_DIRECTIVE",
+                                                    )
+                                                    .emit()
+                                            })
                                         }
+
+                                        // Remove the directive.
+                                        return false;
                                     }
-                                    _ => {
-                                        // Other expression types.
-                                        finished_directives = true;
+                                }
+                                // Match `ParenthesisExpression` which is some formartting tools
+                                // usually do: ('use client'). In these case we need to throw
+                                // an exception because they are not valid directives.
+                                Expr::Paren(ParenExpr { expr, .. }) => {
+                                    finished_directives = true;
+                                    if let Expr::Lit(Lit::Str(Str { value, .. })) = &**expr {
+                                        if &**value == "use client" {
+                                            HANDLER.with(|handler| {
+                                                handler
+                                                    .struct_span_err(
+                                                        expr_stmt.span,
+                                                        "NEXT_RSC_ERR_CLIENT_DIRECTIVE_PAREN",
+                                                    )
+                                                    .emit()
+                                            })
+                                        }
                                     }
                                 }
+                                _ => {
+                                    // Other expression types.
+                                    finished_directives = true;
+                                }
                             }
-                            None => {
-                                // Not an expression.
-                                finished_directives = true;
-                            }
+                        }
+                        None => {
+                            // Not an expression.
+                            finished_directives = true;
                         }
                     }
                 }

packages/next-swc/crates/core/tests/errors/react-server-components/client-graph/use-client/input.js

@@ -0,0 +1,7 @@
+import "react"
+
+"use client"
+
+export default function () {
+  return null;
+}

packages/next-swc/crates/core/tests/errors/react-server-components/client-graph/use-client/output.js

@@ -0,0 +1,4 @@
+import "react";
+export default function () {
+  return null;
+}

packages/next-swc/crates/core/tests/errors/react-server-components/client-graph/use-client/output.stderr

@@ -0,0 +1,6 @@
+
+  x NEXT_RSC_ERR_CLIENT_DIRECTIVE
+   ,-[input.js:3:1]
+ 3 | "use client"
+   : ^^^^^^^^^^^^
+   `----

packages/next-swc/crates/core/tests/fixture/react-server-components/client-graph/client-entry/input.js

@@ -16,8 +16,6 @@
 
 import "fs"
 
-"use client";
-
 "bar";
 
 // This is a comment.

packages/next-swc/crates/core/tests/fixture/react-server-components/client-graph/client-entry/output.js

@@ -3,7 +3,6 @@
 // This is a comment.
 "foo";
 import "fs";
-"use client";
 "bar";
 // This is a comment.
 1 + 1;

packages/next/build/webpack/plugins/wellknown-errors-plugin/parseRSC.ts

@@ -3,9 +3,7 @@ import type { webpack } from 'next/dist/compiled/webpack/webpack'
 import { relative } from 'path'
 import { SimpleWebpackError } from './simpleWebpackError'
 
-export function formatRSCErrorMessage(
-  message: string
-): null | [string, string] {
+function formatRSCErrorMessage(message: string): null | [string, string] {
   if (message && /NEXT_RSC_ERR_/.test(message)) {
     let formattedMessage = message
     let formattedVerboseMessage = ''
@@ -15,6 +13,9 @@ export function formatRSCErrorMessage(
     const NEXT_RSC_ERR_REACT_API = /.+NEXT_RSC_ERR_REACT_API: (.*?)\n/s
     const NEXT_RSC_ERR_SERVER_IMPORT = /.+NEXT_RSC_ERR_SERVER_IMPORT: (.*?)\n/s
     const NEXT_RSC_ERR_CLIENT_IMPORT = /.+NEXT_RSC_ERR_CLIENT_IMPORT: (.*?)\n/s
+    const NEXT_RSC_ERR_CLIENT_DIRECTIVE = /.+NEXT_RSC_ERR_CLIENT_DIRECTIVE\n/s
+    const NEXT_RSC_ERR_CLIENT_DIRECTIVE_PAREN =
+      /.+NEXT_RSC_ERR_CLIENT_DIRECTIVE_PAREN\n/s
 
     if (NEXT_RSC_ERR_REACT_API.test(message)) {
       formattedMessage = message.replace(
@@ -49,6 +50,18 @@ export function formatRSCErrorMessage(
       )
       formattedVerboseMessage =
         '\n\nOne of these is marked as a client entry with "use client":\n'
+    } else if (NEXT_RSC_ERR_CLIENT_DIRECTIVE.test(message)) {
+      formattedMessage = message.replace(
+        NEXT_RSC_ERR_CLIENT_DIRECTIVE,
+        `\n\nThe "use client" directive must be placed before other expressions. Move it to the top of the file to resolve this issue.\n\n`
+      )
+      formattedVerboseMessage = '\n\nImport path:\n'
+    } else if (NEXT_RSC_ERR_CLIENT_DIRECTIVE_PAREN.test(message)) {
+      formattedMessage = message.replace(
+        NEXT_RSC_ERR_CLIENT_DIRECTIVE_PAREN,
+        `\n\n"use client" must be a directive, and placed before other expressions. Remove the parentheses and move it to the top of the file to resolve this issue.\n\n`
+      )
+      formattedVerboseMessage = '\n\nImport path:\n'
     }
 
     return [formattedMessage, formattedVerboseMessage]
@@ -73,6 +86,7 @@ export function getRscError(
   // https://cs.github.com/webpack/webpack/blob/9fcaa243573005d6fdece9a3f8d89a0e8b399613/lib/stats/DefaultStatsFactoryPlugin.js#L414
   const visitedModules = new Set()
   const moduleTrace = []
+
   let current = module
   while (current) {
     if (visitedModules.has(current)) break

packages/next/taskfile-swc.js

@@ -117,7 +117,12 @@ module.exports = function (task) {
       // Make sure the output content keeps the `"use client"` directive.
       // TODO: Remove this once SWC fixes the issue.
       if (/^['"]use client['"]/.test(source)) {
-        output.code = '"use client";\n' + output.code
+        output.code =
+          '"use client";\n' +
+          output.code
+            .split('\n')
+            .map((l) => (/^['"]use client['"]/.test(l) ? '' : l))
+            .join('\n')
       }
 
       // Replace `.ts|.tsx` with `.js` in files with an extension

test/e2e/app-dir/rsc-errors.test.ts

@@ -107,4 +107,23 @@ describe('app dir - rsc errors', () => {
       'The default export is not a React Component in page:'
     )
   })
+
+  it('should throw an error when "use client" is on the top level but after other expressions', async () => {
+    const pageFile = 'app/swc/use-client/page.js'
+    const content = await next.readFile(pageFile)
+    const uncomment = content.replace("// 'use client'", "'use client'")
+    await next.patchFile(pageFile, uncomment)
+    const res = await fetchViaHTTP(next.url, '/swc/use-client')
+    await next.patchFile(pageFile, content)
+
+    await check(async () => {
+      const { status } = await fetchViaHTTP(next.url, '/swc/use-client')
+      return status
+    }, /200/)
+
+    expect(res.status).toBe(500)
+    expect(await res.text()).toContain(
+      'directive must be placed before other expressions'
+    )
+  })
 })

test/e2e/app-dir/rsc-errors/app/swc/use-client/page.js

@@ -0,0 +1,7 @@
+import React from 'react'
+
+// 'use client'
+
+export default function Page() {
+  return 'hello'
+}