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

[Next13 App dir] Bug with server-only being used in an API route handler #43700

Closed
1 task done
KolbySisk opened this issue Dec 4, 2022 · 29 comments · Fixed by #55394
Closed
1 task done

[Next13 App dir] Bug with server-only being used in an API route handler #43700

KolbySisk opened this issue Dec 4, 2022 · 29 comments · Fixed by #55394
Labels
bug Issue was opened via the bug report template. linear: next Confirmed issue that is tracked by the Next.js team. locked

Comments

@KolbySisk
Copy link

KolbySisk commented Dec 4, 2022

Verify canary release

  • I verified that the issue exists in the latest Next.js canary release

Provide environment information

Operating System:
  Platform: darwin
  Arch: arm64
  Version: Darwin Kernel Version 22.0.0: Mon Aug  1 06:32:20 PDT 2022; root:xnu-8792.0.207.0.6~26/RELEASE_ARM64_T6000
Binaries:
  Node: 16.15.0
  npm: 8.5.5
  Yarn: N/A
  pnpm: N/A
Relevant packages:
  next: 13.0.6
  eslint-config-next: 13.0.6
  react: 18.2.0
  react-dom: 18.2.0

Which area(s) of Next.js are affected? (leave empty if unsure)

App directory (appDir: true)

Link to reproduction - Issues with a link to complete (but minimal) reproduction code will be addressed faster

https://github.com/KolbySisk/next-server-only-bug

To Reproduce

  1. Create a client component that makes a request to an api route (A form that posts data to an api route, for example)
    2.npm i server-only
  2. Add import "server-only"; to the api route.
  3. Run and make the request to the api route.

See error:

You're importing a component that needs server-only. That only works in a Server Component but one of its parents is marked with "use client", so it's a Client Component.

Describe the Bug

API routes are handled only on the server, so I would expect import "server-only"; to work fine.

More realistically, I have a function which includes sensitive information that I want to ensure only runs on the server. It can be used in both a server component, and an API route, given the context.

I'm also curious how Next even knows about the relationship between a client component and an API route handler.

Expected Behavior

Code is executed as expected without the error.

Which browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

No response

NEXT-1385

@KolbySisk KolbySisk added the bug Issue was opened via the bug report template. label Dec 4, 2022
@Fredkiss3
Copy link
Contributor

I think you don’t need to import server only in an api route. Since when used in a api route it is sure to only run on the server.

I think the problem is that server-only can only work and give you the correct errors if it used in a context of a component, and maybe NextJs confuse the api handler for a component when import server-only is used, I may be wrong though.

@HStromfelt
Copy link

HStromfelt commented Dec 5, 2022

I think there is still an issue with API routes wrongly being confused with client components. I have a function that uses:

import { cookies } from "next/headers";

and is used in an API route. The function should only be run by server-side code so this is ok, but NextJS thinks that the calling functions (API routes) are client-side:

./lib/getUser.js

