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

Including getPayloadHMR in a NextJS app increases the bundle size significantly #6441

Closed
domenico-ruggiano opened this issue May 21, 2024 · 13 comments
Labels
status: needs-triage Possible bug which hasn't been reproduced yet

Comments

@domenico-ruggiano
Copy link

Link to reproduction

No response

Describe the Bug

When using getPayloadHMR in a NextJS app, the bundle size increases significantly.

To Reproduce

Initial Setup

Install NextJS:

$ pnpx [email protected] --typescript .

✔ Would you like to use ESLint? … No / Yes
✔ Would you like to use Tailwind CSS? … No / Yes
✔ Would you like to use `src/` directory? … No / Yes
✔ Would you like to use App Router? (recommended) … No / Yes
✔ Would you like to customize the default import alias (@/*)? … No / Yes

Move all file into src/app/(app), tree should look like this:

$ tree --gitignore          
.
├── README.md
├── next.config.mjs
├── package.json
├── pnpm-lock.yaml
├── postcss.config.mjs
├── public
│   ├── next.svg
│   └── vercel.svg
├── src
│   └── app
│       └── (app)
│           ├── favicon.ico
│           ├── globals.css
│           ├── layout.tsx
│           └── page.tsx
├── tailwind.config.ts
└── tsconfig.json

Install Payload 3.0.0 with postgress adapter:

$ pnpx create-payload-add@beta

┌   create-payload-app 
│
◇   ────────────────────────────────────────────╮
│                                               │
│  Welcome to Payload. Let's create a project!  │
│                                               │
├───────────────────────────────────────────────╯

◇   ▲ Next.js  project detected!

◇  Install Payload in this project?
│  Yes

◇  Select a database
│  Postgres

◇  Enter PostgreSQL connection string
│  postgres://postgres:[email protected]:5432/testdb

◇  Successfully installed Payload and dependencies

◇  Payload project successfully initialized!

◇   Documentation  ────────────────────────────────────────────────────────────────╮
│                                                                                  │
│  - Getting Started: https://payloadcms.com/docs/getting-started/what-is-payload  │
│  - Configuration: https://payloadcms.com/docs/configuration/overview             │
│                                                                                  │
│                                                                                  │
├──────────────────────────────────────────────────────────────────────────────────╯

└   Have feedback?  Visit us on GitHub: https://github.com/payloadcms/payload.

Create a docker-compose.yml file to run a postgress db locally:

version: "3.9"

services:
  db:
    image: postgres:13-alpine
    environment:
      POSTGRES_PASSWORD: testpass
      POSTGRES_USER: postgres
      POSTGRES_DB: testdb
    volumes:
      - ./db_data:/var/lib/postgresql
    ports:
      - "5432:5432"

Then run docker-compose up -d.

Build Size

Running pnpm build, page / has approximately 92.7 kB of JS on first load:

$ pnpm build

Route (app)                              Size     First Load JS
┌ ○ /                                    5.25 kB        92.7 kB
├ ○ /_not-found                          904 B          88.4 kB
├ ƒ /admin/[[...segments]]               152 B           441 kB
├ ƒ /api/[...slug]                       0 B                0 B
├ ƒ /api/graphql                         0 B                0 B
├ ƒ /api/graphql-playground              0 B                0 B
└ ○ /my-route                            0 B                0 B
+ First Load JS shared by all            87.5 kB
  ├ chunks/009de13e-0259c6395e1b79aa.js  51 kB
  ├ chunks/5091-1d9cb54da3ef9f05.js      33.5 kB
  └ other shared chunks (total)          2.98 kB

Add a src/util/payload.ts file to connect to the Payload API:

"use server";

import { getPayloadHMR } from "@payloadcms/next/utilities";
import config from "@payload-config";

export async function getPayload() {
  return getPayloadHMR({ config });
}

And include it in the src/app/(app)/page.tsx file:

import { getPayload } from "@/util/payload";

export default async function Home() {
  const payload = await getPayload();
  return <h1>Hello World</h1>;
}

Now run pnpm build again, and the page / has approximately 353 kB of JS on first load, 4x the previous size:

Route (app)                              Size     First Load JS
┌ ○ /                                    464 B           353 kB
├ ○ /_not-found                          904 B          88.4 kB
├ ƒ /admin/[[...segments]]               155 B           442 kB
├ ƒ /api/[...slug]                       0 B                0 B
├ ƒ /api/graphql                         0 B                0 B
├ ƒ /api/graphql-playground              0 B                0 B
└ ○ /my-route                            0 B                0 B
+ First Load JS shared by all            87.5 kB
  ├ chunks/009de13e-0259c6395e1b79aa.js  51 kB
  ├ chunks/5091-1d9cb54da3ef9f05.js      33.5 kB
  └ other shared chunks (total)          2.98 kB

Bundle Analyzer

Install NextJS Bundle Analyzer running pnpm i @next/bundle-analyzer and next.config.mjs as following:

import { withPayload } from "@payloadcms/next/withPayload";
import bundleAnalyzer from "@next/bundle-analyzer";

const withBundleAnalyzer = bundleAnalyzer({
  enabled: process.env.ANALYZE === "true",
});

/** @type {import('next').NextConfig} */
const nextConfig = {};

export default withBundleAnalyzer(withPayload(nextConfig));

then run ANALYZE=true pnpm build.

It seems that using getPayloadHMR in the src/util/payload.ts file is causing the whole Payload UI to be included in the client bundle:

Screenshot 2024-05-21 alle 08 51 41

Payload Version

3.0.0-beta.34

Adapters and Plugins

No response

@domenico-ruggiano domenico-ruggiano added the status: needs-triage Possible bug which hasn't been reproduced yet label May 21, 2024
@richardbutler
Copy link

I'm seeing the same issue, but simply importing the getPayload function into a server component, which causes that entire lime green block to get included in the client bundle.

Repro repo is here: https://github.com/richardbutler/payloadcms-bundling-repro

  • (app)/page.tsx
    Root page with no payload imports:
    • Bundles fine, no issues
  • (app)/[...slug]/page.tsx
    Article page with slug, which uses getPayload to payload.find a list of pages in the (app)/[...slug]/layout.tsx and also uses payload.find to get the page itself in (app)/[...slug]/page.tsx.
    • Bundles the page and much of the payload admin bundle, including the (payload)/custom.scss, which causes the background colour to change on this page only.
    • I've dropped an import 'server-only' into the service making the call, and it doesn't throw the exception that it would if it was being imported by a client component.
Route (app)                              Size     First Load JS
┌ ○ /                                    142 B          87.7 kB
├ ○ /_not-found                          903 B          88.5 kB
├ ƒ /[...slug]                           418 B           357 kB
├ ƒ /admin/[[...segments]]               151 B           446 kB
└ ƒ /api/[...slug]                       0 B                0 B
+ First Load JS shared by all            87.6 kB
  ├ chunks/3182-1b8285e46916339b.js      33.6 kB
  ├ chunks/4bd1b696-1585789b00318d32.js  51 kB
  └ other shared chunks (total)          2.95 kB


○  (Static)   prerendered as static content
ƒ  (Dynamic)  server-rendered on demand

✨  Done in 28.17s.

This only happens on production builds, not on development builds, both standard Next builds and standalone Next builds.

Reference: pages side by side
image

Reference: bundle analyzer (app)/[...slug]/page.tsx
image

Reference: bundle analyzer (app)/page.tsx
image

@benyamynbrkyc
Copy link

Had the same issue as well, managed to get it answered in the Payload Discord. Thanks to @AlessioGr and @r1tsuu for helping me resolve this issue. I'm just going to leave their comments from the Discord chat here, and claim no credit.

Alessio:

"Hey hey! This is because next automatically bundles all referenced client-side components to the page's JS bundle even if they are not actually used. Thus, even if you only use getPayloadHMR on a page to fetch data, the entirety of payload gets included in the JS.

This will need to be addressed in next, as there is no way to configure that behavior right now. We have an open PR in the next.js repo here: vercel/next.js#65415

Once that's merged this will no longer be an issue! Would be great if you can give that PR an upvote 🙂"

Ritsu even created a gist for quickly patching Next.js in order to fix the bundle sizes. I managed to get it fixed thanks to it.

Gist: https://gist.github.com/r1tsuu/27cb56223d166620966edbcff44e6842

@r1tsuu
Copy link
Member

r1tsuu commented May 21, 2024

Had the same issue as well, managed to get it answered in the Payload Discord. Thanks to @AlessioGr and @r1tsuu for helping me resolve this issue. I'm just going to leave their comments from the Discord chat here, and claim no credit.

Alessio:

"Hey hey! This is because next automatically bundles all referenced client-side components to the page's JS bundle even if they are not actually used. Thus, even if you only use getPayloadHMR on a page to fetch data, the entirety of payload gets included in the JS.

This will need to be addressed in next, as there is no way to configure that behavior right now. We have an open PR in the next.js repo here: vercel/next.js#65415

Once that's merged this will no longer be an issue! Would be great if you can give that PR an upvote 🙂"

Ritsu even created a gist for quickly patching Next.js in order to fix the bundle sizes. I managed to get it fixed thanks to it.

Gist: https://gist.github.com/r1tsuu/27cb56223d166620966edbcff44e6842

yes patch works like a charm!

@domenico-ruggiano
Copy link
Author

Had the same issue as well, managed to get it answered in the Payload Discord. Thanks to @AlessioGr and @r1tsuu for helping me resolve this issue. I'm just going to leave their comments from the Discord chat here, and claim no credit.
Alessio:
"Hey hey! This is because next automatically bundles all referenced client-side components to the page's JS bundle even if they are not actually used. Thus, even if you only use getPayloadHMR on a page to fetch data, the entirety of payload gets included in the JS.
This will need to be addressed in next, as there is no way to configure that behavior right now. We have an open PR in the next.js repo here: vercel/next.js#65415
Once that's merged this will no longer be an issue! Would be great if you can give that PR an upvote 🙂"
Ritsu even created a gist for quickly patching Next.js in order to fix the bundle sizes. I managed to get it fixed thanks to it.
Gist: https://gist.github.com/r1tsuu/27cb56223d166620966edbcff44e6842

yes patch works like a charm!

Thank you, it works perfectly! Just a note: you need to use Node v22 and if you are using src directory, you need to change next.config.mjs into:

const nextConfig = {
  experimental: {
    serverOnlyDependencies: [
      resolve(import.meta.dirname, "src", "payload.config.proxy.ts"),
    ],
  },
};

@r1tsuu May I ask how to create a patch for the new canary versions of Next.js, such as 14.3.0-canary.75?

Thanks for your support!

@r1tsuu
Copy link
Member

r1tsuu commented May 22, 2024

Had the same issue as well, managed to get it answered in the Payload Discord. Thanks to @AlessioGr and @r1tsuu for helping me resolve this issue. I'm just going to leave their comments from the Discord chat here, and claim no credit.
Alessio:
"Hey hey! This is because next automatically bundles all referenced client-side components to the page's JS bundle even if they are not actually used. Thus, even if you only use getPayloadHMR on a page to fetch data, the entirety of payload gets included in the JS.
This will need to be addressed in next, as there is no way to configure that behavior right now. We have an open PR in the next.js repo here: vercel/next.js#65415
Once that's merged this will no longer be an issue! Would be great if you can give that PR an upvote 🙂"
Ritsu even created a gist for quickly patching Next.js in order to fix the bundle sizes. I managed to get it fixed thanks to it.
Gist: https://gist.github.com/r1tsuu/27cb56223d166620966edbcff44e6842

yes patch works like a charm!

Thank you, it works perfectly! Just a note: you need to use Node v22 and if you are using src directory, you need to change next.config.mjs into:

const nextConfig = {
  experimental: {
    serverOnlyDependencies: [
      resolve(import.meta.dirname, "src", "payload.config.proxy.ts"),
    ],
  },
};

@r1tsuu May I ask how to create a patch for the new canary versions of Next.js, such as 14.3.0-canary.75?

Thanks for your support!

you can change the patch key in the package json (and pray that nothing was changed with this version in the patch diff context)

    "patchedDependencies": {
      "[email protected]": "patches/[email protected]"
    }

@richardbutler
Copy link

richardbutler commented May 22, 2024

@r1tsuu Legend. This works a charm, I owe you beer.

@domenico-ruggiano I manually applied the same changes on Next 14.2.3 (not canary) using yarn patch (instructions here). I'm also using Node 20 and everything is fine. I've added my patch file to the gist.

@denolfe
Copy link
Member

denolfe commented May 22, 2024

Love to see everyone getting crafty in this interim period 👍

@denolfe denolfe closed this as completed May 22, 2024
@denolfe denolfe pinned this issue May 22, 2024
@domenico-ruggiano
Copy link
Author

domenico-ruggiano commented May 22, 2024

@r1tsuu Legend. This works a charm, I owe you beer.

@domenico-ruggiano I manually applied the same changes on Next 14.2.3 (not canary) using yarn patch (instructions here). I'm also using Node 20 and everything is fine. I've added my patch file to the gist.

@richardbutler my bad, I was using node v20.9.0. You're right it works perfectly also using Node v20 lts/iron.

Thanks for the info, I know how to apply a patch with yarn or npm. I'm just curious, how do you generated the git diff file? I tried cloning the repo with the patch and generating the diff file by comparing the pull request with the released version in the official Next.js repository (or visiting https://github.com/vercel/next.js/compare/canary...jmikrut:next.js:feat/%2350285-server-only-deps.diff), but I couldn't manage it because the differences were calculated on the source code and not on the "/dist/build/" folder.

@JarrodMFlesch JarrodMFlesch unpinned this issue May 24, 2024
@domenico-ruggiano
Copy link
Author

@denolfe after upgrading payload to version v3.0.0-beta.49 (I'm currently on v3.0.0-beta.52) the patch doesn't work anymore.

After build, I'm getting this error:

Error: Could not find the module "./node_modules/.pnpm/@[email protected]_@[email protected][email protected]_bcseew2il5vot4h3q2q2cxajke/node_modules/@payloadcms/richtext-lexical/dist/exports/client/index.js#IndentFeatureClientComponent" in the React Client Manifest. This is probably a bug in the React Server Components bundler.
    at ek (./node_modules/.pnpm/[email protected][email protected][email protected]._vbcca4r3lbbuwgj3z4anxkkfoa/node_modules/next/dist/compiled/next-server/app-page.runtime.prod.js:84:14370)
....

@denolfe
Copy link
Member

denolfe commented Jun 23, 2024

@domenico-ruggiano The patch is not officially supported. It will need to be redone by the author.

@rilrom
Copy link
Contributor

rilrom commented Jul 17, 2024

I created a patch based on the PR the payload team have created in next, however it doesn't seem to work. Do we know if the PR is still working with the latest canary versions of next 15?

Edit: the patch works when I upgrade from 15.0.0-canary.53 to 15.0.0-canary.69. Unfortunately I now receive a similar error to @domenico-ruggiano when attempting to visit the admin panel. Once I remove the patch I am able to visit the admin panel again.

jacobsfletch pushed a commit that referenced this issue Aug 5, 2024
This PR ensures that the payload config is 100% server-only. Instead of
importing custom components into your config directly, they are now
defined as file paths and rendered only when needed. That way, the
payload 3.0 release does not rely on
vercel/next.js#65415 which will likely will not
get merged. Now, the Payload config will be significantly more
lightweight.

Related discussion:
#6938

## The issue

This PR fixes a few issues:

#4085
#6598
#6441
#7261 #7339

### Local API within Next.js routes

Previously, if you used the payload local API within Next.js pages, all
the client-side modules are being added to the bundle for that specific
page, even if you only need server-side functionality.

This `/test` route, which uses the payload local API, was previously 460
kb. It is now down to 91 kb and does not bundle the payload client-side
admin panel anymore.

All tests done
[here](https://github.com/payloadcms/payload-3.0-demo/tree/feat/path-test)
with beta.67/PR, db-mongodb and default richtext-lexical:

**dev /admin before:**
![CleanShot 2024-07-29 at 22 49
12@2x](https://github.com/user-attachments/assets/4428e766-b368-4bcf-8c18-d0187ab64f3e)

**dev /admin after:**
![CleanShot 2024-07-29 at 22 50
49@2x](https://github.com/user-attachments/assets/f494c848-7247-4b02-a650-a3fab4000de6)

---

**dev /test before:**
![CleanShot 2024-07-29 at 22 56
18@2x](https://github.com/user-attachments/assets/1a7e9500-b859-4761-bf63-abbcdac6f8d6)

**dev /test after:**
![CleanShot 2024-07-29 at 22 47
45@2x](https://github.com/user-attachments/assets/f89aa76d-f2d5-4572-9753-2267f034a45a)

---

**build before:**
![CleanShot 2024-07-29 at 22 57
14@2x](https://github.com/user-attachments/assets/5f8f7281-2a4a-40a5-a788-c30ddcdd51b5)

**build after::**
![CleanShot 2024-07-29 at 22 56
39@2x](https://github.com/user-attachments/assets/ea8772fd-512f-4db0-9a81-4b014715a1b7)

### Usage of the payload local API / config outside of Next.js

This will make it a lot easier to use the payload config / local API in
other, server-side contexts. Previously, you might encounter errors due
to client files (like .scss files) not being allowed to be imported.

### Custom Client Components are not server-rendered anymore

Previously, custom components would be server-rendered in
`buildComponentMap`, no matter if they are server or client components.
Now, only server components are rendered in `buildComponentMap`. Client
components simply get passed through, using `MappedComponent` as a
container. It is then the client's responsibility to render those.

This makes rendering them on the client a little bit more complex, as
you now have to check if that component is a server component (=>
already has been rendered) or a client component (=> not rendered yet,
has to be rendered here). However, this added complexity has been
alleviated through the easy-to-use `<RenderMappedComponent />` helper.

This helper now also handles rendering arrays of custom components (e.g.
beforeList, beforeLogin ...), which actually makes rendering custom
components easier in some cases.

The benefit of this change:

Custom client components can now receive props. Previously, the only way
for them to receive dynamic props from a parent client component was to
use hooks, e.g. `useFieldProps()`. Now, we do have the option of passing
in props to the custom components directly, if they are client
components. This will be simpler than having to look for the correct
hook,

### Misc improvements

This PR includes misc, breaking changes. For example, we previously
allowed unions between components and config object for the same
property. E.g. for the custom view property, you were allowed to pass in
a custom component or an object with other properties, alongside a
custom component.

Those union types are now gone. You can now either pass an object, or a
component. The previous `{ View: MyViewComponent}` is now `{ View: {
Component: MyViewComponent} }` or `{ View: { Default: { Component:
MyViewComponent} } }`.

This dramatically simplifies the way we read & process those properties,
especially in buildComponentMap. We can now simply check for the
existence of one specific property, which always has to be a component,
instead of running cursed runtime checks on a shared union property
which could contain a component, but could also contain functions or
objects.

![CleanShot 2024-07-29 at 23 07
07@2x](https://github.com/user-attachments/assets/1e75aa4c-7a4c-419f-9070-216bb7b9a5e5)

![CleanShot 2024-07-29 at 23 09
40@2x](https://github.com/user-attachments/assets/b4c96450-6b7e-496c-a4f7-59126bfd0991)


## Custom Components

In order to reference any custom components in the payload config, you
now have to specify a string path to the component instead of importing
it.

E.g.

```ts
import { MyComponent2} from './MyComponent2.js'

admin: {
  components: {
    Label: MyComponent2
  },
},
```

Now becomes:

```ts
admin: {
  components: {
    Label: '/collections/Posts/MyComponent2.js#MyComponent2', // <= has to be a relative path based on a baseDir configured in the payload config - NOT relative based on the importing file
  },
},
```

## Concepts: MappedComponent and PayloadComponent

Instead of `React.FC` / `React.ComponentType`, props within the payload
config that accept components now accept `PayloadComponent`. These are
basically the paths to the component. They can be a string, or an object
containing other properties like serverProps and clientProps - giving
you full control over the props this component will eventually receive.

`MappedComponent` is the result of buildComponentMap processing all
those `PayloadComponent`s. It contains the server-rendered component, or
unrendered/rendered client component. Any `MappedComponent` can be
rendered on both either the client and the server using the
`RenderMappedComponent` helper component.

## Try it out
This can be tested out here:
https://github.com/payloadcms/payload-3.0-demo/tree/feat/path-test

## Todo
- Migrate all payload plugins to new import pattern & get it to build
- Migrate test suites to new import pattern
- Clean-up, simplify component handling within core, add JSDocs
- Add Docs
- Collect all breaking changes

---------

Co-authored-by: PatrikKozak <[email protected]>
Co-authored-by: Paul <[email protected]>
Co-authored-by: Paul Popus <[email protected]>
@AlessioGr AlessioGr mentioned this issue Aug 13, 2024
1 task
@AlessioGr
Copy link
Member

This has been fixed by #7620

Copy link

github-actions bot commented Sep 6, 2024

This issue has been automatically locked.
Please open a new issue if this issue persists with any additional detail.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: needs-triage Possible bug which hasn't been reproduced yet
Projects
None yet
Development

No branches or pull requests

7 participants