You're importing a component that needs next/headers. That only works in a Server Component but one of its parents is marked with "use client", so it's a Client Component.

   ,----
 7 | import { cookies } from "next/headers";
   : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   `----

One of these is marked as a client entry with "use client":
  lib/getUser.js
  pages/api/stripe/create-account.js

I can fix by adding import "server-only" to the api files but then I get the same error as @KolbySisk.

@Fredkiss3
Copy link
Contributor

Fredkiss3 commented Dec 5, 2022

I think it is rightfully a bug, with NextJs confusing the api route as a client component, but I also think that these directives should fail, as you imported cookies which is a function you can only use in server components but not anywhere else, you’d have to use the request object in your api handler.

same thing for server-only which is supposed to be a guardrail when you import them from a react component (wether it is client or server one).

When you try to use them in a different context out of react (i.e api handlers) NextJs is lost and produces and generic incorrect error

@ayhanap

This comment was marked as resolved.

@Fredkiss3

This comment was marked as resolved.

@larsniet

This comment was marked as resolved.

@boatilus

This comment was marked as off-topic.

@Fredkiss3

This comment was marked as off-topic.

@boatilus

This comment was marked as off-topic.

@logemann
Copy link

logemann commented Jan 3, 2023

I also have this bug. Just creating a simple API route like this one:

import type { NextApiRequest, NextApiResponse } from 'next'
import { headers, cookies } from 'next/headers';

type Data = {
  name: string
}

export default function handler(
  req: NextApiRequest,
  res: NextApiResponse<Data>
) {
  console.log(headers);
  res.status(200).json({ name: 'John Doe' })
}

Error is this:

You're importing a component that needs next/headers. That only works in a Server Component but one of its parents is marked with "use client", so it's a Client Component.
...
One of these is marked as a client entry with "use client":
  pages/api/hello.ts

So NextJS treats this api route component as a client component?! In the beta docs there is no hint that API routes are not working with latest release so that i am still a bit unsure if this is really a bug.

NextJS v13.1.1

@Fredkiss3
Copy link
Contributor

@logemann

You can only import { cookies } from 'next/headers'; inside of server components and not anywhere else.
In api handlers, you should use the req object with req.headers (I think) and req.cookies.

@jpreynat

This comment was marked as off-topic.

@m-foskett

This comment was marked as off-topic.

@Fredkiss3

This comment was marked as resolved.

@ovy9086

This comment was marked as spam.

@reza-ebrahimi

This comment was marked as resolved.

@purboindra

This comment was marked as spam.

@OliverRC

This comment was marked as resolved.

@timneutkens

This comment was marked as resolved.

@benminor

This comment was marked as resolved.

@OliverRC

This comment was marked as resolved.

@timneutkens

This comment was marked as resolved.

@OliverRC

This comment was marked as resolved.

@Phoenixmatrix
Copy link

Just throwing my name here for a 👍 for this issue, as I just hit it.

I use import "server-only" in my backend authentication code to make sure no one imports it in a client component tree. But I need to use the same code in server components as I do in APIs. Unfortunately, server-only fails in API routes, so I can't. I have to move it up to a wrapper or something made just for server components, but then my auth code is vulnerable to someone making an honest mistake.

It would be great if I could use server-only to poison my code against client side usage but still have it work with route handlers.

@timneutkens
Copy link
Member

It would be great if I could use server-only to poison my code against client side usage but still have it work with route handlers.

Route handlers should be fine. This issue in particular is about pages/api, not route handlers.

@matzkoh
Copy link

matzkoh commented Aug 3, 2023

I'm using pages/api because tRPC doesn't support App Router yet. However, having server-only in the import chain from pages/api/trpc/[trpc].ts causes issues, and there seems to be no way to avoid it even with file splitting.

Currently, I have to comment out import 'server-only' to get around this problem.

@ksdaylight

This comment was marked as off-topic.

@MincePie

This comment was marked as off-topic.

@kodiakhq kodiakhq bot closed this as completed in #55394 Sep 18, 2023
kodiakhq bot pushed a commit that referenced this issue Sep 18, 2023
…nts targets to be available (#55394)

Users want to use `server-only` to restrict the middleware / app routes / pages api, but now it's failing as we're treating them as different webpack layers, but validating the `server-only` only with server components layers.

Here we modify the rules a bit to let everyone can use "server-only" for the bundles that targeting server-side.

For next-swc transformer, we introduce the new option `bundleType` which only has `"server" | "client" | "default"` 3 values:
* - `server`  for server-side targets, like server components, app routes, pages api, middleware
* - `client`   for client components targets such as client components app pages, or page routes under pages directory.
* - `default` for environment like jest, we don't validate module graph with swc, replaced the `disable_checks` introduced  [#54891](#54891).

Refactor a bit webpack-config to adapt to the new rules, after that `server-only` will be able to used in the server-side targets conventions like middleware and `pages/api`

Fixes #43700
Fixes #54549
Fixes #52833

Closes NEXT-1616
Closes NEXT-1607
Closes NEXT-1385
@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2023

This closed issue has been automatically locked because it had no new activity for 2 weeks. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot added the locked label Oct 3, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue was opened via the bug report template. linear: next Confirmed issue that is tracked by the Next.js team. locked
Projects
None yet